[FFmpeg-devel] [PATCH] fix speex sample

[Michael Niedermayer]

On Thu, Apr 09, 2009 at 01:12:40PM -0700, Baptiste Coudurier wrote:
> 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.

if closed_gop==0 that will still happen even when this feature request is
implemented.


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

encoders should not begin encoding with B frames, they begin with an I frame
thus i cannot see how this could happen


> 
> 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.
> 
[...]
> Basically:
> 
> I pts 3
> B pts 1
> B PTS 2

if you seek to pts=1 in this case you will end up at the I frame of the
previous GOP (which is not shown) or if such does not exist seeking will
fail
it is possible to fix this by using convergence duration based on the
count of b frames, their duration and the closed_gop flag but i pitty
the one who would try to implement this because it wont be fun.


> 
> 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.

no, encoders should start with I frames not B frames


> 
> > [...]
> >>> 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 ?

for functioning concealment? certainly not, it may be enough for avoiding
a crash but it will not look pretty,
best concealment strategy is likely to drop the frames if they access
a non existent reference because they are not used as reference themselfs
droping has very little vissible effect, a single wrong MV is very vissible
(think of a green block in someones face)

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Avoid a single point of failure, be that a person or equipment.
-------------- 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/20090409/b0aaa678/attachment.pgp>