[Ffmpeg-devel] [PATCH] Correctly parse headers of E-AC3 streams

[Ian Caulfield]

On 1/18/07, Michael Niedermayer <michaelni at gmx.at> wrote:
>
> > @@ -635,6 +635,11 @@
> >  static const int ac3_channels[8] = {
> >      2, 1, 2, 3, 3, 4, 4, 5
> >  };
> > +
> > +static const int eac3_blocks[4] = {
> > +    1, 2, 3, 6
> > +};
>
> a uint8_t table would safe 12 bytes ...


Wasn't really thinking about that, just copied the type from the earlier
changes. Those could probably be changed too.

> +        if (fscod == 3) {
> > +            fscod2 = get_bits(&bits, 2);
> > +            numblkscod = 3;
> > +
> > +            if(!ac3_sample_rates[fscod2])
> > +                return 0;
>
> why not if(fscod2 == 3) like above?


Copied that if from the original code.

> +
> > +            *sample_rate = ac3_sample_rates[fscod2] / 2;
> > +        } else {
> > +            numblkscod = get_bits(&bits, 2);
> > +
> > +            *sample_rate = ac3_sample_rates[fscod];
> > +        }
>
> what about: ? (no iam not sure if its better ...)
>
> fscod = get_bits(&bits, 2);
> numblkscod = get_bits(&bits, 2);
> if(fscod == 3){
>    if(numblkscod == 3)
>        return 0;
>
>    *sample_rate = ac3_sample_rates[numblkscod] / 2;
>    numblkscod = 3;
> }else
>    *sample_rate = ac3_sample_rates[fscod];


Possibly less readable - I basically just translated the syntax spec
directly into code.

> +
> > +        acmod = get_bits(&bits, 3);
> > +        lfeon = get_bits1(&bits);
> > +
> > +        *samples = eac3_blocks[numblkscod] * 256;
> > +        *bit_rate = frmsiz * (*sample_rate) * 16 / (*samples);
> > +        *channels = ac3_channels[acmod] + lfeon;
> > +
> > +        return frmsiz * 2;
> > +    }
>
> also i was thinking about something like the following but again iam not
> sure if its better, i hoped more could be merged ...
> iam fine with your variant too
>
>
> //the following 3 are a 16bit CRC in AC-3
> strmtyp = get_bits(&bits, 2);
> substreamid = get_bits(&bits, 3);
> frmsiz = get_bits(&bits, 11) + 1;
>
> fscod = get_bits(&bits, 2);
>
> /* AC-3 */
> frmsizecod = show_bits(&bits, 6);
>
> /* Enhanced AC-3 */
> numblkscod = get_bits(&bits, 2);
> acmod = get_bits(&bits, 3);
> lfeon = get_bits1(&bits);
>
> bsid = get_bits(&bits, 5);
>
> if(bsid <= 8){
>    if(fscod == 3)
>        return 0;
>
>    skip_bits(&bits, 3);        /* bsmod */
>    acmod = get_bits(&bits, 3);
>    if(acmod & 1 && acmod != 1)
>        skip_bits(&bits, 2);    /* cmixlev */
>    if(acmod & 4)
>        skip_bits(&bits, 2);    /* surmixlev */
>    if(acmod & 2)
>        skip_bits(&bits, 2);    /* dsurmod */
>
>    *sample_rate = ac3_sample_rates[fscod];
>    *bit_rate = ac3_bitrates[frmsizecod] * 1000;
>    *samples = 6 * 256;
>
>    frmsiz = ac3_frame_sizes[frmsizecod][fscod];
> }else if (bsid >= 10 && bsid <= 16) { /* Enhanced AC-3 */
>    if (strmtyp != 0 || substreamid != 0)
>        return 0;   /* Currently don't support additional streams */
>
>    if(fscod == 3){
>        if(numblkscod == 3)
>            return 0;
>
>        *sample_rate = ac3_sample_rates[numblkscod] / 2;
>        numblkscod = 3;
>    }else
>        *sample_rate = ac3_sample_rates[fscod];
>
>    acmod = get_bits(&bits, 3);
>
>    *samples = eac3_blocks[numblkscod] * 256;
>    *bit_rate = frmsiz * (*sample_rate) * 16 / (*samples);
> }else
>    return 0;
>
> lfeon = get_bits1(&bits);
> *channels = ac3_channels[acmod] + lfeon;


I personally prefer my way, but that could just be pride talking :) I think
the original patch was possibly easier to follow, but I don't object to
either way.

Ian