[FFmpeg-devel] [PATCh] aac parser: Don't write channels, sample rate, and frame size each frame

[Alex Converse]

On Wed, Mar 3, 2010 at 7:50 AM, Michael Niedermayer <michaelni at gmx.at> wrote:
> On Wed, Mar 03, 2010 at 06:24:38AM -0500, Alex Converse wrote:
>> On Wed, Mar 3, 2010 at 5:12 AM, Michael Niedermayer <michaelni at gmx.at> wrote:
>> > On Wed, Mar 03, 2010 at 05:06:32AM -0500, Alex Converse wrote:
>> >> On Mon, Mar 1, 2010 at 8:23 PM, Michael Niedermayer <michaelni at gmx.at> wrote:
>> >> > On Mon, Mar 01, 2010 at 08:16:48PM -0500, Alex Converse wrote:
>> >> >> They are part of the invariant header and shouldn't change each frame
>> >> >> anyway. This prevents them from overwriting information provided by
>> >> >> backwards compatible extensions.
>> >> >
>> >> > what you attached does not match the description
>> >> >
>> >> > besides "shouldn't change" gives me a strange feeling ...
>> >> > if we can support it changing it would be a nice "wish/feature request"
>> >> >
>> >>
>> >> This patch appears to be necessary for either the put a hack in
>> >> av_find_stream_info() or fix all the demuxers approach to handling
>> >> backwards compatible SBR. It breaks nothing we already support as far
>> >> as I can tell.
>> >>
>> >> Based on that should we move forward with it, make some changes to it,
>> >> or try to move to a new approach?
>> >>
>> >> [patch re-attched]
>> >>
>> >> --Alex
>> >
>> >> ?aac_ac3_parser.c | ? 10 +++++++---
>> >> ?1 file changed, 7 insertions(+), 3 deletions(-)
>> >> 074f9a540038b4836308ca477b806204f56c6229 ?aac_parser_extensions.diff
>> >> diff --git a/libavcodec/aac_ac3_parser.c b/libavcodec/aac_ac3_parser.c
>> >> index 85993c0..23a1d0a 100644
>> >> --- a/libavcodec/aac_ac3_parser.c
>> >> +++ b/libavcodec/aac_ac3_parser.c
>> >> @@ -71,10 +71,12 @@ get_next:
>> >> ? ? ?*poutbuf_size = buf_size;
>> >>
>> >> ? ? ?/* update codec info */
>> >> - ? ?avctx->sample_rate = s->sample_rate;
>> >> ? ? ?if(s->codec_id)
>> >> ? ? ? ? ?avctx->codec_id = s->codec_id;
>> >>
>> >> + ? ?if (avctx->codec_id != CODEC_ID_AAC) {
>> >> + ? ? ? ?avctx->sample_rate = s->sample_rate;
>> >> +
>> >
>> > this one needs a comment, its likely not obvious for the causual reader why
>> > this is not done for aac
>> >
>>
>> fixed
>>
>> >
>> >> ? ? ?/* allow downmixing to stereo (or mono for AC-3) */
>> >> ? ? ?if(avctx->request_channels > 0 &&
>> >> ? ? ? ? ? ? ?avctx->request_channels < s->channels &&
>> >> @@ -83,12 +85,14 @@ get_next:
>> >> ? ? ? ? ? ? ?(avctx->codec_id == CODEC_ID_AC3 ||
>> >> ? ? ? ? ? ? ? avctx->codec_id == CODEC_ID_EAC3)))) {
>> >> ? ? ? ? ?avctx->channels = avctx->request_channels;
>> >> - ? ?} else if (avctx->codec_id != CODEC_ID_AAC || s->channels) {
>> >> + ? ?} else {
>> >> ? ? ? ? ?avctx->channels = s->channels;
>> >> ? ? ? ? ?avctx->channel_layout = s->channel_layout;
>> >> ? ? ?}
>> >> - ? ?avctx->bit_rate = s->bit_rate;
>> >> ? ? ?avctx->frame_size = s->samples;
>> >> + ? ?}
>> >
>> > id guess this could benefit from a comment as well
>> >
>>
>> what do you mean by "this"? bit rate?
>
> i meant the if/else if stuff appears to be underdocumenetd
>
> anyway your patch looks ok
>

Thanks,
Applied