[FFmpeg-devel] [RFC v3 1/2] libavcodec: add a native decoder for the Daala video codec

James Almer jamrial at gmail.com
Fri Jan 1 20:20:46 CET 2016


On 1/1/2016 1:22 PM, Rostislav Pehlivanov wrote:
> +/* Gets a single bit if !!cond and returns ±1 */
> +static av_always_inline int _daalaent_cphase(DaalaEntropy *e, int cond)

Don't start function names with a _ prefix.

> +{
> +    if (cond)
> +        return 1 - 2*daalaent_decode_bits(e, 1);
> +    return 1;
> +}
> +#define daalaent_cphase(x,y) _daalaent_cphase(x, !!y)

Just do

static av_always_inline int daalaent_cphase(DaalaEntropy *e, int cond)
{
    if (cond)
        return 1 - 2*daalaent_decode_bits(e, 1);
    return 1;
}

Instead.

[...]

> +static inline void daalaent_decode_laplace_delta(DaalaEntropy *e, dctcoef *y,
> +                                                 int n, int k, dctcoef *curr,
> +                                                 const dctcoef *means)

I didn't check but i'm guessing compilers will ignore the inline attribute on big
functions like this that are used more than once.

[...]

> +/* Segments frame && decodes */
> +static inline int decode_block_rec(DaalaContext *s, HaarGradient g,
> +                                   int x, int y, uint8_t p, enum DaalaBsize bsize)
> +{
> +    int i, j, lc_skip, cbs;
> +    const int sx = x << bsize;
> +    const int sy = y << bsize;
> +    const int llim = 1 << bsize;
> +    const int off  = 2*bsize + !!p;
> +    const int bst  = s->bsizes_stride;
> +    const int xdec = s->fmt->dec[p][0];
> +    const int aw   = s->width >> xdec;
> +    enum DaalaBsize obsize = DAALA_BSIZE4x4(s->bsizes, bst, sx, sy);
> +
> +    if (s->h.haar) {
> +        obsize = bsize;
> +    } else if (!p) {
> +        lc_skip = daalaent_decode_cdf_adapt(&s->e, &s->skip_cdf, off,
> +                                            4 + (bsize > 0));
> +        obsize = lc_skip < 4 ? bsize : -1;
> +    }
> +
> +    if ((cbs = FFMAX(obsize, xdec)) == bsize) {
> +        cbs -= xdec;
> +        if (!p) {
> +            for (i = 0; i < llim; i++)
> +                for (j = 0; j < llim; j++)
> +                    DAALA_BSIZE4x4(s->bsizes, bst, sx + i, sy + j) = bsize;
> +        }
> +        if (p && s->dsp.cfl) {
> +            s->dsp.cfl((uint8_t *)s->lcoef, 1 << (cbs + DAALA_LOG_BSIZE0),
> +                       (uint8_t *)(s->dcoef[0] + (y << (2 + bsize))*s->width + (x << (2 + bsize))),
> +                       s->width, xdec, s->fmt->dec[p][1], cbs, obsize);
> +        }
> +        if (p && !s->h.haar) {
> +            lc_skip = daalaent_decode_cdf_adapt(&s->e, &s->skip_cdf, off, 4);
> +        }
> +        if (s->h.haar)
> +            decode_block_haar(s, x, y, p, cbs);
> +        else
> +            decode_block_pvq(s, x, y, p, cbs, lc_skip);
> +    } else {
> +        int bs = bsize - s->fmt->dec[p][0];
> +        int bo = (y << (DAALA_LOG_BSIZE0 + bs))*aw + (x << (DAALA_LOG_BSIZE0 + bs));
> +        int hfilter = (x + 1) << (DAALA_LOG_BSIZE0 + bs) <= s->width;
> +        int vfilter = (y + 1) << (DAALA_LOG_BSIZE0 + bs) <= s->height;
> +        if (!s->h.key_frame && s->dsp.pre_split_filter)
> +            s->dsp.pre_split_filter((uint8_t *)(s->ccoef + bo), aw, bs,
> +                                    hfilter, vfilter);
> +        if (s->h.key_frame)
> +            get_haar_dc_lvl(s, &g, s->dcoef[p], 2*x, 2*y, p, bsize - 1);
> +        decode_block_rec(s, g, 2*x + 0, 2*y + 0, p, bsize - 1);

All this is definitely not going to be inlined.

> +        decode_block_rec(s, g, 2*x + 1, 2*y + 0, p, bsize - 1);
> +        decode_block_rec(s, g, 2*x + 0, 2*y + 1, p, bsize - 1);
> +        decode_block_rec(s, g, 2*x + 1, 2*y + 1, p, bsize - 1);
> +        if (s->dsp.post_split_filter)
> +            s->dsp.post_split_filter((uint8_t *)(s->ccoef[p] + bo), aw, bs,
> +                                     hfilter, vfilter);
> +    }
> +
> +    return 0;
> +}

