[FFmpeg-devel] [PATCH 2/2] avcodec/libdav1d: use a custom picture allocator

[James Almer]

On 3/5/2019 3:19 PM, Vittorio Giovara wrote:
> On Mon, Mar 4, 2019 at 4:08 PM James Almer <jamrial at gmail.com> wrote:
> 
>> Replaces the libdav1d internal allocator. It uses an AVBufferPool to
>> reduce the
>> amount of allocated buffers.
>> About 5% speed up when decoding 720p or higher streams.
>>
>> Signed-off-by: James Almer <jamrial at gmail.com>
>> ---
>> get_buffer2() can't be used for this decoder, as there's no guarantee the
>> buffers
>> it returns will respect the constrains specified by libdav1d.
>>
>>  libavcodec/libdav1d.c | 72 ++++++++++++++++++++++++++++++++++++++-----
>>  1 file changed, 65 insertions(+), 7 deletions(-)
>>
>> diff --git a/libavcodec/libdav1d.c b/libavcodec/libdav1d.c
>> index 459bbae687..855c27db7f 100644
>> --- a/libavcodec/libdav1d.c
>> +++ b/libavcodec/libdav1d.c
>> @@ -22,6 +22,7 @@
>>  #include <dav1d/dav1d.h>
>>
>>  #include "libavutil/avassert.h"
>> +#include "libavutil/imgutils.h"
>>  #include "libavutil/opt.h"
>>
>>  #include "avcodec.h"
>> @@ -31,12 +32,21 @@
>>  typedef struct Libdav1dContext {
>>      AVClass *class;
>>      Dav1dContext *c;
>> +    AVBufferPool *pool;
>> +    int pool_size;
>>
>>      Dav1dData data;
>>      int tile_threads;
>>      int apply_grain;
>>  } Libdav1dContext;
>>
>> +static const enum AVPixelFormat pix_fmt[][3] = {
>> +    [DAV1D_PIXEL_LAYOUT_I400] = { AV_PIX_FMT_GRAY8,   AV_PIX_FMT_GRAY10,
>>   AV_PIX_FMT_GRAY12 },
>> +    [DAV1D_PIXEL_LAYOUT_I420] = { AV_PIX_FMT_YUV420P,
>> AV_PIX_FMT_YUV420P10, AV_PIX_FMT_YUV420P12 },
>> +    [DAV1D_PIXEL_LAYOUT_I422] = { AV_PIX_FMT_YUV422P,
>> AV_PIX_FMT_YUV422P10, AV_PIX_FMT_YUV422P12 },
>> +    [DAV1D_PIXEL_LAYOUT_I444] = { AV_PIX_FMT_YUV444P,
>> AV_PIX_FMT_YUV444P10, AV_PIX_FMT_YUV444P12 },
>> +};
>> +
>>  static void libdav1d_log_callback(void *opaque, const char *fmt, va_list
>> vl)
>>  {
>>      AVCodecContext *c = opaque;
>> @@ -44,6 +54,57 @@ static void libdav1d_log_callback(void *opaque, const
>> char *fmt, va_list vl)
>>      av_vlog(c, AV_LOG_ERROR, fmt, vl);
>>  }
>>
>> +static int libdav1d_picture_allocator(Dav1dPicture *p, void *cookie) {
>> +    Libdav1dContext *dav1d = cookie;
>> +    enum AVPixelFormat format = pix_fmt[p->p.layout][p->seq_hdr->hbd];
>> +    int ret, linesize[4], h = FFALIGN(p->p.h, 128);
>> +    uint8_t *aligned_ptr, *data[4];
>> +    AVBufferRef *buf;
>> +
>> +    ret = av_image_fill_arrays(data, linesize, NULL, format,
>> FFALIGN(p->p.w, 128),
>> +                               h, DAV1D_PICTURE_ALIGNMENT);
>> +    if (ret < 0)
>> +        return ret;
>> +
>> +    if (ret != dav1d->pool_size) {
>> +        av_buffer_pool_uninit(&dav1d->pool);
>> +        // Use twice the amount of required padding bytes for aligned_ptr
>> below.
>> +        dav1d->pool = av_buffer_pool_init(ret + DAV1D_PICTURE_ALIGNMENT *
>> 2, NULL);
>> +        if (!dav1d->pool)
>> +            return -ENOMEM;
>>
> 
> AVERROR(ENOMEM) ?

AVERROR(x) will expand to -x on every target libdav1d supports, so
there's no difference. And since this is a callback function, i figured
following the doxy to the letter was proper.

I can replace it if you prefer in any case.

> 
> +        dav1d->pool_size = ret;
>> +    }
>> +    buf = av_buffer_pool_get(dav1d->pool);
>> +    if (!buf)
>> +        return -ENOMEM;
>> +
>> +    // libdav1d requires DAV1D_PICTURE_ALIGNMENT aligned buffers, which
>> av_malloc()
>> +    // doesn't guarantee for example when AVX is disabled at configure
>> time.
>> +    // Use the extra DAV1D_PICTURE_ALIGNMENT padding bytes in the buffer
>> to align it
>> +    // if required.
>> +    aligned_ptr = (uint8_t *)FFALIGN((uintptr_t)buf->data,
>> DAV1D_PICTURE_ALIGNMENT);
>> +    ret = av_image_fill_pointers(data, format, h, aligned_ptr, linesize);
>> +    if (ret < 0) {
>> +        av_buffer_unref(&buf);
>> +        return ret;
>> +    }
>> +
>> +    p->data[0] = data[0];
>> +    p->data[1] = data[1];
>> +    p->data[2] = data[2];
>> +    p->stride[0] = linesize[0];
>> +    p->stride[1] = linesize[1];
>> +    p->allocator_data = buf;
>> +
>> +    return 0;
>> +}
>> +
>> +static void libdav1d_picture_release(Dav1dPicture *p, void *cookie) {
>> +    AVBufferRef *buf = p->allocator_data;
>> +
>> +    av_buffer_unref(&buf);
>> +}
>>
> 
> nit: keep { on a new line for functions

Fixed locally. And i see a couple others that also need to be changed in
a separate commit.