[FFmpeg-devel] [PATCH] fix speex sample

[Baptiste Coudurier]

On 4/9/2009 12:37 PM, Michael Niedermayer wrote:
> On Thu, Apr 09, 2009 at 10:24:27AM -0700, Baptiste Coudurier wrote:
>> On 4/9/2009 7:06 AM, Michael Niedermayer wrote:
>>> On Thu, Apr 09, 2009 at 09:39:24AM +0200, Diego Biurrun wrote:
>>>> On Thu, Apr 09, 2009 at 04:46:34AM +0200, Michael Niedermayer wrote:
>>>>> On Wed, Apr 08, 2009 at 06:30:56PM -0700, Baptiste Coudurier wrote:
>>>>>> Michael Niedermayer wrote:
>>>>>>> On Wed, Apr 08, 2009 at 05:26:11PM -0700, Baptiste Coudurier wrote:
>>>>>>>> On 4/8/2009 5:02 PM, Michael Niedermayer wrote:
>>>>>>>>> On Wed, Apr 08, 2009 at 04:21:32PM -0700, Baptiste Coudurier wrote:
>>>>>>>>>> Mpeg2 decoder has an important bug IMHO since some time. I
>>>>>>>>>> reported it, and libmpeg2 does not have this bug and decodes
>>>>>>>>>> correctly the first 2 frames. I lack some knowledge of the
>>>>>>>>>> surrounding code, but I tried to work on it at least. It should
>>>>>>>>>> not take you much time to figure out the problem I guess.
>>>>>>>>> thats a feature request not a bug, its something very well known
>>>>>>>>> since the code was written.
>>>>>>>> What was your argument about other implementation supporting it ? 
>>>>>>>> Oh yes, users will stop using yours to use the one supporting it.
>>>>>>>>
>>>>>>>> FYI, many of my samples use this mechanism, I just didn't really
>>>>>>>> realize it, I thought it was just broken link but finally, I
>>>>>>>> discovered that libavcodec deliberately _skip_ 2 frames, even
>>>>>>>> without telling you !
>>>>>>>>
>>>>>>>> Solution is simple, until fixed I will use libmpeg2.
>>>>>>> poor libmpeg2
>>>>>> I'd say poor libavcodec, loosing one heavy user because of lousy
>>>>>> maintainership.
>>>>> not implementing all feature requests != lousy maintainership
>>>>> if you want this feature, you can implement it.
>>>> So I will rephrase the question: What will it take for you to implement
>>>> this so that FFmpeg can be a complete libmpeg2 replacement?
>>> I honestly have no interrest in implementing this. This is not a feature
>>> that should affect any ones decission of which decoder to use.
>> I'm not sure but loosing these 2 frames is really annoying for me, I
>> basically loose frame accuracy.
> 
> i dont see how, its a gamble if you will get 0, 1, 2 or even 99 frames prior
> of the keyframe if this feature request is implemented. And that is for
> each seek, not per file.

If you decode without seeking, you loose frame accuracy because these 2
frames are duplicated from the I frame following.

That is the transcoded and the source file have different pictures at
the beginning of the stream.

And if you don't duplicate frames you just loose 2 frames.

You will seek based on pts. If you seek to 3 you will seek to the I
frame, if you seek to 1 you will seek to the I frame also since it is
before in coded order.

[mpeg2video @ 0x2585b90]vbv buffer: 1048576, bitrate:50000000
[mpeg2video @ 0x2585b90]profile: 0, level: 2 vbv buffer: 17825792,
bitrate:50000000
[mpeg2video @ 0x2585b90]sde w:1920, h:1080
[mpeg2video @ 0x2585b90]GOP ( 0:00:00.[00]) closed_gop=1 broken_link=0
[mpeg2video @ 0x2585b90]vbv_delay 25667, ref 2 type:1
[mpeg2video @ 0x2585b90]qp:6 fc:15151515 I   alt top dc:0 pstruct:3
fdct:0 cmv:0 qtype:0 ivlc:0 rff:0
[mpeg2video @ 0x2585b90]vbv_delay 28079, ref 0 type:3
[mpeg2video @ 0x2585b90]vbv_delay 28877, ref 1 type:3

Exactly here the 2 B frames are skipped.

[mpeg2video @ 0x2585b90]vbv_delay 28877, ref 5 type:2
[mpeg2video @ 0x2585b90]qp:4 fc: 4 41515 P   alt top dc:0 pstruct:3
fdct:0 cmv:0 qtype:0 ivlc:0 rff:0
[mpeg2video @ 0x2585b90]vbv_delay 28877, ref 3 type:3
[mpeg2video @ 0x2585b90]qp:6 fc: 2 2 3 3 B   alt top dc:0 pstruct:3
fdct:0 cmv:0 qtype:0 ivlc:0 rff:0
[mpeg2video @ 0x2585b90]vbv_delay 28877, ref 4 type:3
[mpeg2video @ 0x2585b90]qp:6 fc: 3 3 2 2 B   alt top dc:0 pstruct:3
fdct:0 cmv:0 qtype:0 ivlc:0 rff:0

Basically:

I pts 3
B pts 1
B PTS 2

Considering delay you almost have to double check the pts of the decoded
frame in any case IMHO.

> how that improves accuracy relative to not receiving frames prior to
> the timestamp to which you seek, i dont understand.

If you don't duplicate frames, you get 2 frames less than the source file.

> [...]
>>> Also if someone considers this feature to be so critical to him, he
>>> can implement it, its simple "patch welcome"
>> Well, I'm not disagreeing here, I tried to fix it myself and submitted a
>> patch. I'm not sure to understand everything though.
>> Like I don't clearly understand why closed_gop and broken_link would
>> matter if we get error-concealment in this case.
>> Like you said these flags might be wrongly set.
> 
> am i the only one that thinks that duplicating frames randomly through
> error concealment on every seek is a poor idea?
> 
> 
>> Do you mean that If broken_link is set, then these 2 frames cannot be
>> decoded and should be discarded ?
> 
> no closed_gop must be set to 1 for the frames to be decodeable

Ok, what if it is wrongly set to 1 ? You must have concealment right ?
Isn't allocating last_picture_ptr enough ?

-- 
Baptiste COUDURIER                              GnuPG Key Id: 0x5C1ABAAA
Key fingerprint                 8D77134D20CC9220201FC5DB0AC9325C5C1ABAAA
checking for life_signs in -lkenny... no
FFmpeg maintainer                                  http://www.ffmpeg.org