[FFmpeg-devel] [PATCH] fix speex sample

[Michael Niedermayer]

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.
And the second is to deal with regressions like what is caused by the
last commit to flvdec.c

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.


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

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

No great genius has ever existed without some touch of madness. -- Aristotle
-------------- 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/20090403/7ce4e1fb/attachment.pgp>