[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