[FFmpeg-devel] [PATCH 1/3] mp3enc: fix Xing sample rate selection.

[Clément Bœsch]

On Thu, May 10, 2012 at 07:06:00PM +0200, Michael Niedermayer wrote:
> On Wed, May 09, 2012 at 10:41:18PM +0200, Clément Bœsch wrote:
> > On Tue, Mar 20, 2012 at 07:22:58PM +0100, Michael Niedermayer wrote:
> > > On Tue, Mar 20, 2012 at 04:38:12PM +0100, Clément Bœsch wrote:
> > > > From: Clément Bœsch <clement.boesch at smartjog.com>
> > > > 
> > > > This at least avoid raising an unsupported sample rate warning when
> > > > sample rate is different than the sampling rate allowed in MPEG-1.
> > > > 
> > > > ex: ffmpeg -f lavfi -i aevalsrc=0 -ar 22050 -y /tmp/out.mp3
> > > > ---
> > > >  libavformat/mp3enc.c |    4 +++-
> > > >  1 files changed, 3 insertions(+), 1 deletions(-)
> > > > 
> > > > diff --git a/libavformat/mp3enc.c b/libavformat/mp3enc.c
> > > > index b2c1b42..d28e95c 100644
> > > > --- a/libavformat/mp3enc.c
> > > > +++ b/libavformat/mp3enc.c
> > > > @@ -126,7 +126,9 @@ static int mp3_write_xing(AVFormatContext *s)
> > > >          return 0;
> > > >  
> > > >      for (i = 0; i < FF_ARRAY_ELEMS(avpriv_mpa_freq_tab); i++)
> > > > -        if (avpriv_mpa_freq_tab[i] == codec->sample_rate) {
> > > > +        if (codec->sample_rate == avpriv_mpa_freq_tab[i]      ||    // MPEG 1
> > > > +            codec->sample_rate == avpriv_mpa_freq_tab[i] >> 1 ||    // MPEG 2 (LSF)
> > > > +            codec->sample_rate == avpriv_mpa_freq_tab[i] >> 2) {    // MPEG 2.5 (LSF)
> > > >              srate_idx = i;
> > > >              break;
> > > 
> > > is the srate_idx correct for all cases ?
> > > 
> > 
> > The sample rate index should be correct (according to
> > http://www.codeproject.com/Articles/8295/MPEG-Audio-Frame-Header#SamplingRate
> > at least).
> > 
> 
> > Though, the audio version was incorrect. The attached patch should be
> > better.
> 
> i hope mixing audio versions has no sideeffects otherwise patch ok
> if it passes your tests
> 

After encoding the same input with different parameters, here are the
reported duration by ffprobe:

    cbr-test-11025.mp3:        duration=312.215510
    noxing-cbr-test-11025.mp3: duration=312.247500

    cbr-test-12000.mp3:        duration=312.192000
    noxing-cbr-test-12000.mp3: duration=312.224000

    cbr-test-16000.mp3:        duration=312.156000
    noxing-cbr-test-16000.mp3: duration=312.168800

    cbr-test-22050.mp3:        duration=312.137143
    noxing-cbr-test-22050.mp3: duration=312.149950

    cbr-test-24000.mp3:        duration=312.120000
    noxing-cbr-test-24000.mp3: duration=312.132800

    cbr-test-32000.mp3:        duration=312.120000
    noxing-cbr-test-32000.mp3: duration=312.130667

    cbr-test-48000.mp3:        duration=312.096000
    noxing-cbr-test-48000.mp3: duration=312.106667

    cbr-test-8000.mp3:         duration=312.264000
    noxing-cbr-test-8000.mp3:  duration=312.296000

    vbr-test-11025.mp3:        duration=312.215510
    noxing-vbr-test-11025.mp3: duration=312.299000

    vbr-test-12000.mp3:        duration=312.192000
    noxing-vbr-test-12000.mp3: duration=338.388571

    vbr-test-16000.mp3:        duration=312.156000
    noxing-vbr-test-16000.mp3: duration=260.934333

    vbr-test-22050.mp3:        duration=312.137143
    noxing-vbr-test-22050.mp3: duration=299.606429

    vbr-test-24000.mp3:        duration=312.120000
    noxing-vbr-test-24000.mp3: duration=319.621143

    vbr-test-32000.mp3:        duration=312.120000
    noxing-vbr-test-32000.mp3: duration=283.206800

    vbr-test-48000.mp3:        duration=312.096000
    noxing-vbr-test-48000.mp3: duration=350.342000

    vbr-test-8000.mp3:         duration=312.264000
    noxing-vbr-test-8000.mp3:  duration=335.153600

vbr files are generated with -aq 3, cbr with -b:a 192k, noxing with a
return 0 added in lavf/mp3enc.c at the beginning of mp3_write_xing()
(Note: I may add a -noxing muxer option at some point).

Playback is OK with all the files, and the duration is almost the same
with xing files so I guess it works.

> 
> > 
> > Any idea how I could check if the rest of the header is correctly set?
> 
> i guess try some things that can read mp3 and see if adding the xing
> header causes a difference in their output
> 

I've pass a mpck on all the files, and the reports seem OK. Though, this
tool doesn't seem to look for a specific Xing header.

If I see no other comment I'll push this in the next days.

-- 
Clément B.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 490 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20120511/85303c33/attachment.asc>