[FFmpeg-devel] [PATCH] fix speex sample

[Baptiste Coudurier]

Michael,

I'll try to be more calm and clear this time.

On 4/8/2009 7:46 PM, 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:
>>>>>> On 4/8/2009 3:46 PM, Michael Niedermayer wrote:
>>>>> [...]
>>>>>>>> The multiple stsd feature is implemented, and I
>>>>>>>> submited a patch, it depends on one seeking fix in
>>>>>>>> aviobuf.c, for which I also sent a patch.
>>>>>>>> 
>>>>>>>> Not mentioning that it takes me approximately 1 day to 
>>>>>>>> address issues on roundup relative to mov/mp4. Btw you
>>>>>>>> are listed as maintainer for mov as well ;)
>>>>>>>> 
>>>>>>>> This is effectively a different kind of
>>>>>>>> maintainership.
>>>>>>> it is not, you can look at avidec/enc and its also pretty
>>>>>>> bug free, or msmpeg4 or the mpeg1/2 decoder ...
>>>>>> msmpeg4 that should be true.
>>>>>> 
>>>>>> 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.

Well, yes, I did exaggerate here. I wanted to say that in my opinion a
maintainer should try to be more pro-active in fixing bugs if maintainer
can see that going back and forth will take much time.

Of course I understand that time is precious and we all lack time.

>> IMHO you clearly shouldn't treat your users this way.
>> 
>>> [...]
>>>>>>> flv is a little worse but not much and then there is
>>>>>>> mpeg-ts & ffserver for which i belive you are maintainer
>>>>>>> now, they are not even remotely close to bugfree not even
>>>>>>> close to flvdecs bugfreeness.
>>>>>> AFAIK I'm not official maintainer of mpeg-ts, but I don't
>>>>>> mind being maintainer and fixing bugs.
>>>>> i see, so i will suspent all my work on mpeg-ts now
>>>> I didn't know you were working on TS, may I see the patch ?
>>> There where plenty of patches on the ML that received no real
>>> review or no suggestions on how to make them acceptable IIRC. I
>>> did not mean i have some unfinished patch for TS
>> Well you'd better start working on them, then.
> 
> iam not ts maintainer nor are these my patches i tried to keep track
> of them long time ago and occasionally pinged them as mans was
> missing or forgeting about the patches. there where some that i did
> pick up and rewrote so they passed mans review but as your oppinion
> on maintainership is that the maintainer will fix patches thats
> clearly not needed anymore.

Well, if you don't mind pointing me to the patches, I'll try to review
them and apply them.

>>>>>> FFserver is certainly not bug free, however all roundup
>>>>>> issue were closed and fixed AFAIK, and it's working quite
>>>>>> ok for me.
>>>>> issue238 and 797 have ffserver in the title and are open
>>>> Humm right, it seems issue 238 is way old, I was not
>>>> maintainer back in the days, and the version is damn old, I
>>>> will close it.
>>>> 
>>>> About 797, user should use AVOption ab now anyway so this is
>>>> not an issue :)
>>>> 
>>>> Thanks for helping me closing roundup issues.
>>> its not hard to close issues while asking the user if its still 
>>> buggy, one could close anything that way
>> Definitely not, if the issue is still present, user will reopen. I 
>> believe the issue is fixed, but if not, I'll fix it.
>> 
>> I think basing debugging and work on a report that is more than one
>> year old, considering work that has been done on FFserver recently
>> will not be efficient.
> 
> i agree but chances are the user "moved on" and isnt using ffserver
> anymore or has no time/setup/interrest to test after so much time

I agree with this. Like everyone else we sometimes lack the motivation.

>>>>> also there is no working ffserver regression test, you might
>>>>> have closed all issues but as long as ffserver cant produce
>>>>> non random output its not too usefull or did you fix this?
>>>> I produces stable results here, however I'd be happy to
>>>> receive feedback on failures.
>>> if its stable, why is it not enabled in make test ?
>> Ask make test maintainer.
> 
> IMHO its the maintainer of the specific codec/demuxer/tool who should
> add a test. noone else can know better if the part is ready or not.

True. However it's complicated with FFserver. It works, but depending on
cpu/arch, it will sometimes generate different output. I've not yet
determined if this caused by the test usage itself which is wrong or
if there is a deficiency in the code.

>> Why aren't ac3 decoder tests enabled for rm ?
> 
> all written in the source:
> 
> if [ -n "$do_ac3" ] ; then do_audio_encoding ac3.rm "" -vn # gcc
> 2.95.3 compiled binaries decode ac3 differently because of missing
> SSE support #do_audio_decoding #$tiny_psnr $pcm_dst $pcm_ref 2 1024
> >> $logfile fi
> 
> or in 3 words, float rounding differences fix is welcome

Ok, I guess this was discussed before, but why didn't we disable this
specific sse function for gcc 2.95 ? Would it polute the source code too
much ?

> [...]
>>>>> and i refuse you to take co maintainership because you
>>>>> commited broken code already
>>>> No, it's not broken. It fixed the issue and I'm still waiting
>>>> for your "correct" fix since your last proposition does _not_
>>>> work.
>>>> 
>>>> Furthermore you guessed something which was wrong since flv
>>>> demuxer could not even return empty packets. How good is this
>>>> ?
>>>> 
>>>> Then I gave you the sample.
>>>> 
>>>>> and submited a broken patch to flvdec thats 2 bad out of 2.
>>>> No patch is _perfectly_ fine, and it actually fixes the
>>>> problem. You just twisted specs to fit your arguments here,
>>>> even Mike acknowledged my argument.
>>> what argument? your patch is broken no matter which way you
>>> define sample_rate
>> No it's not broken, it always set sample for speex to 16khz, this
>> is _right_, no matter how much you twist the spec.
> 
> there are 2 things
> 1. the sample rate of speex
> 2. the patch
> 
>>>> Please stop the FUD.
>>> please accept that you are not and will not be flvdec maintainer
>>> or co maintainer. This decisson is final.
> 
>> Btw your so well FLV demuxer output packet with different codec in
>> the same stream. Do you plan to fix this or will you wait for 6
>> months ?
> 
> i have no plans to fix this, but i certainly will review and comment
> on a patch fixing it.
> 
> 
>> Btw I added H264/AAC/SPEEX demuxing to flv demuxer, I added h264
>> and AAC to flv muxer. This is why Mike forwards me bugs.
> 
> bugs should be entered in roundup i mean you complain that i dont fix
> bugs but i never saw these bugreports. How can i if mike sends them
> to you via private mail?

To be honest, I thought this was trivial enough to not cause so many
problems.
I thought that jumping in, fixing it and submitting a patch was more
efficient.

We all lack motivation, sometimes preparing patches and submitting them
is boring.

I find that comiting and then receiving peer review in -cvslog
is often more convenient and it seems you and Reimar in particular
always do review in -cvslog.

Now it seems that flv demuxer will need some cleanup before getting this
issue fixed. To be honnest, right now, I just don't have the motivation
to do it by submiting patches.

Some time ago, people raised the possibility to be more flexible
concerning maintainership between FFmpeg active official developers.

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