[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 & 
>>>> FLV_AUDIO_SAMPLERATE_MASK) >> FLV_AUDIO_SAMPLERATE_OFFSET) >>
>>>> 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