[FFmpeg-devel] [PATCH] fix speex sample

Baptiste Coudurier baptiste.coudurier
Fri Apr 3 01:02:58 CEST 2009

Michael Niedermayer wrote:
> On Thu, Apr 02, 2009 at 02:45:25PM -0700, Baptiste Coudurier wrote:
>> Michael Niedermayer wrote:
>>> On Thu, Apr 02, 2009 at 12:17:18PM -0700, Baptiste Coudurier
>>> wrote:
>>>> Hi
>>>> Sample testingSpeex.flv Problem is that the code above is
>>>> setting sample rate according to the value stored, but for
>>>> speex and nellymoser 8lhz sample rate is fixed and the values
>>>> stored is 0.
>>>> One possibility is the to always call set_audio_codec according
>>>> to what is stored in the packet, the function will correctly
>>>> set sample rate for speex and nellymoser 8khz. We could also
>>>> special case these 2 codecs.
>>>> I checked when the "if" above was added, and it seems that was
>>>> for K70707-ARIA229.flv. I checked that with my patch, file is
>>>> still detected correctly.
>>>> Btw Michael, if you want, I volunteer to maintain FLV code if
>>>> it can helps, to do cleanup, simplification, and fix bugs.
>>> i would prefer to review all changes that might change behavior
>>> of flvdec
>> It's up to you, but you are also the first person in charge of
>> fixing bugs, as the flv maintainer.
> The first job of the maintainer is to prevent more bugs from being
> added.

Oh yeah, then you just refuse any change, so you are safe :)
Very productive method, indeed ;)

> And the second is to deal with regressions like what is caused by the
>  last commit to flvdec.c

I don't think it introduced a regression. I'd like you to explain
though, I _will_ fix it.

In any case you are free to revert and find a better way to _fix_ the
bug, but that means actually caring about it, coding, and committing ;)

> If you want to fix bugs, that is very welcome, if you just want to
> flame me into steping back as flvdec maintainer so you can exchange
> some bugs against others i will not do so.

No I offered my help in maintaining aka fixing bugs _and_ regressions
introduced, cleaning the code (it really needs it).

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.

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.

At the end I hope to get a clean, stable and maintained piece of code.

>>> [...]
>>>> Index: libavformat/flvdec.c 
>>>> ===================================================================
>>>>  --- libavformat/flvdec.c	(revision 18316) +++
>>>> libavformat/flvdec.c (working copy) @@ -399,9 +399,7 @@ 
>>>> st->codec->sample_rate = (44100 << ((flags & 
>>>> 3);
>>> the bug is in this line, this is clearly not correct for
>>> nelly&speex
>>> your proposed change will break flv files that really use a non 
>>> standard samplerate. (no i have no such file, its just
>>> hypothetical, but IMHO if the file says samplerate=X and X!= 0
>>> then this should be used)
>> Point is to override sample for nelly 8khz and speex, this code is 
>> already present in set_audio_codec.
>> If you want to duplicate this code before this line, feel free to
>> do so.
>> There is no case where storing a wrong sample would be necessary
>> nor supported. Specifications clearly states so.
> id like to support more than the spec where its natural to do so. iam
> not interrested in knowingly breaking other samplerates.

Thanks for helping and supporting the file format nightmare, I know
several people here will really thank you for making their life horrible.

Why don't you support putting fourcc in TS through registration
descriptor, go on.

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

More information about the ffmpeg-devel mailing list