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