[FFmpeg-devel] [PATCH] correct MPEG video parser not to return incomplete frames

[Ivan Schreter]

Hi,

regarding this conversation in "base av_read_frame() function for 
seeking in mpegps files" thread:

Ivan Schreter wrote:
> Hi Baptiste,
>
> Baptiste Coudurier wrote:
>> On 07/26/2009 10:48 AM, Ivan Schreter wrote:
>>> zhentan feng wrote:
>>>> [...]
>>>> I am not clear why the parser return the wrong frame size when fist 
>>>> call
>>>> av_read_frame() after seek.
>>>> anyone explain it futher?
>>>>
>>> I looked at the code. Consider following example: A PES packet contains
>>> some rest of previous frame and start of a new frame. In normal case,
>>> the rest of the previous frame is simply glued to the data in the 
>>> buffer
>>> and returned as whole frame with PTS of the previous frame during
>>> processing of this PES packet in the parser.
>>>
>>> However, after a seek, parser is reset. Instead of ignoring the portion
>>> of previous frame, it returns it back as a frame with PTS of the 
>>> current
>>> frame. Beginning of current frame (to which PTS actually belongs) is 
>>> put
>>> into the buffer and when the frame is completed, it is returned back
>>> without PTS.
>>>
>>> So, we have one corrupted frame with the timestamp of the actual frame
>>> before actual frame (most probably of correct size, but I didn't 
>>> check),
>>> which is returned without timestamp.
>>>
>>> Code in mpegvideo_parser.c and possibly in ff_combine_frame() must be
>>> fixed to take this case into account (i.e., newly-initialized parser 
>>> and
>>> rest of old frame in the first packet received from the stream).
>>
>> Yes I think this partial frame must be discarded and it's easy to do 
>> that by checking headers.
>>
>
> [...]
> Could you please elaborate which headers you meant? I can surely find 
> them myself, but it would help if you give me (or Zhentan) some 
> pointers...

Update: I found a way how to easily detect it. According to standard, 
each access unit in MPEG video contains a picture start code (which 
defines for instance picture type - I/P/B) and at least one slice. Thus, 
it is enough to ignore buffers, which don't contain both.

With the attached patch, reading a video frame after a seek results in 
proper frame.

Please review.

Note: The patch causes a regression in seek tests. It looks wrong, but I 
checked the stream - it's really correct. After seeking to a TS which 
would cause positioning the stream to video TS 1.460000, next frame read 
will return audio frame with TS 1.022444. The cause is, the audio frame 
is interleaved with video frame and will finish reading first. There are 
follow-up packets for video frame outstanding. Old code would return the 
last fragment of _previous_ video frame tagged with PTS 1.460000, which 
is wrong.

Thanks & regards,

Ivan

-------------- next part --------------
A non-text attachment was scrubbed...
Name: mpegvideo_parser_fix.patch
Type: text/x-patch
Size: 2686 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090802/5f6ab874/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: mpegvideo_parser_regress.patch
Type: text/x-patch
Size: 2222 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090802/5f6ab874/attachment-0001.bin>