[FFmpeg-devel] [PATCH] als in mp4

[Jai Menon]

On 4/9/09, Baptiste Coudurier <baptiste.coudurier at gmail.com> wrote:
> Hi Jai,
>
>
>  On 4/9/2009 12:06 AM, Jai Menon wrote:
>  > On Wed, Apr 8, 2009 at 10:38 AM, Baptiste Coudurier
>  > <baptiste.coudurier at gmail.com> wrote:
>  >> Hi Jai,
>  >>
>  >> On 4/6/2009 5:12 AM, Jai Menon wrote:
>  >>> On Fri, Apr 3, 2009 at 11:28 PM, Baptiste Coudurier
>  >>>
>  >>> [...]
>  >>>
>  >>> diff --git a/libavcodec/mpeg4audio.c b/libavcodec/mpeg4audio.c
>  >>> index 9c7e02b..b391609 100644
>  >>> --- a/libavcodec/mpeg4audio.c
>  >>> +++ b/libavcodec/mpeg4audio.c
>  >>> @@ -55,6 +55,8 @@ int ff_mpeg4audio_get_config(MPEG4AudioConfig *c, const uint8_t *buf, int buf_si
>  >>>      c->object_type = get_object_type(&gb);
>  >>>      c->sample_rate = get_sample_rate(&gb, &c->sampling_index);
>  >>>      c->chan_config = get_bits(&gb, 4);
>  >>> +    c->absolute_channels = 0;
>  >>> +    c->bits_per_sample   = 0;
>  >>>      c->sbr = -1;
>  >>>      if (c->object_type == 5) {
>  >>>          c->ext_object_type = c->object_type;
>  >>> @@ -80,5 +82,11 @@ int ff_mpeg4audio_get_config(MPEG4AudioConfig *c, const uint8_t *buf, int buf_si
>  >>>                  get_bits1(&gb); // skip 1 bit
>  >>>          }
>  >>>      }
>  >>> +    if (c->object_type == AOT_ALS) {
>  >>> +        c->absolute_channels = 1;
>  >>> +        c->sample_rate       = AV_RB32(buf + 10);
>  >>> +        c->chan_config       = AV_RB16(buf + 18) + 1;
>  >>> +        c->bits_per_sample   = (((buf[20] >> 2) & 0x07) + 1) << 3;
>  >>> +    }
>  >> There is something I don't understand according to the code above,
>  >> position might not be fixed depending on several conditions.
>  >>
>  >> How can you read at fixed position in "buf" ?
>  >> It would depends on what has been read.
>  >
>  > very sorry, i didn't quite get what you mean. extradata is the
>  > audiospecificconfig object and the code parses required values from
>  > the alspecificconfig, after skipping an additional 5 bits of padding
>  > (Table 1.15 ? Syntax of AudioSpecificConfig - 14496-3 sp01).
>
>
> Well, first you need to ensure buf size is > 20.
>  Let me try to explain better:
>
>  Code will read more bits depending on some values, for example
>  get_sample_rate, if escape is used, will read 24 bits more.
>
>  I wonder why then, the sample rate value for ALS would be at fixed
>  buf[10] position since it will depends on wether escape is used or not.

I realize that, the patch was written this way because at this point,
only the reference code can generate ALS streams :). But yes, you are
correct.

>  >> Futhermore, I don't get why ALS does not use correctly sample rate like
>  >> others AOT. Can you please explain/quote specs ?
>  >
>  > i could remove the samplerate and bits_per_coded_sample part. however,
>  > setting no. of channels this way is correct.
>  > Table 1.19 ? Channel Configuration - 14496-3 sp01
>  > 0     -       -       defined in AOT related SpecificConfig
>  > So i think that part of the hunk is correct.
>
>
> I don't have latest specs unfortunately, but if specs says that sample
>  rate is set in ALSSpecific config, then it's fine. Though I wonder why
>  since sample rate can be anything fitting in 24 bits using escape...

Actually, the spec says this only for channel configuration and not
for samplerate. The reference code does indeed escape the value and
write absolute samplerate. I will remove that part in the next patch.

>  Are the new fields really necessary for now ? They could be added later.

So how should we do this in your opinion? There should be some way we
can instruct the demuxer that chan_config is an absolute value and not
an index. The patch does this using the absolute_channels flag. As for
the bits_per_sample field, I will remove it in the next patch.

[...]

-- 
Regards,

Jai