[FFmpeg-devel] [PATCH v3 1/5] libavutil: some VAAPI infrastructure

Mark Thompson sw at jkqxz.net
Tue Jan 19 14:19:36 CET 2016


On 19/01/16 11:54, wm4 wrote:
> On Mon, 18 Jan 2016 22:49:51 +0000
> Mark Thompson <sw at jkqxz.net> wrote:
> 
>> ---
>>  configure          |   4 +
>>  libavutil/Makefile |   1 +
>>  libavutil/vaapi.c  | 497 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>>  libavutil/vaapi.h  | 123 +++++++++++++
>>  4 files changed, 625 insertions(+)
>>  create mode 100644 libavutil/vaapi.c
>>  create mode 100644 libavutil/vaapi.h
>>
> 
> Trying to do a more thorough review.

Thanks :)

>> diff --git a/configure b/configure
>> index 7cef6f5..1c77015 100755
>> --- a/configure
>> +++ b/configure
>> @@ -5739,6 +5739,10 @@ enabled vaapi && enabled xlib &&
>>      check_lib2 "va/va.h va/va_x11.h" vaGetDisplay -lva -lva-x11 &&
>>      enable vaapi_x11
>>
>> +enabled vaapi &&
>> +    check_lib2 "va/va.h va/va_drm.h" vaGetDisplayDRM -lva -lva-drm &&
>> +    enable vaapi_drm
>> +
>>  enabled vdpau &&
>>      check_cpp_condition vdpau/vdpau.h "defined VDP_DECODER_PROFILE_MPEG4_PART2_ASP" ||
>>      disable vdpau
>> diff --git a/libavutil/Makefile b/libavutil/Makefile
>> index bf8c713..8025f9f 100644
>> --- a/libavutil/Makefile
>> +++ b/libavutil/Makefile
>> @@ -146,6 +146,7 @@ OBJS-$(!HAVE_ATOMICS_NATIVE)            += atomic.o                     \
>>
>>  OBJS-$(CONFIG_LZO)                      += lzo.o
>>  OBJS-$(CONFIG_OPENCL)                   += opencl.o opencl_internal.o
>> +OBJS-$(CONFIG_VAAPI)                    += vaapi.o
>>
>>  OBJS += $(COMPAT_OBJS:%=../compat/%)
>>
>> diff --git a/libavutil/vaapi.c b/libavutil/vaapi.c
>> new file mode 100644
>> index 0000000..8a9a524
>> --- /dev/null
>> +++ b/libavutil/vaapi.c
>> @@ -0,0 +1,497 @@
>> +/*
>> + * VAAPI helper functions.
>> + *
>> + * Copyright (C) 2016 Mark Thompson <mrt at jkqxz.net>
>> + *
>> + * 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 "vaapi.h"
>> +
>> +#include "avassert.h"
>> +#include "imgutils.h"
>> +#include "pixfmt.h"
>> +
>> +
>> +void av_vaapi_lock_hardware_context(AVVAAPIHardwareContext *ctx)
>> +{
>> +    if(ctx->lock)
>> +        ctx->lock();
>> +}
>> +
>> +void av_vaapi_unlock_hardware_context(AVVAAPIHardwareContext *ctx)
>> +{
>> +    if(ctx->unlock)
>> +        ctx->unlock();
>> +}
> 
> The lock callbacks should take some sort of opaque context pointer.

Ok; will add.

>> +
>> +static const AVClass vaapi_pipeline_class = {
>> +    .class_name = "VAAPI/pipeline",
>> +    .item_name  = av_default_item_name,
>> +    .version    = LIBAVUTIL_VERSION_INT,
>> +};
>> +
>> +static int vaapi_create_surfaces(AVVAAPIPipelineContext *ctx,
>> +                                 AVVAAPISurfaceConfig *config,
>> +                                 AVVAAPISurface *surfaces,
>> +                                 VASurfaceID *ids)
>> +{
>> +    VAStatus vas;
>> +    int i;
>> +
>> +    vas = vaCreateSurfaces(ctx->hardware_context->display, config->rt_format,
>> +                           config->width, config->height, ids, config->count,
>> +                           config->attributes, config->attribute_count);
>> +    if(vas != VA_STATUS_SUCCESS) {
>> +        av_log(ctx, AV_LOG_ERROR, "Failed to create "
>> +               "surfaces: %d (%s).\n", vas, vaErrorStr(vas));
>> +        return AVERROR(EINVAL);
>> +    }
>> +
>> +    for(i = 0; i < config->count; i++) {
>> +        surfaces[i].id       = ids[i];
>> +        surfaces[i].refcount = 0;
>> +        surfaces[i].hardware_context = ctx->hardware_context;
>> +        surfaces[i].config   = config;
>> +    }
>> +
>> +    return 0;
>> +}
>> +
>> +static void vaapi_destroy_surfaces(AVVAAPIPipelineContext *ctx,
>> +                                   AVVAAPISurfaceConfig *config,
>> +                                   AVVAAPISurface *surfaces,
>> +                                   VASurfaceID *ids)
>> +{
>> +    VAStatus vas;
>> +    int i;
>> +
>> +    for(i = 0; i < config->count; i++) {
>> +        av_assert0(surfaces[i].id == ids[i]);
>> +        if(surfaces[i].refcount > 0)
>> +            av_log(ctx, AV_LOG_WARNING, "Destroying surface %#x which is "
>> +                   "still in use.\n", surfaces[i].id);
>> +        av_assert0(surfaces[i].hardware_context == ctx->hardware_context);
>> +        av_assert0(surfaces[i].config == config);
>> +    }
>> +
>> +    vas = vaDestroySurfaces(ctx->hardware_context->display,
>> +                            ids, config->count);
>> +    if(vas != VA_STATUS_SUCCESS) {
>> +        av_log(ctx, AV_LOG_ERROR, "Failed to destroy surfaces: "
>> +               "%d (%s).\n", vas, vaErrorStr(vas));
>> +    }
>> +}
>> +
>> +int ff_vaapi_pipeline_init(AVVAAPIPipelineContext *ctx,
>> +                           AVVAAPIHardwareContext *hw_ctx,
>> +                           AVVAAPIPipelineConfig *config,
>> +                           AVVAAPISurfaceConfig *input,
>> +                           AVVAAPISurfaceConfig *output)
>> +{
>> +    VAStatus vas;
>> +    int err;
>> +
>> +    // Currently this only supports a pipeline which actually creates
>> +    // output surfaces.  An intra-only encoder (e.g. JPEG) won't, so
>> +    // some modification would be required to make that work.
>> +    if(!output)
>> +        return AVERROR(EINVAL);
>> +
>> +    memset(ctx, 0, sizeof(*ctx));
>> +    ctx->class = &vaapi_pipeline_class;
>> +
>> +    ctx->hardware_context = hw_ctx;
>> +    ctx->config = config;
>> +
>> +    vas = vaCreateConfig(hw_ctx->display, config->profile,
>> +                         config->entrypoint, config->attributes,
>> +                         config->attribute_count, &ctx->config_id);
>> +    if(vas != VA_STATUS_SUCCESS) {
>> +        av_log(ctx, AV_LOG_ERROR, "Failed to create pipeline configuration: "
>> +               "%d (%s).\n", vas, vaErrorStr(vas));
>> +        err = AVERROR(EINVAL);
>> +        goto fail_config;
>> +    }
>> +
>> +    if(input) {
>> +        ctx->input_surfaces = av_calloc(input->count, sizeof(AVVAAPISurface));
>> +        if(!ctx->input_surfaces) {
>> +            err = AVERROR(ENOMEM);
>> +            goto fail_alloc_input_surfaces;
>> +        }
>> +
>> +        err = vaapi_create_surfaces(ctx, input, ctx->input_surfaces,
>> +                                    ctx->input_surface_ids);
>> +        if(err)
>> +            goto fail_create_input_surfaces;
>> +        ctx->input = input;
>> +    } else {
>> +        av_log(ctx, AV_LOG_INFO, "No input surfaces.\n");
>> +        ctx->input = 0;
>> +    }
>> +
>> +    if(output) {
>> +        ctx->output_surfaces = av_calloc(output->count, sizeof(AVVAAPISurface));
>> +        if(!ctx->output_surfaces) {
>> +            err = AVERROR(ENOMEM);
>> +            goto fail_alloc_output_surfaces;
>> +        }
>> +
>> +        err = vaapi_create_surfaces(ctx, output, ctx->output_surfaces,
>> +                                    ctx->output_surface_ids);
>> +        if(err)
>> +            goto fail_create_output_surfaces;
>> +        ctx->output = output;
>> +    } else {
>> +        av_log(ctx, AV_LOG_INFO, "No output surfaces.\n");
>> +        ctx->output = 0;
>> +    }
>> +
>> +    vas = vaCreateContext(hw_ctx->display, ctx->config_id,
>> +                          output->width, output->height,
>> +                          VA_PROGRESSIVE,
>> +                          ctx->output_surface_ids, output->count,
>> +                          &ctx->context_id);
>> +    if(vas != VA_STATUS_SUCCESS) {
>> +        av_log(ctx, AV_LOG_ERROR, "Failed to create pipeline context: "
>> +               "%d (%s).\n", vas, vaErrorStr(vas));
>> +        err = AVERROR(EINVAL);
>> +        goto fail_context;
>> +    }
>> +
>> +    av_log(ctx, AV_LOG_INFO, "VAAPI pipeline initialised: config %#x "
>> +           "context %#x.\n", ctx->config_id, ctx->context_id);
>> +    if(input)
>> +        av_log(ctx, AV_LOG_INFO, "  Input: %u surfaces of %ux%u.\n",
>> +               input->count, input->width, input->height);
>> +    if(output)
>> +        av_log(ctx, AV_LOG_INFO, "  Output: %u surfaces of %ux%u.\n",
>> +               output->count, output->width, output->height);
>> +
>> +    return 0;
>> +
>> +  fail_context:
>> +    vaapi_destroy_surfaces(ctx, output, ctx->output_surfaces,
>> +                           ctx->output_surface_ids);
>> +  fail_create_output_surfaces:
>> +    av_freep(&ctx->output_surfaces);
>> +  fail_alloc_output_surfaces:
>> +    vaapi_destroy_surfaces(ctx, input, ctx->input_surfaces,
>> +                           ctx->input_surface_ids);
>> +  fail_create_input_surfaces:
>> +    av_freep(&ctx->input_surfaces);
>> +  fail_alloc_input_surfaces:
>> +    vaDestroyConfig(hw_ctx->display, ctx->config_id);
>> +    if(vas != VA_STATUS_SUCCESS) {
>> +        av_log(ctx, AV_LOG_ERROR, "Failed to destroy pipeline "
>> +               "configuration: %d (%s).\n", vas, vaErrorStr(vas));
>> +    }
>> +  fail_config:
>> +    return err;
>> +}
>> +
>> +int ff_vaapi_pipeline_uninit(AVVAAPIPipelineContext *ctx)
>> +{
>> +    VAStatus vas;
>> +
>> +    av_assert0(ctx->hardware_context);
>> +    av_assert0(ctx->config);
>> +
>> +    vas = vaDestroyContext(ctx->hardware_context->display, ctx->context_id);
>> +    if(vas != VA_STATUS_SUCCESS) {
>> +        av_log(ctx, AV_LOG_ERROR, "Failed to destroy pipeline context: "
>> +               "%d (%s).\n", vas, vaErrorStr(vas));
>> +    }
>> +
>> +    if(ctx->output) {
>> +        vaapi_destroy_surfaces(ctx, ctx->output, ctx->output_surfaces,
>> +                               ctx->output_surface_ids);
>> +        av_freep(&ctx->output_surfaces);
>> +    }
>> +
>> +    if(ctx->input) {
>> +        vaapi_destroy_surfaces(ctx, ctx->input, ctx->input_surfaces,
>> +                               ctx->input_surface_ids);
>> +        av_freep(&ctx->input_surfaces);
>> +    }
>> +
>> +    vaDestroyConfig(ctx->hardware_context->display, ctx->config_id);
>> +    if(vas != VA_STATUS_SUCCESS) {
>> +        av_log(ctx, AV_LOG_ERROR, "Failed to destroy pipeline configuration: "
>> +               "%d (%s).\n", vas, vaErrorStr(vas));
>> +    }
>> +
>> +    return 0;
>> +}
>> +
>> +static void vaapi_codec_release_surface(void *opaque, uint8_t *data)
>> +{
>> +    AVVAAPISurface *surface = opaque;
>> +
>> +    av_assert0(surface->refcount > 0);
>> +    --surface->refcount;
>> +}
> 
> Maybe that's just me, but I think vaapi surfaces shouldn't break what
> is guaranteed by AVFrame:
> 
> - unreffing them is thread-safe
> - unreffing the last AVFrame will also unref whatever manages the
>   surfaces (or alternatively, if the vaapi state is not refcounted, kill
>   the process if there are still AVFrames referencing the vaapi state,
>   as these AVFrames would essentially have dangling pointers)
> 
> That's a bit more complex, but IMO worth it.

Unref being thread safe is a bit annoying because of the locking - if the lock were recursive then it could work, but that pushes me into pthreadland to do it.  Is that ok?

I'm not sure what you mean by the second point.  What do you want to also go when the last AVFrame gets unreferenced?

I know the AVFrame setup is all a bit dubious because I still don't really "get" how AVFrames are meant to be used.  I will think about it further.

>> +
>> +static int vaapi_get_surface(AVVAAPIPipelineContext *ctx,
>> +                             AVVAAPISurfaceConfig *config,
>> +                             AVVAAPISurface *surfaces, AVFrame *frame)
>> +{
>> +    AVVAAPISurface *surface;
>> +    int i;
>> +
>> +    for(i = 0; i < config->count; i++) {
>> +        if(surfaces[i].refcount == 0)
>> +            break;
>> +    }
>> +    if(i >= config->count) {
>> +        av_log(ctx, AV_LOG_ERROR, "Failed to allocate surface "
>> +               "(%d in use).\n", config->count);
>> +        return AVERROR(ENOMEM);
>> +    }
>> +    surface = &surfaces[i];
>> +
>> +    ++surface->refcount;
>> +    frame->data[3] = (uint8_t*)(uintptr_t)surface->id;
>> +    frame->buf[0] = av_buffer_create((uint8_t*)surface, 0,
>> +                                     &vaapi_codec_release_surface,
>> +                                     surface, AV_BUFFER_FLAG_READONLY);
>> +    if(!frame->buf[0]) {
>> +        av_log(ctx, AV_LOG_ERROR, "Failed to allocate dummy buffer "
>> +               "for surface %#x.\n", surface->id);
>> +        return AVERROR(ENOMEM);
>> +    }
>> +
>> +    frame->format = AV_PIX_FMT_VAAPI;
>> +    frame->width  = config->width;
>> +    frame->height = config->height;
>> +
>> +    return 0;
>> +}
>> +
>> +int ff_vaapi_get_input_surface(AVVAAPIPipelineContext *ctx, AVFrame *frame)
>> +{
>> +    return vaapi_get_surface(ctx, ctx->input, ctx->input_surfaces, frame);
>> +}
>> +
>> +int ff_vaapi_get_output_surface(AVVAAPIPipelineContext *ctx, AVFrame *frame)
>> +{
>> +    return vaapi_get_surface(ctx, ctx->output, ctx->output_surfaces, frame);
>> +}
> 
> I wonder if vaapi_get_surface() could instead be some sort of
> independent frame pool? (But maybe the way you did is already elegant
> wrt. how vaapi works?)

The only trickiness here is that you have to declare all of the surfaces you are going to use for output when you create the pipeline context.

So yes, probably, if I understood AVFrames better...  Will consider along with the previous point.

>> +
>> +
>> +int ff_vaapi_map_surface(AVVAAPISurface *surface, int get)
>> +{
>> +    AVVAAPIHardwareContext *hw_ctx = surface->hardware_context;
>> +    AVVAAPISurfaceConfig *config = surface->config;
>> +    VAStatus vas;
>> +    int err;
>> +    void *address;
>> +    // On current Intel drivers, derive gives you memory which is very slow
>> +    // to read (uncached?).  It can be better for write-only cases, but for
>> +    // now play it safe and never use derive.
>> +    int derive = 0;
> 
> You need a GPU-optimized memcpy.

Yes.  Speeding up these operations was on my list of things to look at later.

>> +
>> +    vas = vaSyncSurface(hw_ctx->display, surface->id);
>> +    if(vas != VA_STATUS_SUCCESS) {
>> +        av_log(0, AV_LOG_ERROR, "Failed to sync surface "
>> +               "%#x: %d (%s).\n", surface->id, vas, vaErrorStr(vas));
>> +        err = AVERROR(EINVAL);
>> +        goto fail;
>> +    }
>> +
>> +    if(derive) {
>> +        vas = vaDeriveImage(hw_ctx->display,
>> +                            surface->id, &surface->image);
>> +        if(vas != VA_STATUS_SUCCESS) {
>> +            av_log(0, AV_LOG_ERROR, "Failed to derive image from surface "
>> +                   "%#x: %d (%s).\n", surface->id, vas, vaErrorStr(vas));
>> +            derive = 0;
>> +        }
>> +    }
>> +    if(!derive) {
>> +        vas = vaCreateImage(hw_ctx->display,
>> +                            &config->image_format,
>> +                            config->width, config->height,
>> +                            &surface->image);
>> +        if(vas != VA_STATUS_SUCCESS) {
>> +            av_log(0, AV_LOG_ERROR, "Failed to create image for surface "
>> +                   "%#x: %d (%s).\n", surface->id, vas, vaErrorStr(vas));
>> +            err = AVERROR(EINVAL);
>> +            goto fail;
>> +        }
>> +
>> +        if(get) {
>> +            vas = vaGetImage(hw_ctx->display,
>> +                             surface->id, 0, 0,
>> +                             config->width, config->height,
>> +                             surface->image.image_id);
>> +            if(vas != VA_STATUS_SUCCESS) {
>> +                av_log(0, AV_LOG_ERROR, "Failed to get image for surface "
>> +                       "%#x: %d (%s).\n", surface->id, vas, vaErrorStr(vas));
>> +                err = AVERROR(EINVAL);
>> +                goto fail_image;
>> +            }
>> +        }
>> +    }
>> +
>> +    av_assert0(surface->image.format.fourcc == config->image_format.fourcc);
>> +
>> +    vas = vaMapBuffer(hw_ctx->display,
>> +                      surface->image.buf, &address);
>> +    if(vas != VA_STATUS_SUCCESS) {
>> +        av_log(0, AV_LOG_ERROR, "Failed to map image from surface "
>> +               "%#x: %d (%s).\n", surface->id, vas, vaErrorStr(vas));
>> +        err = AVERROR(EINVAL);
>> +        goto fail_image;
>> +    }
>> +
>> +    surface->mapped_address = address;
>> +
>> +    return 0;
>> +
>> +  fail_image:
>> +    vas = vaDestroyImage(hw_ctx->display, surface->image.image_id);
>> +    if(vas != VA_STATUS_SUCCESS) {
>> +        av_log(0, AV_LOG_ERROR, "Failed to destroy image for surface "
>> +               "%#x: %d (%s).\n", surface->id, vas, vaErrorStr(vas));
>> +    }
>> +  fail:
>> +    return err;
>> +}
>> +
>> +int ff_vaapi_unmap_surface(AVVAAPISurface *surface, int put)
>> +{
>> +    AVVAAPIHardwareContext *hw_ctx = surface->hardware_context;
>> +    AVVAAPISurfaceConfig *config = surface->config;
>> +    VAStatus vas;
>> +    int derive = 0;
>> +
>> +    surface->mapped_address = 0;
>> +
>> +    vas = vaUnmapBuffer(hw_ctx->display,
>> +                        surface->image.buf);
>> +    if(vas != VA_STATUS_SUCCESS) {
>> +        av_log(0, AV_LOG_ERROR, "Failed to unmap image from surface "
>> +               "%#x: %d (%s).\n", surface->id, vas, vaErrorStr(vas));
>> +    }
>> +
>> +    if(!derive && put) {
>> +        vas = vaPutImage(hw_ctx->display, surface->id,
>> +                         surface->image.image_id,
>> +                         0, 0, config->width, config->height,
>> +                         0, 0, config->width, config->height);
>> +        if(vas != VA_STATUS_SUCCESS) {
>> +            av_log(0, AV_LOG_ERROR, "Failed to put image for surface "
>> +                   "%#x: %d (%s).\n", surface->id, vas, vaErrorStr(vas));
>> +        }
>> +    }
>> +
>> +    vas = vaDestroyImage(hw_ctx->display,
>> +                         surface->image.image_id);
>> +    if(vas != VA_STATUS_SUCCESS) {
>> +        av_log(0, AV_LOG_ERROR, "Failed to destroy image for surface "
>> +               "%#x: %d (%s).\n", surface->id, vas, vaErrorStr(vas));
>> +    }
>> +
>> +    return 0;
>> +}
>> +
>> +int ff_vaapi_copy_to_surface(const AVFrame *f, AVVAAPISurface *surface)
>> +{
>> +    VAImage *image = &surface->image;
>> +    char *data = surface->mapped_address;
>> +    av_assert0(data);
>> +
>> +    switch(f->format) {
>> +
>> +    case AV_PIX_FMT_YUV420P:
>> +        av_assert0(image->format.fourcc == VA_FOURCC_YV12);
>> +        av_image_copy_plane(data + image->offsets[0], image->pitches[0],
>> +                            f->data[0], f->linesize[0],
>> +                            f->width, f->height);
>> +        av_image_copy_plane(data + image->offsets[1], image->pitches[1],
>> +                            f->data[2], f->linesize[2],
>> +                            f->width / 2, f->height / 2);
>> +        av_image_copy_plane(data + image->offsets[2], image->pitches[2],
>> +                            f->data[1], f->linesize[1],
>> +                            f->width / 2, f->height / 2);
>> +        break;
>> +
>> +    case AV_PIX_FMT_NV12:
>> +        av_assert0(image->format.fourcc == VA_FOURCC_NV12);
>> +        av_image_copy_plane(data + image->offsets[0], image->pitches[0],
>> +                            f->data[0], f->linesize[0],
>> +                            f->width, f->height);
>> +        av_image_copy_plane(data + image->offsets[1], image->pitches[1],
>> +                            f->data[1], f->linesize[1],
>> +                            f->width, f->height / 2);
>> +        break;
>> +
>> +    case AV_PIX_FMT_BGR0:
>> +        av_assert0(image->format.fourcc == VA_FOURCC_BGRX);
>> +        av_image_copy_plane(data + image->offsets[0], image->pitches[0],
>> +                            f->data[0], f->linesize[0],
>> +                            f->width * 4, f->height);
>> +        break;
>> +
>> +    default:
>> +        return AVERROR(EINVAL);
>> +    }
>> +
>> +    return 0;
>> +}
>> +
>> +int ff_vaapi_copy_from_surface(AVFrame *f, AVVAAPISurface *surface)
>> +{
>> +    VAImage *image = &surface->image;
>> +    char *data = surface->mapped_address;
>> +    av_assert0(data);
>> +
>> +    switch(f->format) {
>> +
>> +    case AV_PIX_FMT_YUV420P:
>> +        av_assert0(image->format.fourcc == VA_FOURCC_YV12);
>> +        av_image_copy_plane(f->data[0], f->linesize[0],
>> +                            data + image->offsets[0], image->pitches[0],
>> +                            f->width, f->height);
>> +        // Um, apparently these are not the same way round...
> 
> While we're at it, there is also VA_FOURCC_IYUV, which is YV12 with
> swapped chroma planes. Don't know if it's still in use anywhere.

Which is presumably what this supposedly-YV12 format actually is, hence the unexpected swap (yay Intel).  I'll just add that because it clarifies what's going on.

>> +        av_image_copy_plane(f->data[2], f->linesize[2],
>> +                            data + image->offsets[1], image->pitches[1],
>> +                            f->width / 2, f->height / 2);
>> +        av_image_copy_plane(f->data[1], f->linesize[1],
>> +                            data + image->offsets[2], image->pitches[2],
>> +                            f->width / 2, f->height / 2);
>> +        break;
>> +
>> +    case AV_PIX_FMT_NV12:
>> +        av_assert0(image->format.fourcc == VA_FOURCC_NV12);
>> +        av_image_copy_plane(f->data[0], f->linesize[0],
>> +                            data + image->offsets[0], image->pitches[0],
>> +                            f->width, f->height);
>> +        av_image_copy_plane(f->data[1], f->linesize[1],
>> +                            data + image->offsets[1], image->pitches[1],
>> +                            f->width, f->height / 2);
>> +        break;
>> +
>> +    default:
>> +        return AVERROR(EINVAL);
>> +    }
>> +
>> +    return 0;
>> +}
> 
> It might be better to setup a temporary AVFrame to point to the
> VAImage's data, and then use av_frame_copy().

Yeah, ok.  I was thinking cleverer things might be able to happen here, but I guess that really that should be the responsibility of the user.

(Doing/undoing the interleave to convert YV12 <-> NV12 in this step was the obvious one I was thinking of.)

>> diff --git a/libavutil/vaapi.h b/libavutil/vaapi.h
>> new file mode 100644
>> index 0000000..3bbae6e
>> --- /dev/null
>> +++ b/libavutil/vaapi.h
>> @@ -0,0 +1,123 @@
>> +/*
>> + * VAAPI helper functions.
>> + *
>> + * Copyright (C) 2016 Mark Thompson <mrt at jkqxz.net>
>> + *
>> + * 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
>> + */
>> +
>> +#ifndef LIBAVUTIL_VAAPI_H_
>> +#define LIBAVUTIL_VAAPI_H_
>> +
>> +#include <va/va.h>
>> +
>> +#include "pixfmt.h"
>> +#include "frame.h"
>> +
>> +
>> +typedef struct AVVAAPIHardwareContext {
>> +    VADisplay display;
>> +
>> +    VAConfigID  decoder_pipeline_config_id;
>> +    VAContextID decoder_pipeline_context_id;
>> +
>> +    void (*lock)(void);
>> +    void (*unlock)(void);
>> +} AVVAAPIHardwareContext;
>> +
>> +void av_vaapi_lock_hardware_context(AVVAAPIHardwareContext *ctx);
>> +void av_vaapi_unlock_hardware_context(AVVAAPIHardwareContext *ctx);
>> +
>> +
>> +
>> +// Everything below this point does not constitute a public API.
> 
> Then why is it not in a private header?

