On Fri, Apr 08, 2011 at 01:41:37AM +0200, Benjamin Larsson wrote:
>
> ---
> libavcodec/Makefile | 1 +
> libavcodec/allcodecs.c | 2 +-
> libavcodec/dcaenc.c | 581
> ++++++++++++++++++++++++++++++++++++++++++++++++
> libavcodec/dcaenc.h | 544 +++++++++++++++++++++++++++++++++++++++++++++
> 4 files changed, 1127 insertions(+), 1 deletions(-)
changelog, docs update, minor bump
> --- /dev/null
> +++ b/libavcodec/dcaenc.c
> @@ -0,0 +1,581 @@
> +/*
> + * DCA encoder
> + * Copyright (C) 2008 Alexander E. Patrakov
> + * 2010 Benjamin Larsson
> + * 2011 Xiang Wang
> + * This file is part of Libav.
> + *
> + * FFmpeg is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * FFmpeg is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with FFmpeg; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301
> USA
That still contains some FFmpeg references, please fix.
> +static void add_new_samples(DCAContext *c, const int32_t *in, int count, int
> channel){
{ on next line, long line
> +static void qmf_decompose(DCAContext *c, int32_t in[32], int32_t out[32],
> int channel)
long line
> + int band, i, j, k;
> + int32_t resp;
> + int32_t accum[DCA_SUBBANDS_32];
> +
> + memset(accum,0,sizeof(accum));
Just initialize to 0
> +static int32_t lfe_fir_64i[512];
> +static int lfe_downsample(DCAContext *c, int32_t in[LFE_INTERPOLATION]){
{ on next line
> +static void init_lfe_fir(void){
ditto
> + static int initialized;
> + int i;
> + if(initialized)
if (
> + for(i=0; i<512; i++)
for (
.. and please give those operators some room to breathe ..
> +static void put_primary_audio_header(DCAContext *c)
> +{
> + /* From dca.c */
> + static const int bitlen[11] = { 0, 1, 2, 2, 2, 2, 3, 3, 3, 3, 3 };
> + static const int thr[11] = { 0, 1, 3, 3, 3, 3, 7, 7, 7, 7, 7 };
That comment makes me suspicious - are these duplicated?
> + /* Subband activity count */
> + for (ch=0; ch<c->prim_channels; ch++) {
> +
> + /* High frequency VQ start subband */
> + for (ch=0; ch<c->prim_channels; ch++) {
> +
> + /* Joint intensity coding index: 0, 0 */
> + for (ch=0; ch<c->prim_channels; ch++) {
> +
> + /* Transient mode codebook: A4, A4 (arbitrary) */
> + for (ch=0; ch<c->prim_channels; ch++) {
> +
> + /* Scale factor code book: 7 bit linear, 7-bit sqrt table (for each
> channel) */
> + for (ch=0; ch<c->prim_channels; ch++) {
> +
> + /* Bit allocation quantizer select: linear 5-bit */
> + for (ch=0; ch<c->prim_channels; ch++) {
> +
> + /* Quantization index codebook select: dummy data
> + to avoid transmission of scale factor adjustment */
> + for (i=1; i<11; i++) {
> + for (ch=0; ch<c->prim_channels; ch++) {
.. more operators in need of elbow room .. :)
There are more instances below, same for 'if(' and 'for(', { on the wrong
line and excessively long lines that could be shortened easily. Fixing
would be appreciated.
You could also drop (or not) some {} around if/for blocks.
> +/**
> + * 8-23 bits quantization
> + * @param sample
> + * @param bits
> + */
> +static inline uint32_t quantize(int32_t sample, int bits)
These Doxygen parameter comments are pretty useless.
> + switch(avctx->channel_layout) {
> + case AV_CH_LAYOUT_STEREO: c->a_mode = 2; c->num_channel = 2; break;
switch (
> + case AV_CH_LAYOUT_5POINT0: c->a_mode = 9; c->num_channel = 9; break;
> + case AV_CH_LAYOUT_5POINT1: c->a_mode = 9; c->num_channel = 9; break;
> + case AV_CH_LAYOUT_5POINT0_BACK: c->a_mode = 9; c->num_channel = 9;
> break;
> + case AV_CH_LAYOUT_5POINT1_BACK: c->a_mode = 9; c->num_channel = 9;
> break;
> + default:
> + av_log(avctx, AV_LOG_ERROR, "Only stereo, 5.1, 5.0, 5.0back and
> 5.0front channel layouts supported at the moment!\n");
> + return AVERROR_PATCHWELCOME;
> + }
Indent the case statements at the same depth as the switch and fix that
instance of 2-space indentation.
> +AVCodec ff_dca_encoder = {
> + .name = "dca",
> + .type = CODEC_TYPE_AUDIO,
> + .id = CODEC_ID_DTS,
> + .priv_data_size = sizeof(DCAContext),
> + .init = DCA_encode_init,
> + .encode = DCA_encode_frame,
> + .capabilities = CODEC_CAP_EXPERIMENTAL,
> + .sample_fmts = (const enum
> AVSampleFormat[]){AV_SAMPLE_FMT_S16,AV_SAMPLE_FMT_NONE},
> + NULL,
> + NULL,
> +};
pointless trailing NULLs, long_name missing
> --- /dev/null
> +++ b/libavcodec/dcaenc.h
> @@ -0,0 +1,544 @@
> +/*
> + * DCA encoder tables
> + * Copyright (C) 2008 Alexander E. Patrakov
> + *
> + * This file is part of FFmpeg.
Fix this please
> +/* This is a scaled version of the response of the reference decoder to
> + this vector of subband samples: ( 1.0 0.0 0.0 ... 0.0 )
> + */
> +
> +static const int32_t UnQMF[512] = {
> +7,
> +4,
> +-961,
> +-2844,
> +-8024,
> +-18978,
> +-32081,
Indentation and maybe several columns would make this more readable.
Diego
_______________________________________________
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel
