[FFmpeg-devel] [PATCH] code to parse mpeg4audio extradata

[Michael Niedermayer]

Hi

On Mon, Mar 31, 2008 at 11:00:31AM +0200, Baptiste Coudurier wrote:
> Hi,
> 
> Michael Niedermayer wrote:
> > On Mon, Mar 31, 2008 at 12:41:08AM +0200, Baptiste Coudurier wrote:
> >> Hi,
> >>
> >> Michael Niedermayer wrote:
> >>> On Wed, Mar 19, 2008 at 04:54:03PM +0100, Baptiste Coudurier wrote:
> >>>> Hi,
> >>>>
> >>>> Here is a patch that adds parsing of mpeg4audio extradata, and retrieve
> >>>> needed infos for decoders (only mp3on4 atm, but will be needed for aac),
> >>>> and will be used by mov demuxer ideally to correctly compute sample rate
> >>>> and channels.
> >>>> Also extract common mpeg4audio code from aac_parser.c.
> >>> [...]
> >>>
> >>>> Index: libavcodec/mpegaudiodec.c
> >>>> ===================================================================
> >>>> --- libavcodec/mpegaudiodec.c	(revision 12503)
> >>>> +++ libavcodec/mpegaudiodec.c	(working copy)
> >>>> @@ -2486,9 +2486,11 @@
> >>>>  #endif /* CONFIG_MP3ADU_DECODER */
> >>>>  
> >>>>  #ifdef CONFIG_MP3ON4_DECODER
> >>>> +
> >>>> +#include "mpeg4audio.h"
> >>>> +
> >>>>  /* Next 3 arrays are indexed by channel config number (passed via codecdata) */
> >>>> -static int mp3Frames[16] = {0,1,1,2,3,3,4,5,2};   /* number of mp3 decoder instances */
> >>>> -static int mp3Channels[16] = {0,1,2,3,4,5,6,8,4}; /* total output channels */
> >>>> +static const uint8_t mp3_frames[8] = {0,1,1,2,3,3,4,5}; /* number of mp3 decoder instances */
> >>> seperate commit and why does the 2 at the end disapear?
> >>>
> >> Yes, of course. I'll post an updated patch for mp3on4 decoder anyway.
> >>
> >> It disappears because this last configuration is not mentioned anymore
> >> in published specs. It was in old draft, and I expect/hope nobody ever
> >> used it.
> >>
> >>> [...]
> >>>
> >>  > This either needs a function ptr as a argument for parsing codec
> >> specific stuff
> >>> or return some ptr / bitindex to where that is stored. Later is more flexible
> >>> for example with a binary decoder which expects a pointer to this stuff.
> >>>
> >> I like the idea of bitindex. Updated patch attached.
> > 
> > [...]
> >> +    if (c->ext_object_type != 5) {
> >> +        int bits_left = buf_size*8 - specific_config_bitindex;
> >> +        for (; bits_left > 15; bits_left--) {
> >> +            if (show_bits(&gb, 11) == 0x2b7) { // sync extension
> >> +                get_bits(&gb, 11);
> > 
> >> +                if ((c->ext_object_type = get_bits(&gb, 5)) == 31)
> >> +                    c->ext_object_type = 32 + get_bits(&gb, 6);
> > 
> > That code occurs 3 times, maybe it would be more readable in a fuction.
> > The same is true for the sample_rate code below.
> > 
> >> +                if (c->ext_object_type == 5)
> >> +                    if ((c->sbr = get_bits1(&gb)) == 1) {
> >> +                        if ((c->ext_sampling_index = get_bits(&gb, 4)) == 0x0f)
> >> +                            c->ext_sample_rate = get_bits(&gb, 24);
> >> +                        else
> >> +                            c->ext_sample_rate = ff_mpeg4audio_sample_rates[c->ext_sampling_index];
> >> +                    }
> > 
> > [...]
> >> +typedef struct {
> >> +    int object_type;
> >> +    int sampling_index;
> > 
> > Is the *sampling_index really needed? Iam asking because it would complicate
> > a factored out function to read the sample_rate.
> > 
> 
> You're right, I factored out, updated patch attached.

looks ok

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

Let us carefully observe those good qualities wherein our enemies excel us
and endeavor to excel them, by avoiding what is faulty, and imitating what
is excellent in them. -- Plutarch
-------------- 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/20080401/bb8de69d/attachment.pgp>