[...]

> +av_cold int daaladsp_init(DaalaDSP *d, int bit_depth)

non static functions need to have an ff_ prefix (or avpriv_ if they need to be
used by other libraries).

> +{
> +    switch(bit_depth) {
> +    case 8:
> +        daaladsp_init_8bit(d);
> +        break;
> +    case 10:
> +    case 12:
> +    default:
> +        return 1;
> +        break;
> +    }
> +    return 0;
> +}
> \ No newline at end of file

This should be fixed.

[...]

> diff --git a/libavcodec/daalatab.c b/libavcodec/daalatab.c
> new file mode 100644
> index 0000000..3994387
> --- /dev/null
> +++ b/libavcodec/daalatab.c
> @@ -0,0 +1,1563 @@
> +/*
> + * Copyright 2001-2015 Xiph.Org and contributors.
> + * Copyright 2015 Rostislav Pehlivanov <atomnuker at gmail.com>
> + *
> + * Redistribution and use in source and binary forms, with or without
> + * modification, are permitted provided that the following conditions
> + * are met:
> + *
> + * - Redistributions of source code must retain the above copyright
> + *   notice, this list of conditions and the following disclaimer.
> + *
> + * - Redistributions in binary form must reproduce the above copyright
> + *   notice, this list of conditions and the following disclaimer in the
> + *   documentation and/or other materials provided with the distribution.
> + *
> + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
> + * ``AS IS'' AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
> + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
> + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER
> + * OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL,
> + * EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO,
> + * PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR
> + * PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF
> + * LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING
> + * NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
> + * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> + *
> + * This file is part of FFmpeg.
> + *
> + * 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
> + */
> +
> +#include "daalatab.h"
> +
> +const struct DaalaPixFmts daala_valid_formats[] = {

Same with tables, ff_ prefix.

> +    {AV_PIX_FMT_YUV420P,      3,  8, 1, {{0,0}, {1,1}, {1,1}, {0,0}}},
> +    {AV_PIX_FMT_YUV444P,      3,  8, 1, {{0,0}, {0,0}, {0,0}, {0,0}}}
> +};
> +const int daala_valid_formats_num = FF_ARRAY_ELEMS(daala_valid_formats);
> +
> +/* Haar "quantization matrix" for each decomposition level */
> +const int daala_haar_qm[][DAALA_LOG_BSIZE_MAX] = {
> +    {16, 16, 16, 16, 24, 32}, /* horizontal/vertical direction. */
> +    {16, 16, 16, 24, 32, 48}, /* "diagonal" direction. */
> +};
> +
> +/* Keyframe blur filter strength for every plane */
> +const int daala_bilinear_blur[] = {5, 20, 20, 5};
> +
> +/* Haar basis scaling compensation, [0] - x,y; [1] - diag */
> +const int32_t daala_dc_comp[][2] = { {21, 25}, {18, 20}, {17, 18}, {17, 17} };
> +
> +/* Flat (e.g. PSNR) QM */
> +const int16_t daala_qm_flat[] = {
> +    16, 16, 16, 16, 16, 16, 16, 16,
> +    16, 16, 16, 16, 16, 16, 16, 16,
> +    16, 16, 16, 16, 16, 16, 16, 16,
> +    16, 16, 16, 16, 16, 16, 16, 16,
> +    16, 16, 16, 16, 16, 16, 16, 16,
> +    16, 16, 16, 16, 16, 16, 16, 16,
> +    16, 16, 16, 16, 16, 16, 16, 16,
> +    16, 16, 16, 16, 16, 16, 16, 16
> +};
> +
> +/* HVS quantization matrix */
> +static const int16_t daala_qm_hvs[] = {
> +    16, 16, 18, 21, 24, 28, 32, 36,
> +    16, 17, 20, 21, 24, 27, 31, 35,
> +    18, 20, 24, 25, 27, 31, 33, 38,
> +    21, 21, 25, 28, 30, 34, 37, 42,
> +    24, 24, 27, 30, 34, 38, 43, 49,
> +    28, 27, 31, 34, 38, 44, 50, 58,
> +    32, 31, 33, 37, 43, 50, 58, 68,
> +    36, 35, 38, 42, 49, 58, 68, 78
> +};
> +
> +const int16_t *const daala_qmatrices[] = {
> +    daala_qm_flat,
> +    daala_qm_hvs
> +};
> +
> +/* Chroma from luma scaling */
> +const uint16_t daaladsp_cfl_scale[4][4] = {
> +    { 128, 128, 100,  36 },
> +    { 128,  80,  71,  35 },
> +    { 100,  71,  35,  31 },
> +    {  36,  35,  31,  18 },
> +};

Why not use uint8_t for all these?



More information about the ffmpeg-devel mailing list