[FFmpeg-devel] [RFC v5] libavcodec: add a native Daala decoder

[Ganesh Ajjanagadde]

On Wed, Jan 6, 2016 at 5:22 PM, Ronald S. Bultje <rsbultje at gmail.com> wrote:
> Hello,
>
[...]
>
>> +#define DAALA_QM_SCALE (1 << 15)
>> +#define DAALA_QM_SCALE_MAX (DAALA_QM_SCALE - 1)
>> +#define DAALA_QM_SCALE_UNIT (1.0f/DAALA_QM_SCALE_MAX)
>> +#define DAALA_QM_INV_SCALE (1 << 12)
>> +#define DAALA_QM_INV_SCALE_UNIT (1.0f/DAALA_QM_INV_SCALE)
>> +#define DAALA_QM_BSIZE (DAALA_BSIZE_MAX*DAALA_BSIZE_MAX)
>> +#define DAALA_QM_BUFFER_SIZE (DAALA_NBSIZES*2*DAALA_QM_BSIZE)
>
> Floats in a video decoder? That looks strange. I’m pretty sure these are
> placeholders for integer arithmetic?

Just my curiosity, but isn't it easier while developing video codecs
to work with floats, and then convert to fixed point arithmetic? It
can be hard while designing algorithms from the start to agonize over
fixed point at the very beginning, echoing your remark about
placeholders.

[...]
>
>> +static inline double daalapvq_gain_raise(double cg, int q0, double beta)
>> +{
>> +    if (beta == 1.0f)
>> +        return cg*q0;
>> +    else if (beta == 1.5f) {
>> +        cg *= q0*DAALAPVQ_COMPAND_SCALE_1;
>> +        return DAALAPVQ_COMPAND_SCALE*cg*sqrt(cg);
>> +    }
>> +    return DAALAPVQ_COMPAND_SCALE*pow(cg*q0*DAALAPVQ_COMPAND_SCALE_1,
> beta);
>> +}
>
> Pow, sqrt, really?

That pow may be translatable to sqrt, code above suggest half
integers, though I have not studied this. In any case, a comment with
a TODO/FIXME maybe useful for reference.

[...]
>
>> +#define DAALA_BSIZE8x8(arr, bstride, bx, by) ((arr)[(by)*(bstride) +
> (bx)])
>> +#define DAALA_BSIZE4x4(arr, bstride, bx, by) DAALA_BSIZE8x8(arr,
> bstride, (bx) >> 1, (by) >> 1)
>
> ? So, this may just be me, but I feel this is getting in the #define inc(a)
> ((a)++) territory.

+1

[...]