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

Alex Converse alex.converse
Fri Oct 9 22:00:49 CEST 2009


On Fri, Oct 9, 2009 at 3:55 PM, Baptiste Coudurier
<baptiste.coudurier at gmail.com> wrote:
> Hi guys,
>
> On 10/9/09 11:34 AM, Alex Converse wrote:
>>
>> On Fri, Oct 9, 2009 at 2:12 PM, Thilo Borgmann
>> <thilo.borgmann at googlemail.com> ?wrote:
>>>
>>> Alex Converse schrieb:
>>>>
>>>> On Fri, Oct 9, 2009 at 10:30 AM, Thilo Borgmann
>>>> <thilo.borgmann at googlemail.com> ?wrote:
>>>>>
>>>>> Thilo Borgmann schrieb:
>>>>>>
>>>>>> Alex Converse schrieb:
>>>>>>>
>>>>>>> On Fri, Sep 18, 2009 at 6:42 PM, Baptiste Coudurier
>>>>>>> <baptiste.coudurier at gmail.com> ?wrote:
>>>>>>>>
>>>>>>>> Hi,
>>>>>>>>
>>>>>>>> On 09/18/2009 03:35 PM, Thilo Borgmann wrote:
>>>>>>>>>
>>>>>>>>> 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...)
>>>>>>>>>>>
>>>>>>>>>>> [...]
>>>>>>>>>>>
>>>>>>>>>>> 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.
>>>>>>>>
>>>>>>>> Maybe we should always set ->channels in mpeg4audio_get_config, that
>>>>>>>> would
>>>>>>>> simplify the code everywhere else. What do you think ?
>>>>>>>
>>>>>>> In principle that seems fine as long as it doesn't break muxing or
>>>>>>> decoding files with (those awful) PCEs. In practice this probably
>>>>>>> means adding yet more PCE code to mpeg4audio.[ch] since it doesn't
>>>>>>> look like there is any way to adapt ff_copy_pce_data (mpeg4audio.c)
>>>>>>> or
>>>>>>> decode_pce (aac.c) to this purpose.
>>>>>>>
>>>>>>> I would also accept having this as an inevitable goal and leaving it
>>>>>>> on a todo list for a while. This whole multichannel business in MPEG
>>>>>>> 4
>>>>>>> audio was dreadfully thought out but there is nothing we can do about
>>>>>>> it now.
>>>>>>
>>>>>> So........ I should move these "st->codec->channels = ..." into
>>>>>> ff_mpeg4audio_get_config() ?
>>>>>
>>>>> ping
>>>>
>>>> However you want to do it is fine as far as I'm concerned. I don't
>>>> want to make refactoring stuff in AAC a prereq for landing this.
>>>
>>> Thus if altering aac and may be others is a prereq for moving set
>>> ->channels into ff_mpeg4audio_get_config() I might not be able to do
>>> this.
>>>
>>> So I would suggest to apply the patch as is, if the rest is ok.
>>
>> If Rob and Baptiste are ok with it then go ahead and apply.
>
> I was more thinking of always setting cfg.channels, to simplify, maybe make
> mpeg4audio_get_config return -1 in case of error.
>

As I said earlier always setting cfg.channels is going to require some
major changes in aac.



More information about the ffmpeg-devel mailing list