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

[Robert Swain]

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.

However, while parse_adts_frame_header() is tied to AAC at the moment,
maybe in future we'll support the CELP part of the spec too. As such
I'm moving the check to just after parse_adts_frame_header() is called
as it isn't the value that is wrong in the header, it's that we don't
support the formats in the spec that require that sampling_index.

Thanks for the patch. Applied.

Regards,
Rob