[FFmpeg-devel] [PATCH] split codec_id for ac-3 and e-ac-3

[Justin Ruggles]

Hi,

Aurelien Jacobs wrote:
> On Sun, 31 Aug 2008 23:29:16 -0400
> Justin Ruggles <justin.ruggles at gmail.com> wrote:
> 
>> Ronald S. Bultje wrote:
>>> Hi Justin,
>>>
>>> On Sun, Aug 31, 2008 at 10:24 PM, Justin Ruggles
>>> <justin.ruggles at gmail.com> wrote:
>>>> Here is a patch to make a separate CODEC_ID_EAC3.  It adds a separate
>>>> demuxer, muxer, parser, and decoder for E-AC-3.
>>>>
>>>> After this is done, the various other muxers/demuxers can be modified to
>>>> treat E-AC-3 differently if needed.
>>> Index: libavcodec/ac3_parser.c
>>> ===================================================================
>>> --- libavcodec/ac3_parser.c	(revision 15137)
>>> +++ libavcodec/ac3_parser.c	(working copy)
>>> @@ -194,3 +194,11 @@
>>>      ff_aac_ac3_parse,
>>>      ff_parse_close,
>>>  };
>>> +
>>> +AVCodecParser eac3_parser = {
>>> +    { CODEC_ID_EAC3 },
>>> +    sizeof(AACAC3ParseContext),
>>> +    ac3_parse_init,
>>> +    ff_aac_ac3_parse,
>>> +    ff_parse_close,
>>> +};
>>>
>>> Can't you just add EAC3 in the array of ac3_parser? MP2/3 does that
>>> too, I think. No need for the separate entry.
>>>
>>> [...]
>> new patch attached which shares the parser.
>>
>> [...]
>>
>> ===================================================================
>> --- libavformat/raw.c	(revision 15137)
>> +++ libavformat/raw.c	(working copy)
>> @@ -487,8 +487,8 @@
>>  }
>>  #endif
>>  
>> -#ifdef CONFIG_AC3_DEMUXER
>> -static int ac3_probe(AVProbeData *p)
>> +#if (CONFIG_AC3_DEMUXER || CONFIG_EAC3_DEMUXER)
> 
> With #if you need to use ENABLE_* instead of CONFIG_*:
> +#if (ENABLE_AC3_DEMUXER || ENABLE_EAC3_DEMUXER)

thanks. i'll fix it.

> Also have a look at attached patch. It contains a slighly
> simplified version of your raw.c probe functions.
> 
> Index: libavformat/raw.c
> ===================================================================
> --- libavformat/raw.c	(revision 15121)
> +++ libavformat/raw.c	(working copy)
> @@ -487,10 +487,11 @@
>  }
>  #endif
>  
> -#ifdef CONFIG_AC3_DEMUXER
> -static int ac3_probe(AVProbeData *p)
> +#if (ENABLE_AC3_DEMUXER || ENABLE_EAC3_DEMUXER)
> +static int ac3_eac3_probe(AVProbeData *p, enum CodecID expected_codec_id)
>  {
>      int max_frames, first_frames = 0, frames;
> +    enum CodecID codec_id = CODEC_ID_AC3;
>      uint8_t *buf, *buf2, *end;
>      AC3HeaderInfo hdr;
>      GetBitContext gbc;
> @@ -509,12 +510,15 @@
>              if(buf2 + hdr.frame_size > end ||
>                 av_crc(av_crc_get_table(AV_CRC_16_ANSI), 0, buf2 + 2, hdr.frame_size - 2))
>                  break;
> +            if (hdr.bitstream_id > 10)
> +                codec_id = CODEC_ID_EAC3;
>              buf2 += hdr.frame_size;
>          }
>          max_frames = FFMAX(max_frames, frames);
>          if(buf == p->buf)
>              first_frames = frames;
>      }
> +    if(codec_id != expected_codec_id) return 0;
>      if   (first_frames>=3) return AVPROBE_SCORE_MAX * 3 / 4;
>      else if(max_frames>=3) return AVPROBE_SCORE_MAX / 2;
>      else if(max_frames>=1) return 1;
> @@ -522,6 +526,20 @@
>  }
>  #endif
>  
> +#ifdef CONFIG_AC3_DEMUXER
> +static int ac3_probe(AVProbeData *p)
> +{
> +    return ac3_eac3_probe(p, CODEC_ID_AC3);
> +}
> +#endif
> +
> +#ifdef CONFIG_EAC3_DEMUXER
> +static int eac3_probe(AVProbeData *p)
> +{
> +    return ac3_eac3_probe(p, CODEC_ID_EAC3);
> +}
> +#endif
> +

I do like that better.  I'll change it.

thanks,
Justin