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

[Michael Niedermayer]

Hi

On Thu, Jan 18, 2007 at 11:22:57PM +0000, Ian Caulfield wrote:
> 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.

ok, just resubmit the patch with the int->uint8_t and (fscod2 == 3) change
and ill approve it

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

The greatest way to live with honor in this world is to be what we pretend
to be. -- Socrates
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20070119/2d74b95c/attachment.pgp>