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

[Clément Bœsch]

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.

Any idea how I could check if the rest of the header is correctly set?

-- 
Clément B.
-------------- next part --------------
From db326520b4083ae53348e7c145ecfef713720deb Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Cl=C3=A9ment=20B=C5=93sch?= <ubitux at gmail.com>
Date: Wed, 9 May 2012 22:39:40 +0200
Subject: [PATCH] lavf/mp3enc: support MPEG-2 and MPEG-2.5 sample rates when
 writing Xing header.

---
 libavformat/mp3enc.c |   18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/libavformat/mp3enc.c b/libavformat/mp3enc.c
index 7dec824..8a9f788 100644
--- a/libavformat/mp3enc.c
+++ b/libavformat/mp3enc.c
@@ -121,17 +121,21 @@ static int mp3_write_xing(AVFormatContext *s)
     int64_t          xing_offset;
     int32_t          header, mask;
     MPADecodeHeader  c;
-    int              srate_idx, i, channels;
+    int              srate_idx, ver = 0, i, channels;
     int              needed;
 
     if (!s->pb->seekable)
         return 0;
 
-    for (i = 0; i < FF_ARRAY_ELEMS(avpriv_mpa_freq_tab); i++)
-        if (avpriv_mpa_freq_tab[i] == codec->sample_rate) {
-            srate_idx = i;
-            break;
-        }
+    for (i = 0; i < FF_ARRAY_ELEMS(avpriv_mpa_freq_tab); i++) {
+        const uint16_t base_freq = avpriv_mpa_freq_tab[i];
+        if      (codec->sample_rate == base_freq)     ver = 0x3; // MPEG 1
+        else if (codec->sample_rate == base_freq / 2) ver = 0x2; // MPEG 2
+        else if (codec->sample_rate == base_freq / 4) ver = 0x0; // MPEG 2.5
+        else continue;
+        srate_idx = i;
+        break;
+    }
     if (i == FF_ARRAY_ELEMS(avpriv_mpa_freq_tab)) {
         av_log(s, AV_LOG_WARNING, "Unsupported sample rate, not writing Xing header.\n");
         return -1;
@@ -145,7 +149,7 @@ static int mp3_write_xing(AVFormatContext *s)
 
     /* dummy MPEG audio header */
     header  =  0xff                                  << 24; // sync
-    header |= (0x7 << 5 | 0x3 << 3 | 0x1 << 1 | 0x1) << 16; // sync/mpeg-1/layer 3/no crc*/
+    header |= (0x7 << 5 | ver << 3 | 0x1 << 1 | 0x1) << 16; // sync/audio-version/layer 3/no crc*/
     header |= (srate_idx << 2) <<  8;
     header |= channels << 6;
 
-- 
1.7.10.1

-------------- 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/20120509/c8ce24ac/attachment.asc>