[FFmpeg-devel] [PATCH] fix speex sample

[Baptiste Coudurier]

On 4/8/2009 2:28 PM, Michael Niedermayer wrote:
> On Wed, Apr 08, 2009 at 01:22:21PM -0700, Baptiste Coudurier wrote:
>> On 4/8/2009 1:09 PM, Michael Niedermayer wrote:
>>> On Wed, Apr 08, 2009 at 10:51:02AM -0700, Baptiste Coudurier wrote:
>>>> Hi Michael,
>>>>
>>>> On 4/2/2009 8:46 PM, Michael Niedermayer wrote:
>>>>> On Thu, Apr 02, 2009 at 08:08:41PM -0700, Baptiste Coudurier wrote:
>>>>>> On 4/2/2009 7:11 PM, Michael Niedermayer wrote:
>>>>>>> On Thu, Apr 02, 2009 at 06:09:15PM -0700, Baptiste Coudurier wrote:
>>>>>>>> Michael Niedermayer wrote:
>>>>>>> [...]
>>>>>>>
>>>>>>>>>> Then I guess you noticed how many bugs are rotting in roundup ? This is
>>>>>>>>>> becoming a nightmare and is really frightening. Nobody jumps in, and
>>>>>>>>>> this was proved by the last 2 attempts to organize a bug fixing weekend.
>>>>>>>>> i know and agree, you also remember i did occassionally in the past fix a
>>>>>>>>> bunch of bugs but ATM i lack the motivation, i know thats my problem and
>>>>>>>>> i should go and bang my head against the wall ;)
>>>>>>>> Well, yes, but it's not only your problem, I believe it is our problem,
>>>>>>>> and it would be nice if everybody could participate in the effort.
>>>>>>> fine but i have to check this with the insurance first, if they cover damage
>>>>>>> from a bunch of madman banging their heads against the walls
>>>>>>>
>>>>>>>
>>>>>>>> I was motivated yesterday night.
>>>>>>>>
>>>>>>>>>> Now I do care and I proved it, so I claim that yes some changes my
>>>>>>>>>> introduce regression but these will be fixed quickly, I engage myself.
>>>>>>>>> i think the proposed fix is wrong and a correct one will not introduce a
>>>>>>>>> regression
>>>>>>>> Well, I still don't really see how and why, I've explained more below.
>>>>>>> iam not talking about "SoundRate UB[2]" but that:
>>>>>>>             else if(!strcmp(key, "audiosamplerate") && acodec && num_val >= 0) {
>>>>>>>                 //some tools, like FLVTool2, write consistently approximate metadata sample rates
>>>>>>>                 if (!acodec->sample_rate) {
>>>>>>>                     switch((int)num_val) {
>>>>>>>                         case 44000: acodec->sample_rate = 44100  ; break;
>>>>>>>                         case 22000: acodec->sample_rate = 22050  ; break;
>>>>>>>                         case 11000: acodec->sample_rate = 11025  ; break;
>>>>>>>                         case 5000 : acodec->sample_rate = 5512   ; break;
>>>>>>>                         default   : acodec->sample_rate = num_val;
>>>>>> Yes, however I'm sorry, I don't see how this relates to the patch.
>>>>>>
>>>>>> The patch makes the demuxer always set speex and nellymoser to their
>>>>>> correct _codec_ value IMHO, this "audiosamplerate" is a metadata thing
>>>>>> according to the specs, defined with onMetadata mechanism.
>>>>> I agree but then there would be no point to even decode audiosamplerate
>>>>> and i suspect there was some sense in this, i just dont remember it now
>>>>>
>>>> Any update on this ? It would be nice to fix bugs ;)
>>> no update, iam still waiting for someone to post a corrected patch
>>>
>>> The samplerate as set a few lines above is incorrect, i suspect fixing that
>>> will fix the problem
>>> also a function with the name "flv_set_audio_codec" has no business setting
>>> the samplerate, this should be split out and run seperately when needed
>>>
>> Renaming the function to "set_audio_codec_params" is ok, codec_id is a
>> codec parameter.
> 
> thats the wrong way around, its not the name that is bad ;)
> 
> 
>> I guess we differ on the idea of "maintainership",
> 
> probably not, you just seem to expect me to fix the bug that itches you.

See below, this does not itches me. I'm just devoted and kind enough to
try to fix this issue, because I think with have too many bugs currently
and something should be done.

> ive already said that i dont have the time to fix all bugs in all code
> and rewrite all patches that dont pass review.

I agree with you. However maintainers should have some kind of
commitment IMHO.

> I can fix some and i will certainly not concentrate on ones that other
> developers have a lot of interrest in and are capable to fix
> because such bugs alraedy have people who will eventually fix them

So this means basically I can stop concentrate on _any_ bug since you
are capable of fixing them ;)

> if someone submits a patch, the maintainer reviews it
> its the same with patches from you to code maintained by me as it is
> with patches by me to code maintained by you or mans.
> IIRC you didnt fix every issue reimar pointed out in mp4/mov either.

Which ones ? AFAIK all isssues has been adressed except maybe the
complete edit list support which is non trivial at _all_, but I applied
somehow acceptable fix myself _based_ on Reimar patch. I just did not
refuse it, and I based my efforts on his work, assuming my maintainer role.

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.

>> but that's all right
>> with me. Let's wait a few months ;)
> 
> you dont need to wait a few month before you fix your patch ;)

Well, like you said pretty well, I can spend my time on something else
than something maintainer is able to fix ;)
I don't have problem with this file personally, someone submited this
file. I just don't like bugs.

If I were maintainer, I would have fixed it already, that's the whole
point. I would even have addressed your comments on -cvslog and changed
my original commit ;)

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