[FFmpeg-devel] [PATCH] Parsing ALS object type in MPEG-4
Thilo Borgmann
thilo.borgmann
Tue Nov 10 21:10:27 CET 2009
Baptiste Coudurier schrieb:
> On 10/11/2009 03:04 AM, Thilo Borgmann wrote:
>> Baptiste Coudurier schrieb:
>>> On 10/9/09 1:48 PM, Alex Converse wrote:
>>>> On Sun, Aug 23, 2009 at 3:51 PM, Thilo Borgmann
>>>> <thilo.borgmann at googlemail.com> wrote:
>>>>> 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;
>>>>
>>>> Can we put "channels" directly below "chan_config" without API/ABI
>>>> breakage? If so I'd like to try to keep related thing together.
>>>
>>> We could since this struct is not part of the public API, though since
>>> libavformat uses libavcodec, it is not safe.
>>>
>>
>> Patch updated.
>>
>>
>>> This reminds me, I think avcodec_register_all and av_register_all should
>>> put requirements for libavcodec and libavutil versions respectively, to
>>> avoid shared libraries problems, because soname only contains major
>>> version and API changes only update minor version.
>>>
>>
>> [...]
>>
>> Index: libavcodec/mpeg4audio.h
>> ===================================================================
>> --- libavcodec/mpeg4audio.h (revision 20011)
>> +++ libavcodec/mpeg4audio.h (working copy)
>> @@ -31,6 +31,7 @@
>> int sampling_index;
>> int sample_rate;
>> int chan_config;
>> + int channels;
>> int sbr; //< -1 implicit, 1 presence
>> int ext_object_type;
>> int ext_sampling_index;
>
> Forgot to change that ?
Hm, no... looking good locally as well as in the patch I had sent (rev7)
>
>> Index: libavformat/mov.c
>> ===================================================================
>> --- libavformat/mov.c (revision 20011)
>> +++ 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;
>> + }
>
> By always setting cfg.channels I wanted to avoid the
> if (cfg.chan_config) ...
>
> That's ok though.
>
Hm I tried to but cannot really follow the arguing about setting
cfg.channels. Do you mean to always set
st->codec->channels = cfg.channels;
? This would set it to 0 (for PCE streams == !ALS) but don't we loose
something from ff_mpeg4audio_channes[] in that case? And are PCE streams
always given if ALS is not given?
-Thilo
More information about the ffmpeg-devel
mailing list