[FFmpeg-devel] [PATCH] check sampling index validity when parsing adts frame header

[Robert Swain]

2009/2/10 Alex Converse <alex.converse at gmail.com>:
> On Tue, Feb 10, 2009 at 8:48 AM, Robert Swain <robert.swain at gmail.com> wrote:
>> 2009/2/9 Robert Swain <robert.swain at gmail.com>:
>>> 2009/2/9 Jai Menon <jmenon86 at gmail.com>:
>>>> On Mon, Feb 9, 2009 at 1:51 PM, Baptiste Coudurier
>>>> <baptiste.coudurier at gmail.com> wrote:
>>>>> Jai Menon wrote:
>>>>>> On Mon, Feb 9, 2009 at 12:41 PM, Alex Converse <alex.converse at gmail.com> wrote:
>>>>>>>> Index: libavcodec/aac.c
>>>>>>>> ===================================================================
>>>>>>>> --- libavcodec/aac.c  (revision 16998)
>>>>>>>> +++ libavcodec/aac.c  (working copy)
>>>>>>>> @@ -1529,6 +1529,11 @@
>>>>>>>>          if (hdr_info.chan_config)
>>>>>>>>              ac->m4ac.chan_config = hdr_info.chan_config;
>>>>>>>>          ac->m4ac.sample_rate     = hdr_info.sample_rate;
>>>>>>>> +
>>>>>>>> +        if(hdr_info.sampling_index > 11) {
>>>>>>> 12 is a valid sampling index. It corresponds to 7350 Hz.
>>>>>>
>>>>>> Shouldn't it be in the table then?
>>>>>
>>>>> Isn't it ? check mpeg4audio.c
>>>>
>>>> the table I referred to is swb_offset_1024 in aacdectab.h
>>>
>>> I'll have to check the spec for this. There are other tables which are
>>> indexed by sampling_index ( swb_offset, ff_aac_num_swb, tns_max_bands,
>>> ff_aac_pred_sfb_max as well as ff_mpeg4audio_sample_rates ). If these
>>> other tables only support up to index 11 in the spec then that's what
>>> we shall have to check. If they should be extended and for whatever
>>> reason they were truncated, we'll extend them.
>>
>> From the spec, it seems that a sample rate of 7350Hz (sampling_index
>> == 12) is not supported in AAC. At least, none of the above mentioned
>> tables contain entries for 7350Hz, only for 8kHz and above. In which
>> case, I consider the checks valid.
>>
>
> "Table 4.68 - Sampling frequency mapping" (1496-3:2005) says that for
> f<9391 the tables for 8000 Hz should be used.

Hmm. Indeed, this may need a bit more work.

Looking at 13818-7, the ADTS headers do not specify a sample rate,
only a sampling_frequency_index. "Table 35 ? Sampling frequency
dependent on sampling_frequency_index" only covers indices mapping
down to 8kHz, the rest of the values are reserved. When using ADTS, it
seems the limitation of indices >11 being 'reserved' (and hence should
cause an error if used) should be imposed.

Looking at 14496-3, libavcodec/mpeg4audio.c gets it right in
get_sample_rate() as it parses the sample rate in the case of the
sampling_frequency_index == 15. However, indices 13 and 14 are still
reserved so we should deal with this.

If sampling_frequency_index == 15 and a sample rate has been
specified, we should map this sample rate according to "Table 4.68 -
Sampling frequency mapping" as you noted. I'm inclined to suggest
reusing the m4ac.sampling_index variable for the result of the mapping
but we should check if this would have any adverse side effects.

Would you agree with all that Alex?

Regards,
Rob