[FFmpeg-devel] [PATCH] mp3on4 heavy cleanup

Michael Niedermayer michaelni
Fri Apr 25 19:30:19 CEST 2008


On Fri, Apr 25, 2008 at 04:56:58PM +0200, Baptiste Coudurier wrote:
> Hi,
> 
> Several patches. I bet Michael will wonder how this got into svn ;)
> 
> I tried to split it as much as I could and until I got too lazy.
> This update will handle mp3on4 iso reference files publicly available.
> When applying I'll be sure to split commits very carefully.
> This standard defines 8khz mp3 which we were thinking about a long time ago.
> 
> mp3on4_dec.patch is the full diff I have on my tree.
> 
> Thanks for the review.
> 
> -- 
> Baptiste COUDURIER                              GnuPG Key Id: 0x5C1ABAAA
> SMARTJOG SAS                                     http://www.smartjog.com
> Key fingerprint                 8D77134D20CC9220201FC5DB0AC9325C5C1ABAAA
> Phone: +33 1 49966312

> Index: libavcodec/mpegaudiodec.c
> ===================================================================
> --- libavcodec/mpegaudiodec.c	(revision 12960)
> +++ libavcodec/mpegaudiodec.c	(working copy)
> @@ -2581,13 +2581,10 @@
>  
>      for (fr = 0; fr < s->frames; fr++) {
>          start = start2;
> -        fsize = (start[0] << 4) | (start[1] >> 4);
> +        fsize = AV_RB16(start[0]) >> 4;

typo as already pointed out, the rest of this patch is ok

> +        fsize = FFMIN(fsize, FFMIN(len, MPA_MAX_CODED_FRAME_SIZE));
>          start2 += fsize;
> -        if (fsize > len)
> -            fsize = len;
>          len -= fsize;
> -        if (fsize > MPA_MAX_CODED_FRAME_SIZE)
> -            fsize = MPA_MAX_CODED_FRAME_SIZE;
>          m = s->mp3decctx[fr];
>          assert (m != NULL);
>  

> Index: libavcodec/mpegaudiodec.c
> ===================================================================
> --- libavcodec/mpegaudiodec.c	(revision 12960)
> +++ libavcodec/mpegaudiodec.c	(working copy)
> @@ -62,6 +62,7 @@
>  typedef struct MP3On4DecodeContext {
>      int frames;   ///< number of mp3 frames per block (number of mp3 decoder instances)
>      int chan_cfg; ///< channel config number
> +    int syncword; ///< syncword patch
>      MPADecodeContext *mp3decctx[5]; ///< MPADecodeContext for every decoder instance
>  } MP3On4DecodeContext;
>  
> @@ -2475,11 +2476,13 @@
>  #endif /* CONFIG_MP3ADU_DECODER */
>  
>  #ifdef CONFIG_MP3ON4_DECODER
> +
> +#include "mpeg4audio.h"
> +
>  /* Next 3 arrays are indexed by channel config number (passed via codecdata) */
> -static int mp3Frames[16] = {0,1,1,2,3,3,4,5,2};   /* number of mp3 decoder instances */
> -static int mp3Channels[16] = {0,1,2,3,4,5,6,8,4}; /* total output channels */
> +static const uint8_t mp3_frames[8] = {0,1,1,2,3,3,4,5}; /* number of mp3 decoder instances */
>  /* offsets into output buffer, assume output order is FL FR BL BR C LFE */
> -static int chan_offset[9][5] = {
> +static const uint8_t chan_offset[9][5] = {
>      {0},
>      {0},            // C
>      {0},            // FLR
> @@ -2495,6 +2498,7 @@
>  static int decode_init_mp3on4(AVCodecContext * avctx)
>  {
>      MP3On4DecodeContext *s = avctx->priv_data;
> +    MPEG4AudioConfig cfg;
>      int i;
>  
>      if ((avctx->extradata_size < 2) || (avctx->extradata == NULL)) {
> @@ -2502,14 +2506,22 @@
>          return -1;
>      }
>  
> -    s->chan_cfg = (((unsigned char *)avctx->extradata)[1] >> 3) & 0x0f;
> -    s->frames = mp3Frames[s->chan_cfg];
> -    if(!s->frames) {
> +    ff_mpeg4audio_get_config(&cfg, avctx->extradata, avctx->extradata_size);
> +    if (!cfg.chan_config || cfg.chan_config > 7) {
>          av_log(avctx, AV_LOG_ERROR, "Invalid channel config number.\n");
>          return -1;
>      }
> -    avctx->channels = mp3Channels[s->chan_cfg];
> +    avctx->channels = ff_mpeg4audio_channels[cfg.chan_config];
>  
> +    s->frames = mp3_frames[cfg.chan_config];
> +    s->syncword = 0xfff00000;
> +    if (cfg.object_type == 29 && cfg.chan_config < 3) // old mp3on4
> +        avctx->sample_rate = ff_mpa_freq_tab[cfg.chan_config];
> +    else {
> +        avctx->sample_rate = cfg.sample_rate;
> +        if (cfg.sample_rate < 16000)
> +            s->syncword = 0xffe00000;
> +    }
>      /* Init the first mp3 decoder in standard way, so that all tables get builded
>       * We replace avctx->priv_data with the context of the first decoder so that
>       * decode_init() does not have to be changed.

ok


> Index: libavcodec/mpegaudiodec.c
> ===================================================================
> --- libavcodec/mpegaudiodec.c	(revision 12960)
> +++ libavcodec/mpegaudiodec.c	(working copy)
> @@ -2600,11 +2600,10 @@
>          }
>  
>          ff_mpegaudio_decode_header(m, header);
> -        mp_decode_frame(m, decoded_buf, start, fsize);
> +        out_size += mp_decode_frame(m, decoded_buf, start, fsize);
>  
> -        n = MPA_FRAME_SIZE * m->nb_channels;
> -        out_size += n * sizeof(OUT_INT);
>          if(s->frames > 1) {
> +            n = m->avctx->frame_size*m->nb_channels;
>              /* interleave output data */
>              bp = out_samples + coff[fr];
>              if(m->nb_channels == 1) {

ok


> Index: libavcodec/mpegaudiodec.c
> ===================================================================
> --- libavcodec/mpegaudiodec.c	(revision 12960)
> +++ libavcodec/mpegaudiodec.c	(working copy)
> @@ -2570,11 +2570,10 @@
>  
>      len = buf_size;
>  
> +    *data_size = 0;
>      // Discard too short frames
> -    if (buf_size < HEADER_SIZE) {
> -        *data_size = 0;
> -        return buf_size;
> -    }
> +    if (buf_size < HEADER_SIZE)
> +        return -1;
>  
>      // If only one decoder interleave is not needed
>      outptr = s->frames == 1 ? out_samples : decoded_buf;

ok


> @@ -2594,10 +2593,8 @@
>          // Get header
>          header = AV_RB32(start) | 0xfff00000;
>  
> -        if (ff_mpa_check_header(header) < 0) { // Bad header, discard block
> -            *data_size = 0;
> -            return buf_size;
> -        }
> +        if (ff_mpa_check_header(header) < 0) // Bad header, discard block
> +            return -1
>  
>          ff_mpegaudio_decode_header(m, header);
>          mp_decode_frame(m, decoded_buf, start, fsize);

This is incorrect, as some frames could have been decoded sucessfully
i think and this would discard them.

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

Opposition brings concord. Out of discord comes the fairest harmony.
-- Heraclitus
-------------- 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/20080425/837f7a8c/attachment.pgp>



More information about the ffmpeg-devel mailing list