Right, I didn't think of that at all.

It kindof wants to be a public API (if users want to do anything interesting with VAAPI beyond a vanilla encode or decode, they want these functions or will reimplement them), but I wasn't confident
enough that it has the right form to fix forever to actually declare it as such initially.

>> +
>> +
>> +#define AV_VAAPI_MAX_SURFACES 64
>> +
>> +
>> +typedef struct AVVAAPISurfaceConfig {
>> +    enum AVPixelFormat av_format;
>> +    unsigned int rt_format;
>> +    VAImageFormat image_format;
>> +
>> +    unsigned int count;
>> +    unsigned int width;
>> +    unsigned int height;
>> +
>> +    unsigned int attribute_count;
>> +    VASurfaceAttrib *attributes;
>> +} AVVAAPISurfaceConfig;
>> +
>> +typedef struct AVVAAPISurface {
>> +    VASurfaceID id;
>> +    int refcount;
>> +
>> +    VAImage image;
>> +    void *mapped_address;
>> +
>> +    AVVAAPIHardwareContext *hardware_context;
>> +    AVVAAPISurfaceConfig *config;
>> +} AVVAAPISurface;
>> +
>> +
>> +typedef struct AVVAAPIPipelineConfig {
>> +    VAProfile profile;
>> +    VAEntrypoint entrypoint;
>> +
>> +    unsigned int attribute_count;
>> +    VAConfigAttrib *attributes;
>> +} AVVAAPIPipelineConfig;
>> +
>> +typedef struct AVVAAPIPipelineContext {
>> +    const AVClass *class;
>> +
>> +    AVVAAPIHardwareContext *hardware_context;
>> +    AVVAAPIPipelineConfig *config;
>> +    AVVAAPISurfaceConfig *input;
>> +    AVVAAPISurfaceConfig *output;
>> +
>> +    VAConfigID config_id;
>> +    VAContextID context_id;
>> +
>> +    AVVAAPISurface *input_surfaces;
>> +    VASurfaceID input_surface_ids[AV_VAAPI_MAX_SURFACES];
>> +
>> +    AVVAAPISurface *output_surfaces;
>> +    VASurfaceID output_surface_ids[AV_VAAPI_MAX_SURFACES];
>> +} AVVAAPIPipelineContext;
>> +
>> +
>> +int ff_vaapi_pipeline_init(AVVAAPIPipelineContext *ctx,
>> +                           AVVAAPIHardwareContext *hw_ctx,
>> +                           AVVAAPIPipelineConfig *config,
>> +                           AVVAAPISurfaceConfig *input,
>> +                           AVVAAPISurfaceConfig *output);
>> +int ff_vaapi_pipeline_uninit(AVVAAPIPipelineContext *ctx);
>> +
>> +int ff_vaapi_get_input_surface(AVVAAPIPipelineContext *ctx, AVFrame *frame);
>> +int ff_vaapi_get_output_surface(AVVAAPIPipelineContext *ctx, AVFrame *frame);
>> +
>> +int ff_vaapi_map_surface(AVVAAPISurface *surface, int get);
>> +int ff_vaapi_unmap_surface(AVVAAPISurface *surface, int put);
>> +
>> +
>> +int ff_vaapi_copy_to_surface(const AVFrame *f, AVVAAPISurface *surface);
>> +int ff_vaapi_copy_from_surface(AVFrame *f, AVVAAPISurface *surface);
>> +
>> +
>> +#endif /* LIBAVUTIL_VAAPI_H_ */
> 



More information about the ffmpeg-devel mailing list