[FFmpeg-devel] [PATCH] Fix MPEG-TS seek and frame positions in general

[Ivan Schreter]

Michael Niedermayer wrote:
> On Tue, Feb 10, 2009 at 11:28:01AM +0100, Ivan Schreter wrote:
>   
>> Michael Niedermayer wrote:
>>     
>>> On Mon, Feb 09, 2009 at 01:35:58AM +0100, Ivan Schreter wrote:
>>>   
>>>       
>>>> So here we go, patch #4 attached.
>>>>     
>>>>         
>>> ive looked more at it and it looks very wrong
>>> the newly added variable is a duplicate of cur_pkt.pos and you override
>>> the use of cur_pkt.pos at some places with it.
>>> i suspect that moving got_packet: up one line has the same effect as your
>>> rather bloated patch
>>>
>>>   
>>>       
>> Maybe it looks wrong to you, but I still believe it's very right.
>>
>> Moving the label one line up will address only the case where frame is 
>> completely contained in one packet (where current code sets no position 
>>     
>
> right, sorry, current code is broken no matter where that label is.
> but yours still is nowhere near correct
>
>   
I think this is way too harsh statement.

> the time when a frame end/start is detected can be several packets after
> the point where the first byte of that frame was stored.
> the obvious way to fix this is to pass pos into av_parser_parse()
> and reorder it like the timestamps.
>   
The only objection could be (to see the problem you are mentioning), if 
packets inside one AVStream (e.g., single video stream) would be 
interleaved. I cannot really imagine why would someone do it, but the 
generic solution would not work here correctly. For such case, the 
solution with passing of position to the parser and letting the parser 
do the reordering would be needed.

I feel 99-100% of the formats won't need this and would unneccessarily 
have to implement position storage by themselves, when it can be done 
generically.

My solution is maybe not 100% complete, but for real-world is definitely 
better than no solution. So why not use it? We both agree it's better 
than what is there today. In the future, if someone sees the need to 
support position in AVStreams with interleaved packets correctly, he can 
still implement it.

Regards,

Ivan