[FFmpeg-devel] [PATCH] avfilter/mpegaudiodec_template: Fix wrong decision of illegal

[Ted Lee]

Dear FFmpeg developers,

I'm glad to have a chance to contribute to FFmpeg.

Since this is the first time for me, please give feedback if I missed
something. I will reflect on that.

*Summary:*

   - Test signal: 790_pomnyun_taebaek2.mp3
      - MP3 file with the same left channel and right channel except for
      the first 7 seconds and the last 10 seconds.
      (download:
      <https://www.dropbox.com/s/sivz18sixvhoo66/790_pomnyun_taebaek2.mp3?dl=0>
      https://www.dropbox.com/s/sivz18sixvhoo66/790_pomnyun_taebaek2.mp3?dl=0
      )
   - When decoding the test signal using FFmpeg as below, a difference
   occurs in the same part of L and R.
      - $ffmpeg -i 790_pomnyun_taebaek2.mp3 -acodec pcm_s16le
      790_pomnyun_taebaek2.wav


Expected result (Adobe Audition or Core Audio):

Decoded PCM
[image: image.png]
(L-R)
[image: image.png]
(You can reproduce it using Adobe Audition or CoreAudio of MacOS)

Actual result (FFmpeg):

Decoded PCM
[image: image.png]

(L-R)
[image: image.png]

*Why it happens?:*

   - In MPEG-1 Layer 3 (MP3), the illegal position of intensity stereo is
   defined as the maximum value of scale factor. Since intensity stereo
   positions (scale factor of the right channel) are always allocated to 3
   bits, intensity stereo positions are limited from 0 to 6 and 7 is treated
   as the illegal position.
   - In MPEG-2 LSF, the illegal position is also defined as the maximum
   value of scale factor, but the number of bits of the scale factor is not
   fixed. So the number of bits needs to be calculated (slen[]) and the
   illegal stereo position should be changed as the maximum value. However, in
   the current implementation, the illegal position is defined as fixed value
   16.

*How to fix this?:*

   - The maximum value of scale factor (the illegal position) should be
   store to sf_max_table[] and an illegal position is determined when the
   scale factor is sf_max_table[](sf_max).


>From 754545f6c675aede07abd912992f9ed4a17d4ddc Mon Sep 17 00:00:00 2001
From: Ted <t at gaudiolab.com>
Date: Wed, 8 Jan 2020 11:53:56 +0900
Subject: [PATCH] avfilter/mpegaudiodec_template: Fix wrong decision of
illegal
 intensity stereo position for MPEG-2 LSF

Signed-off-by: Ted <t at gaudiolab.com>
---
 libavcodec/mpegaudiodec_template.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/libavcodec/mpegaudiodec_template.c
b/libavcodec/mpegaudiodec_template.c
index 3f1674e827..d6d7cd5ad0 100644
--- a/libavcodec/mpegaudiodec_template.c
+++ b/libavcodec/mpegaudiodec_template.c
@@ -115,6 +115,7 @@ static uint16_t band_index_long[9][23];
 static INTFLOAT is_table[2][16];
 static INTFLOAT is_table_lsf[2][2][16];
 static INTFLOAT csa_table[8][4];
+static uint8_t sf_max_table[40];

 static int16_t division_tab3[1<<6 ];
 static int16_t division_tab5[1<<8 ];
@@ -1050,7 +1051,6 @@ static void compute_stereo(MPADecodeContext *s,
GranuleDef *g0, GranuleDef *g1)
             sf_max = 7;
         } else {
             is_tab = is_table_lsf[g1->scalefac_compress & 1];
-            sf_max = 16;
         }

         tab0 = g0->sb_hybrid + 576;
@@ -1077,6 +1077,8 @@ static void compute_stereo(MPADecodeContext *s,
GranuleDef *g0, GranuleDef *g1)
                         }
                     }
                     sf = g1->scale_factors[k + l];
+                    if (s->lsf)
+                        sf_max = sf_max_table[k + 1];
                     if (sf >= sf_max)
                         goto found1;

@@ -1122,6 +1124,8 @@ found1:
                 /* for last band, use previous scale factor */
                 k  = (i == 21) ? 20 : i;
                 sf = g1->scale_factors[k];
+                if (s->lsf)
+                    sf_max = sf_max_table[k + 1];
                 if (sf >= sf_max)
                     goto found2;
                 v1 = is_tab[0][sf];
@@ -1523,11 +1527,15 @@ static int mp_decode_layer3(MPADecodeContext *s)
                     n  = lsf_nsf_table[tindex2][tindex][k];
                     sl = slen[k];
                     if (sl) {
-                        for (i = 0; i < n; i++)
+                        for (i = 0; i < n; i++) {
+                            sf_max_table[j] = (uint8_t) exp2(sl) - 1;
                             g->scale_factors[j++] = get_bits(&s->gb, sl);
+                        }
                     } else {
-                        for (i = 0; i < n; i++)
+                        for (i = 0; i < n; i++) {
+                            sf_max_table[j] = 0;
                             g->scale_factors[j++] = 0;
+                        }
                     }
                 }
                 /* XXX: should compute exact size */
-- 
2.17.0
-------------- next part --------------
A non-text attachment was scrubbed...
Name: image.png
Type: image/png
Size: 239287 bytes
Desc: not available
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20200108/46ae93c8/attachment.png>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: image.png
Type: image/png
Size: 37000 bytes
Desc: not available
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20200108/46ae93c8/attachment-0001.png>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: image.png
Type: image/png
Size: 259970 bytes
Desc: not available
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20200108/46ae93c8/attachment-0002.png>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: image.png
Type: image/png
Size: 50129 bytes
Desc: not available
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20200108/46ae93c8/attachment-0003.png>