[FFmpeg-devel] [PATCH] Parsing ALS object type in MPEG-4

[Thilo Borgmann]

Alex Converse schrieb:
> On Sun, Aug 23, 2009 at 3:51 PM, Thilo
> Borgmann<thilo.borgmann at googlemail.com> wrote:
>> Revision 6 attached (rev. 5 skipped...)
>>
>> Regards,
>> Thilo
>>
>> Index: libavcodec/mpeg4audio.c
>> ===================================================================
>> --- libavcodec/mpeg4audio.c     (revision 19689)
>> +++ libavcodec/mpeg4audio.c     (working copy)
>> @@ -24,6 +24,34 @@
>>  #include "put_bits.h"
>>  #include "mpeg4audio.h"
>>
>> +/**
>> + * Parse MPEG-4 audio configuration for ALS object type.
>> + * @param[in] gb       bit reader context
>> + * @param[in] c        MPEG4AudioConfig structure to fill
>> + * @return on success 0 is returned, otherwise a value < 0
>> + */
>> +static int parse_config_ALS(GetBitContext *gb, MPEG4AudioConfig *c)
>> +{
>> +    if (gb->size_in_bits - get_bits_count(gb) < 112)
>> +        return -1;
>> +
>> +    if (get_bits_long(gb, 32) != MKBETAG('A','L','S','\0'))
>> +        return -1;
>> +
>> +    // override AudioSpecificConfig channel configuration and sample rate
>> +    // which are buggy in old ALS conformance files
>> +    c->sample_rate = get_bits_long(gb, 32);
>> +
>> +    // skip number of samples
>> +    skip_bits_long(gb, 32);
>> +
>> +    // read number of channels
>> +    c->chan_config = 0;
>> +    c->channels    = get_bits(gb, 16) + 1;
>> +
>> +    return 0;
>> +}
>> +
>>  const int ff_mpeg4audio_sample_rates[16] = {
>>     96000, 88200, 64000, 48000, 44100, 32000,
>>     24000, 22050, 16000, 12000, 11025, 8000, 7350
>> @@ -71,8 +99,19 @@
>>     }
>>     specific_config_bitindex = get_bits_count(&gb);
>>
>> +    if (c->object_type == AOT_ALS) {
>> +        skip_bits(&gb, 5);
>> +        if (show_bits_long(&gb, 24) != MKBETAG('\0','A','L','S'))
>> +            skip_bits_long(&gb, 24);
>> +
>> +        specific_config_bitindex = get_bits_count(&gb);
>> +
>> +        if (parse_config_ALS(&gb, c))
>> +            return -1;
>> +    }
>> +
> 
> This is inconsistent with the way other AOTs are handled. Particularly
> AAC with a PCE based configuration has the same channel count issues.
> Perhaps dispatching the AOT specific config here for every AOT would
> be the smart thing to do. I also feel like I'm having deja-vu about
> this conversation. Did we make some sort of decision about this
> before?

I was not involved in that, if that was the case. Anyone?


> 
>>     if (c->ext_object_type != AOT_SBR) {
>> -        int bits_left = buf_size*8 - specific_config_bitindex;
>> +        int bits_left = buf_size*8 - get_bits_count(&gb);
>>         for (; bits_left > 15; bits_left--) {
>>             if (show_bits(&gb, 11) == 0x2b7) { // sync extension
>>                 get_bits(&gb, 11);
>> Index: libavcodec/mpeg4audio.h
>> ===================================================================
>> --- libavcodec/mpeg4audio.h     (revision 19689)
>> +++ libavcodec/mpeg4audio.h     (working copy)
>> @@ -36,6 +36,7 @@
>>     int ext_sampling_index;
>>     int ext_sample_rate;
>>     int ext_chan_config;
>> +    int channels;
>>  } MPEG4AudioConfig;
>>
>>  extern const int ff_mpeg4audio_sample_rates[16];
>> Index: libavformat/mov.c
>> ===================================================================
>> --- libavformat/mov.c   (revision 19689)
>> +++ libavformat/mov.c   (working copy)
>> @@ -434,9 +434,13 @@
>>                 MPEG4AudioConfig cfg;
>>                 ff_mpeg4audio_get_config(&cfg, st->codec->extradata,
>>                                          st->codec->extradata_size);
>> +                if (cfg.chan_config) {
>>                 if (cfg.chan_config > 7)
>>                     return -1;
>>                 st->codec->channels = ff_mpeg4audio_channels[cfg.chan_config];
>> +                } else {
>> +                    st->codec->channels = cfg.channels;
>> +                }
>>                 if (cfg.object_type == 29 && cfg.sampling_index < 3) // old mp3on4
>>                     st->codec->sample_rate = ff_mpa_freq_tab[cfg.sampling_index];
>>                 else
> 
> The rest of this seems OK but Rob and Baptiste are the maintainers here.

Ping.

-Thilo