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

Michael Niedermayer michaelni
Sun Aug 2 17:16:25 CEST 2009

On Sun, Aug 02, 2009 at 05:11:55PM +0200, Ivan Schreter wrote:
> Hi,
> Michael Niedermayer wrote:
>> On Sun, Aug 02, 2009 at 03:58:59PM +0200, Ivan Schreter wrote:
>> [...]
>>> @@ -146,7 +153,12 @@
>>>      /* we have a full frame : we just parse the first few MPEG headers
>>>         to have the full timing information. The time take by this
>>>         function should be negligible for uncorrupted streams */
>>> -    mpegvideo_extract_headers(s, avctx, buf, buf_size);
>>> +    if (mpegvideo_extract_headers(s, avctx, buf, buf_size) < 0) {
>>> +        /* garbled frame, ignore (possibly first read after seek) */
>>> +        *poutbuf = NULL;
>>> +        *poutbuf_size = 0;
>>> +        return next;
>>> +    }
>>>  #if 0
>>>      printf("pict_type=%d frame_rate=%0.3f repeat_pict=%d\n",
>>>             s->pict_type, (double)avctx->time_base.den / 
>>> avctx->time_base.num, s->repeat_pict);
>> as already said elsewhere, its not the job of parsers to discard data
>> it would break decoders that are capable to decode damaged frames, like
>> our decoder.
> The problem is not in the decoder, but rather parser. Namely, associated 
> data (PTS, DTS, position, etc.) are passed incorrectly.

your patch _creates_ a new problem for the decoder

>> in addition to this your patch is VERY broken in a subtile and hard to
>> debug way, that is it will detect 99% of the incomplete frames but not
>> all from what it doesnt detect again most but not all would be fine,
>> that would be a nightmare to debug as it would occur so rarely and
>> require exact seeking to the problematic spot to reproduce ...
>> the issue and that might not be the only one is that there can be
>> things prior to the picture start code that can matter ...
> It is also not the intention to detect 100% of incomplete frames. The 
> intention is only to detect partial frame after the seek, so we don't 
> return nonsense back (which breaks seeking).

but it does not detect 100% of the partial frames after a seek

> Possibly, the check could be made in a "fresh" parser only, so that we at 
> most lose very first broken frame of a stream and it would still correct 
> the seeking...
> Otherwise, how would you do it? The problem:

i dont know, my first thought would be just to discard the first frame
the parser outputs after seeking no matter if it looks valid or not.
Of course thats just a thought and may have issues as well i dont know


Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Those who are too smart to engage in politics are punished by being
governed by those who are dumber. -- Plato 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090802/1e10bb0a/attachment.pgp>

More information about the ffmpeg-devel mailing list