[FFmpeg-devel] [PATCH 1/2] aptx: implement the aptX bluetooth codec

[Rostislav Pehlivanov]

On 8 November 2017 at 17:26, Michael Niedermayer <michael at niedermayer.cc>
wrote:

> On Wed, Nov 08, 2017 at 02:06:09PM +0100, Aurelien Jacobs wrote:
> [...]
> > +typedef const struct {
> > +    const int32_t *quantize_intervals;
> > +    const int32_t *invert_quantize_dither_factors;
> > +    const int32_t *quantize_dither_factors;
>
> > +    const int32_t *quantize_factor_select_offset;
>
> this would fit in int16_t *
>
>
> > +    int tables_size;
> > +    int32_t quantized_bits;
> > +    int32_t prediction_order;
> > +} ConstTables;
> > +
> > +static ConstTables tables[NB_SUBBANDS] = {
> > +    [LF]  = { quantize_intervals_LF,
> > +              invert_quantize_dither_factors_LF,
> > +              quantize_dither_factors_LF,
> > +              quantize_factor_select_offset_LF,
> > +              FF_ARRAY_ELEMS(quantize_intervals_LF),
> > +              7, 24 },
> > +    [MLF] = { quantize_intervals_MLF,
> > +              invert_quantize_dither_factors_MLF,
> > +              quantize_dither_factors_MLF,
> > +              quantize_factor_select_offset_MLF,
> > +              FF_ARRAY_ELEMS(quantize_intervals_MLF),
> > +              4, 12 },
> > +    [MHF] = { quantize_intervals_MHF,
> > +              invert_quantize_dither_factors_MHF,
> > +              quantize_dither_factors_MHF,
> > +              quantize_factor_select_offset_MHF,
> > +              FF_ARRAY_ELEMS(quantize_intervals_MHF),
> > +              2, 6 },
> > +    [HF]  = { quantize_intervals_HF,
> > +              invert_quantize_dither_factors_HF,
> > +              quantize_dither_factors_HF,
> > +              quantize_factor_select_offset_HF,
> > +              FF_ARRAY_ELEMS(quantize_intervals_HF),
> > +              3, 12 },
> > +};
> > +
>
> > +static const int32_t quantization_factors[32] = {
> > +    2048, 2093, 2139, 2186, 2233, 2282, 2332, 2383,
> > +    2435, 2489, 2543, 2599, 2656, 2714, 2774, 2834,
> > +    2896, 2960, 3025, 3091, 3158, 3228, 3298, 3371,
> > +    3444, 3520, 3597, 3676, 3756, 3838, 3922, 4008,
> > +};
>
> this too would fir in int16_t
>
> [...]
> > +/*
> > + * Push one sample into a circular signal buffer.
> > + */
> > +av_always_inline
> > +static void aptx_qmf_filter_signal_push(FilterSignal *signal, int32_t
> sample)
> > +{
> > +    signal->buffer[signal->pos            ] = sample;
> > +    signal->buffer[signal->pos+FILTER_TAPS] = sample;
> > +    signal->pos = (signal->pos + 1) % FILTER_TAPS;
>
> % could be replaced by &
>
>
> > +}
> > +
> > +/*
> > + * Compute the convolution of the signal with the coefficients, and
> reduce
> > + * to 24 bits by applying the specified right shifting.
> > + */
> > +av_always_inline
> > +static int32_t aptx_qmf_convolution(FilterSignal *signal,
> > +                                    const int32_t coeffs[FILTER_TAPS],
> > +                                    int shift)
> > +{
> > +    int32_t *sig = &signal->buffer[signal->pos];
> > +    int64_t e = 0;
> > +
>
> > +    for (int i = 0; i < FILTER_TAPS; i++)
>
> "for (int" is something we avoided as some comilers didnt like it,
> iam not sure if this is still true but there are none in the codebase
>
> also a fate test for this would be a good idea
>
>
No, I completely reject this idea.

I want to use for (int loops and have always wanted to use them.
Show me a comiler which we support which doesn't accept those. Then I'll
suggest we drop support for it.

Its time we bring the codebase to the 21st century and this is my main
issue - being unable to use for (int loops.