On Tue, Jan 03, 2012 at 12:23:51PM +0530, Shitiz Garg wrote:
> --- a/libavcodec/dca.c
> +++ b/libavcodec/dca.c
> @@ -793,6 +818,7 @@ static int dca_subframe_header(DCAContext * s, int 
> base_channel, int block_index
>          /* LFE samples */
>          int lfe_samples = 2 * s->lfe * (4 + block_index);
>          int lfe_end_sample = 2 * s->lfe * (4 + block_index + 
> s->subsubframes[s->current_subframe]);
> +
>          float lfe_scale;

unnecessary

> @@ -860,20 +893,26 @@ static int dca_subframe_header(DCAContext * s, int 
> base_channel, int block_index
>      if (!base_channel && s->lfe) {
>          int lfe_samples = 2 * s->lfe * (4 + block_index);
> -        int lfe_end_sample = 2 * s->lfe * (4 + block_index + 
> s->subsubframes[s->current_subframe]);
> +        int lfe_end_sample =
> +                2 * s->lfe * (4 + block_index + 
> s->subsubframes[s->current_subframe]);

I spoke about this before.

> @@ -1373,17 +1414,16 @@ static int dca_exss_mask2count(int mask)
>      /* count bits that mean speaker pairs twice */
>      return av_popcount(mask)
> -        + av_popcount(mask & (
> -            DCA_EXSS_CENTER_LEFT_RIGHT
> -          | DCA_EXSS_FRONT_LEFT_RIGHT
> -          | DCA_EXSS_FRONT_HIGH_LEFT_RIGHT
> -          | DCA_EXSS_WIDE_LEFT_RIGHT
> -          | DCA_EXSS_SIDE_LEFT_RIGHT
> -          | DCA_EXSS_SIDE_HIGH_LEFT_RIGHT
> -          | DCA_EXSS_SIDE_REAR_LEFT_RIGHT
> -          | DCA_EXSS_REAR_LEFT_RIGHT
> -          | DCA_EXSS_REAR_HIGH_LEFT_RIGHT
> -          ));
> +           + av_popcount(mask & (DCA_EXSS_CENTER_LEFT_RIGHT
> +                               | DCA_EXSS_FRONT_LEFT_RIGHT
> +                               | DCA_EXSS_FRONT_HIGH_LEFT_RIGHT
> +                               | DCA_EXSS_WIDE_LEFT_RIGHT
> +                               | DCA_EXSS_SIDE_LEFT_RIGHT
> +                               | DCA_EXSS_SIDE_HIGH_LEFT_RIGHT
> +                               | DCA_EXSS_SIDE_REAR_LEFT_RIGHT
> +                               | DCA_EXSS_REAR_LEFT_RIGHT
> +                               | DCA_EXSS_REAR_HIGH_LEFT_RIGHT
> +                               ));

Move the closing parentheses to the previous line and align the '|' on
the right side.

> @@ -1425,9 +1465,9 @@ static int dca_exss_parse_asset_header(DCAContext *s)
>  
>      if (s->static_fields) {
>          if (get_bits1(&s->gb))
> -            skip_bits(&s->gb, 4); // asset type descriptor
> +            skip_bits(&s->gb, 4);  // asset type descriptor
>          if (get_bits1(&s->gb))
> -            skip_bits_long(&s->gb, 24); // language descriptor
> +            skip_bits_long(&s->gb, 24);  // language descriptor

This was also commented on before.

> @@ -1578,7 +1617,7 @@ static void dca_exss_parse_header(DCAContext *s)
>          skip_bits(&s->gb, 3); // frame duration code
>  
>          if (get_bits1(&s->gb))
> -            skip_bits_long(&s->gb, 36); // timestamp
> +            skip_bits_long(&s->gb, 36);  // timestamp

and this

> @@ -1603,7 +1642,7 @@ static void dca_exss_parse_header(DCAContext *s)
>          for (i = 0; i < num_audiop; i++)
>              for (j = 0; j <= ss_index; j++)
>                  if (active_ss_mask[i] & (1 << j))
> -                    skip_bits(&s->gb, 8); // active asset mask
> +                    skip_bits(&s->gb, 8);  // active asset mask

and this and more below

There is too much repetition in this review for my taste, so I'll leave
it here until all issues from my last review are addressed.

> @@ -1947,17 +1983,18 @@ static const AVProfile profiles[] = {
>  };
>  
>  AVCodec ff_dca_decoder = {
> -    .name = "dca",
> -    .type = AVMEDIA_TYPE_AUDIO,
> -    .id = CODEC_ID_DTS,
> -    .priv_data_size = sizeof(DCAContext),
> -    .init = dca_decode_init,
> -    .decode = dca_decode_frame,
> -    .close = dca_decode_end,
> -    .long_name = NULL_IF_CONFIG_SMALL("DCA (DTS Coherent Acoustics)"),
> -    .capabilities = CODEC_CAP_CHANNEL_CONF | CODEC_CAP_DR1,
> -    .sample_fmts = (const enum AVSampleFormat[]) {
> -        AV_SAMPLE_FMT_FLT, AV_SAMPLE_FMT_S16, AV_SAMPLE_FMT_NONE
> -    },
> -    .profiles = NULL_IF_CONFIG_SMALL(profiles),
> +    .name               = "dca",
> +    .type               = AVMEDIA_TYPE_AUDIO,
> +    .id                 = CODEC_ID_DTS,
> +    .priv_data_size     = sizeof(DCAContext),
> +    .init               = dca_decode_init,
> +    .decode             = dca_decode_frame,
> +    .close              = dca_decode_end,
> +    .long_name          = NULL_IF_CONFIG_SMALL("DCA (DTS Coherent 
> Acoustics)"),
> +    .capabilities       = CODEC_CAP_CHANNEL_CONF | CODEC_CAP_DR1,
> +    .sample_fmts        = (const enum AVSampleFormat[]) {
> +        AV_SAMPLE_FMT_FLT, AV_SAMPLE_FMT_S16,
> +        AV_SAMPLE_FMT_NONE
> +     },
> +    .profiles           = NULL_IF_CONFIG_SMALL(profiles),

Check how it's done elsewhere.

Diego
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel

Reply via email to