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

Michael Niedermayer michaelni
Mon Aug 31 14:35:41 CEST 2009


On Sun, Aug 30, 2009 at 03:46:33PM +0200, Ivan Schreter wrote:
> Michael Niedermayer wrote:
>> On Sat, Aug 22, 2009 at 10:07:48AM +0200, Ivan Schreter wrote:
>>   
>>> Hi Michael,
>>>
>>> Michael Niedermayer wrote:
[...]
>
>> [...]
>>> OK, it would be maybe possible to get the additional information about 
>>> where in PES packet the frame starts and handle it in generic parser. 
>>> BUT:     
>>
>> its certainly possible
>>   
> In MPEG-1 and MPEG-2 video it is indeed possible to detect, where the next 
> frame starts, but this is done by searching for picture start code and 
> slice start code. There is no additional information for frame start 
> detection.

yes


>
>>
>>   
>>> This won't work as well. Namely, this would only fix the problem with 
>>> first stream encountered after a seek. If there is a PES packet for 
>>> stream 1 followed by a PES packet for stream 2, which has exactly the 
>>> same problem (rest of previous frame without picture start code and slice 
>>> start code and start of a new frame and contains PTS/DTS), then the 
>>> problem would simply happen in stream 2.
>>>     
>>
>> AVStream.initial_bytes_to_discard
>> and that amount is discarded after the parser
>>   
> A possiblity, yes. But who will fill it and when?

your seeking code, and when a seek happens


>
> I actually find your suggestion of fixing the misassociation of the 
> timestamp to the frame first (at the end) much better. So let's concentrate 
> on that one.

You speak of them as if they where 2 alternatives, they are not!
... neither where the 4 h264 timestamp cases from the spec, you implemented
one leaving 3/4 of the cases unsupported ...

here we do have a bug with timestamps being associated wrongly, this should be
fixed
Also the mpeg demuxer returns random trash prior to the first frame, this IS
a bug in your seeking code. Fix it please! And before you start arguing about
how unlikely it would cause a problem in mpeg video, it will cause problems
in mpeg audio as well.
The goal of the seeking code is to be exact, if now the first frame can be
invalid without this being detectable the decoder or parser would have to
discard it and this would break exactness


[...]
> > [...]
>> IIRC our parser misassociates a timestamp in the partial frame case, this
>> has to be fixed _first_. I am against adding new features on top of known
>> bugs.
>>   
> I looked at the code in mpegvideo_parse() and ff_mpeg1_find_frame_end(). To 
> me, it seems like ff_mpeg1_find_frame_end() misassociates the timestamp. 
> Namely, the timestamp is fetched there, but actually the _previous_ frame 
> (or in this case, a non-frame) is returned from the parsing routine.
>
> I'm not sure how to fix this, though. Possibly, instead of fetching the 
> timestamp in ff_mpeg1_find_frame_end(), it should be done in this way:
>

>    * If the frame starts at offset 0 in the PES packet AND we don't
>      have anything in the buffer, then fetch the timestamp.

sounds like another one of your not always working hacks

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Complexity theory is the science of finding the exact solution to an
approximation. Benchmarking OTOH is finding an approximation of the exact
-------------- 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/20090831/bcace9ad/attachment.pgp>



More information about the ffmpeg-devel mailing list