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

[Alex Converse]

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?

> ? ? 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.