On Thu, Jun 05, 2014 at 05:33:19PM +0200, Niels Möller wrote:
>
> these are my changes to implement dca xll (not yet bit exact), relative
> to current master 39ec5e1cf8444f827c42effb76e5694e091bbff3. Tested with
> 5.1 and 7.1 streams.
>
> Comments appreciated. I see no obvious way to split it in smaller
> independent pieces, but maybe someone else does.
You could add the tables as a separate dummy commit. That should not break
anything and reduce the line count considerably. The TRACE_XLL bits can
probably go away. Otherwise no good idea.
some stylistic remarks below
> --- a/libavcodec/dcadata.h
> +++ b/libavcodec/dcadata.h
> @@ -7508,6 +7536,601 @@ DECLARE_ALIGNED(16, static const float,
> lfe_fir_128)[] =
>
> +#define SCALE(c) ((float)(c) / (256.0f * 32768.0f*8388608.0f))
> +DECLARE_ALIGNED(16, static const float, lfe_xll_fir_64)[] =
> +{
> + SCALE( 6103),SCALE( 52170),SCALE(-558064),SCALE(1592440),
> + SCALE(6290049),SCALE(1502534),SCALE(-546669),SCALE( 53047),
Four spaces indentation and spaces after comma please.
> +DECLARE_ALIGNED(16, static const float, fir_64bands)[1024] =
> +{
> + -2.3590980216497750e-5, -2.3955708939763280e-5,
> + -2.4307547722489910e-5, -2.4645348200903720e-5,
> + -2.4967930863542620e-5, -2.5274085082324960e-5,
> + /* Bank 1 */
> + 2.5562569399199020e-5, 2.5832111724562040e-5,
> + 2.6081413415833200e-5, 2.6309149390520640e-5,
Extra good karma for aligning the numbers - if you don't do it, I will
send you a patch.
> --- a/libavcodec/dcadec.c
> +++ b/libavcodec/dcadec.c
> @@ -4,6 +4,8 @@
> * Copyright (C) 2004 Benjamin Zores
> * Copyright (C) 2006 Benjamin Larsson
> * Copyright (C) 2007 Konstantin Shishkov
> + * Copyright (C) 2012 Paul B Mahol
Which part is Paul's?
> + * Copyright (C) 2014 Niels Möller
Something is wrong with your UTF-8 setup.
> @@ -52,6 +55,7 @@
>
> //#define TRACE
> +#define TRACE_XLL
leftover debug code?
> @@ -285,6 +297,62 @@ static av_always_inline int get_bitalloc(GetBitContext
> *gb, BitAlloc *ba,
> }
>
> typedef struct {
> +} XllChSetSubHeader;
> +
> +typedef struct {
> +} XllNavi;
Please avoid anonymously typedeffed structs. Yes, I know there are many
already, but let's not add new ones.
> +typedef struct QMF64_table
> +{
{ on the previous line
> @@ -379,12 +449,30 @@ typedef struct {
> int current_subsubframe;
>
> int core_ext_mask; ///< present extensions in the core substream
> -
> + int exss_ext_mask; // Non-core extensions
stray tabs (more below)
> @@ -941,8 +1033,85 @@ static void qmf_32_subbands(DCAContext *s, int chans,
>
> -static void lfe_interpolation_fir(DCAContext *s, int decimation_select,
> - int num_deci_sample, float *samples_in,
> +static QMF64_table *qmf64_precompute(void)
> +{
> + QMF64_table *table = av_malloc (sizeof(*table));
Drop the space between function name and '(' (more below).
> + unsigned i, j;
> + if (!table)
> + return NULL;
> +
> + for (i = 0; i < 32; i++)
> + for (j = 0; j < 32; j++)
> + table->dct4_coeff[i][j] = cos((2*i+1)*(2*j+1) * M_PI / 128);
Give the operators a bit of space to breathe (more below).
> @@ -1241,29 +1413,57 @@ static int dca_subsubframe(DCAContext *s, int
> base_channel, int block_index)
> + M_SQRT2 / 32768.0);
> + }
> + }
> + else {
} and else on the same line (more below)
> /* static float pcm_to_double[8] = { 32768.0, 32768.0, 524288.0,
> 524288.0,
> - 0, 8388608.0, 8388608.0 };*/
> + 0, 8388608.0, 8388608.0 };*/
stray change
> @@ -1461,6 +1661,869 @@ static void dca_exss_skip_mix_coeffs(GetBitContext
> *gb, int channels, int out_ch
>
> +static inline int
> +get_bits_sm(GetBitContext *s, unsigned n)
> +
> +static int32_t
> +dca_get_dmix_coeff(DCAContext *s)
> +
> +static int32_t
> +dca_get_inv_dmix_coeff(DCAContext *s)
Keep type and function name on the same line.
> + s->xll_channels = s->xll_residual_channels = 0;
> + s->xll_nch_sets = get_bits(&s->gb, 4) + 1;
> + s->xll_segments = 1 << get_bits(&s->gb, 4);
> + s->xll_log_smpl_in_seg = get_bits(&s->gb, 4);
> + s->xll_smpl_in_seg = 1 << s->xll_log_smpl_in_seg;
> + s->xll_bits4seg_size = get_bits(&s->gb, 5) + 1;
> + s->xll_banddata_crcen = get_bits(&s->gb, 2);
> + s->xll_scalable_lsb = get_bits1(&s->gb);
> + s->xll_bits4ch_mask = get_bits(&s->gb, 5) + 1;
nit: align the =
> @@ -1684,15 +2754,51 @@ static void dca_exss_parse_header(DCAContext *s)
> + /* parse extensions that we know about */
> + switch (mkr)
> + {
{ on the same line as the switch
Diego
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel
