ffprobe reports MPEG 2 GOP-header timecodes a frame too early

[MaxEliaserAWS]

Hi maintainers! Reaching out with an issue in the mpeg2video decoder and corresponding ffprobe output, but I also want to extend the gratitude of AWS Elemental for this awesome analysis tool!

Versions Tested

I have reproduced this in the following FFMPEG/ffprobe versions:

• 3.4.7 (packaged by RedHat)
• 5.1.1 (static build from johnvansickle.com)
• 7.0.2 (static build from johnvansickle.com)
• Git commit 6cf4186d1b, compiled myself on AmazonLinux 2. This was the latest master as of this writing.

Input File Description

Just to give everyone their due, I used as test content the short film _Big Buck Bunny_, which is copyright 2008 by the Blender Foundation under a CC-BY 3.0 license, see ​https://peach.blender.org/about/.

This test file is an MPEG2 essence (e.g. ISO/IEC 13818-2 without using a transport stream wrapper.) It has a fixed 90 frame GOP size and GOP timecodes are enabled. There are no B-frames and no frame reordering. Therefore, there should be embedded timecodes on each GOP header, at frames 0, 90, 180, 270, etc, and these frame indices should be the same in decode order and presentation order. I have confirmed that this is the case using multiple other analyzer tools. You can see in these screenshots that Telestream Switch shows a timecode 00:00:03;00 on the I-frame at frame index 90 and indicates that the GOP header is present on that frame, whereas Switch does not show a GOP header present the P-frame at frame index 89 (although it can still extrapolate a timecode for that frame.)

Steps To Reproduce

ffprobe -v quiet -print_format json -show_frames out_MPEG.m2v

Expected Result

The GOP timecodes should be reported on frames 0, 90, 180, 270, etc. The results should be consistent with the coded_picture_number field as well as the actual index of the frame within the JSON array.

Since coded_picture_number appears to have been removed in latest Git, I wrote this short Python script to identify which frames have GOP timecodes in a way that will be compatible with the latest ffprobe's output format.

#! /usr/bin/env python3

import sys, json

if len(sys.argv) > 1:
    infile = open(sys.argv[1], "r")
else:
    infile = sys.stdin

content = json.load(infile)

for i, frame in enumerate(content["frames"]):
    if "side_data_list" in frame:
        for side_data in frame["side_data_list"]:
            if side_data["side_data_type"] == "GOP timecode":
                print("Found GOP timecode %s on frame %d" % (side_data["timecode"], i))
                break

Actual Result

Although the timecode is reported on frame 0 as expected, subsequently they are reported a frame too early; on frames 89, 179, 269, etc.

~/FFmpeg$ ./ffprobe -v quiet -print_format json -show_frames /tmp/out_MPEG.m2v | /tmp/analyze_ffprobe_output.py
Found GOP timecode 00:00:00;00 on frame 0
Found GOP timecode 00:00:03;00 on frame 89
Found GOP timecode 00:00:06;00 on frame 179
Found GOP timecode 00:00:09;00 on frame 269
Found GOP timecode 00:00:12;00 on frame 359
Found GOP timecode 00:00:15;00 on frame 449

I am confident that this does not match the actual structure of the file.

Tentative Analysis

I did some debugging of the issue in ffprobe 3.4.7; although I have reproed in latest Git, I did not repeat the debugging exercise there. This is the best diagnosis I can give but I am not an expert on this codebase so take with a grain of salt.

From what I can tell, the code in mpeg12dec.c can sometimes buffer a frame internally before emitting it. I believe this is the "latency of 1 frame" documented here:
​https://github.com/FFmpeg/FFmpeg/blob/afb06aef7ebeaecd843d0af62dd32546245475c2/libavcodec/mpeg12dec.c#L1752

So when decode_chunks in that file encounters the GOP header, it stashes the GOP timecode in Mpeg1Context.timecode_frame_start:
​https://github.com/FFmpeg/FFmpeg/blob/afb06aef7ebeaecd843d0af62dd32546245475c2/libavcodec/mpeg12dec.c#L2331
​https://github.com/FFmpeg/FFmpeg/blob/afb06aef7ebeaecd843d0af62dd32546245475c2/libavcodec/mpeg12dec.c#L2149

It then encounters the first frame in the new GOP and stores it in MpegEncContext.cur_pic:
​https://github.com/FFmpeg/FFmpeg/blob/afb06aef7ebeaecd843d0af62dd32546245475c2/libavcodec/mpeg12dec.c#L2446
​https://github.com/FFmpeg/FFmpeg/blob/afb06aef7ebeaecd843d0af62dd32546245475c2/libavcodec/mpeg12dec.c#L1358

And, when decode_chunks is about to return, it calls into slice_end to flush the decoded frame:
​https://github.com/FFmpeg/FFmpeg/blob/afb06aef7ebeaecd843d0af62dd32546245475c2/libavcodec/mpeg12dec.c#L2198
However, slice_end can (for I-frames and P-frames) instead emit MpegEncContext.last_pic, which goes into the AVFrame which we are working on:
​https://github.com/FFmpeg/FFmpeg/blob/afb06aef7ebeaecd843d0af62dd32546245475c2/libavcodec/mpeg12dec.c#L1752
After slice_end and decode_chunks have returned, mpeg_decode_frame sees the timecode stashed in Mpeg1Context.timecode_frame_start and always attaches it to the same AVFrame, regardless of whether that timecode was actually destined for cur_pic instead of for last_pic:
​https://github.com/FFmpeg/FFmpeg/blob/afb06aef7ebeaecd843d0af62dd32546245475c2/libavcodec/mpeg12dec.c#L2560

Since the MPEG2 format only ever puts timecodes at the start of a new GOP (definitionally always an I frame,) then aside from frame 0, I believe this code will _always_ assign the timecode to the wrong frame.

[MaxEliaserAWS]

the input file in question