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

[Michael Niedermayer]

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

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

I wish the Xiph folks would stop pretending they've got something they
do not.  Somehow I fear this will remain a wish. -- M?ns Rullg?rd
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20100303/b9c0c4f5/attachment.pgp>