[FFmpeg-devel] [PATCH v4 2/5] ffmpeg: VAAPI hwaccel helper and related initialisation

Mark Thompson sw at jkqxz.net
Sun Jan 24 15:20:54 CET 2016


On 24/01/16 12:42, wm4 wrote:
> On Sat, 23 Jan 2016 19:14:29 +0000
> Mark Thompson <sw at jkqxz.net> wrote:
> 
>> ---
>>  Makefile       |   1 +
>>  configure      |   5 +
>>  ffmpeg.c       |   6 +
>>  ffmpeg.h       |   5 +
>>  ffmpeg_opt.c   |  16 ++
>>  ffmpeg_vaapi.c | 633 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>  6 files changed, 666 insertions(+)
>>  create mode 100644 ffmpeg_vaapi.c
>>
>> diff --git a/Makefile b/Makefile
>> index f3bd5f6..4cccffd 100644
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -37,6 +37,7 @@ OBJS-ffmpeg-$(CONFIG_VDA)     += ffmpeg_videotoolbox.o
>>  endif
>>  OBJS-ffmpeg-$(CONFIG_VIDEOTOOLBOX) += ffmpeg_videotoolbox.o
>>  OBJS-ffmpeg-$(CONFIG_LIBMFX)  += ffmpeg_qsv.o
>> +OBJS-ffmpeg-$(CONFIG_VAAPI_RECENT) += ffmpeg_vaapi.o
>>  OBJS-ffserver                 += ffserver_config.o
>>
>>  TESTTOOLS   = audiogen videogen rotozoom tiny_psnr tiny_ssim base64
>> diff --git a/configure b/configure
>> index cd386b4..791334f 100755
>> --- a/configure
>> +++ b/configure
>> @@ -1971,6 +1971,7 @@ HAVE_LIST="
>>      section_data_rel_ro
>>      texi2html
>>      threads
>> +    vaapi_drm
>>      vaapi_x11
>>      vdpau_x11
>>      xlib
>> @@ -5749,6 +5750,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/ffmpeg.c b/ffmpeg.c
>> index 97aca10..5439761 100644
>> --- a/ffmpeg.c
>> +++ b/ffmpeg.c
>> @@ -2603,6 +2603,12 @@ static int init_output_stream(OutputStream *ost, char *error, int error_len)
>>              !av_dict_get(ost->encoder_opts, "ab", NULL, 0))
>>              av_dict_set(&ost->encoder_opts, "b", "128000", 0);
>>
>> +#if CONFIG_VAAPI_RECENT
>> +        if(ost->enc->type == AVMEDIA_TYPE_VIDEO &&
>> +           strstr(ost->enc->name, "vaapi"))
>> +            vaapi_hardware_set_options(&ost->encoder_opts);
>> +#endif
>> +
>>          if ((ret = avcodec_open2(ost->enc_ctx, codec, &ost->encoder_opts)) < 0) {
>>              if (ret == AVERROR_EXPERIMENTAL)
>>                  abort_codec_experimental(codec, 1);
>> diff --git a/ffmpeg.h b/ffmpeg.h
>> index 20322b0..2134213 100644
>> --- a/ffmpeg.h
>> +++ b/ffmpeg.h
>> @@ -65,6 +65,7 @@ enum HWAccelID {
>>      HWACCEL_VDA,
>>      HWACCEL_VIDEOTOOLBOX,
>>      HWACCEL_QSV,
>> +    HWACCEL_VAAPI,
>>  };
>>
>>  typedef struct HWAccel {
>> @@ -577,5 +578,9 @@ int vda_init(AVCodecContext *s);
>>  int videotoolbox_init(AVCodecContext *s);
>>  int qsv_init(AVCodecContext *s);
>>  int qsv_transcode_init(OutputStream *ost);
>> +int vaapi_decode_init(AVCodecContext *s);
>> +
>> +int vaapi_hardware_init(const char *device);
>> +int vaapi_hardware_set_options(AVDictionary **dict);
>>
>>  #endif /* FFMPEG_H */
>> diff --git a/ffmpeg_opt.c b/ffmpeg_opt.c
>> index 9b341cf..fd41600 100644
>> --- a/ffmpeg_opt.c
>> +++ b/ffmpeg_opt.c
>> @@ -82,6 +82,9 @@ const HWAccel hwaccels[] = {
>>  #if CONFIG_LIBMFX
>>      { "qsv",   qsv_init,   HWACCEL_QSV,   AV_PIX_FMT_QSV },
>>  #endif
>> +#if CONFIG_VAAPI_RECENT
>> +    { "vaapi", vaapi_decode_init, HWACCEL_VAAPI, AV_PIX_FMT_VAAPI },
>> +#endif
>>      { 0 },
>>  };
>>
>> @@ -442,6 +445,15 @@ static int opt_sdp_file(void *optctx, const char *opt, const char *arg)
>>      return 0;
>>  }
>>
>> +#if CONFIG_VAAPI_RECENT
>> +static int opt_vaapi(void *optctx, const char *opt, const char *arg)
>> +{
>> +    if(vaapi_hardware_init(arg))
>> +        exit_program(1);
>> +    return 0;
>> +}
>> +#endif
>> +
>>  /**
>>   * Parse a metadata specifier passed as 'arg' parameter.
>>   * @param arg  metadata string to parse
>> @@ -3438,5 +3450,9 @@ const OptionDef options[] = {
>>      { "dn", OPT_BOOL | OPT_VIDEO | OPT_OFFSET | OPT_INPUT | OPT_OUTPUT, { .off = OFFSET(data_disable) },
>>          "disable data" },
>>
>> +#if CONFIG_VAAPI_RECENT
>> +    { "vaapi", HAS_ARG, { .func_arg = opt_vaapi }, "set VAAPI hardware context" },
>> +#endif
>> +
>>      { NULL, },
>>  };
>> diff --git a/ffmpeg_vaapi.c b/ffmpeg_vaapi.c
>> new file mode 100644
>> index 0000000..9a71cc2
>> --- /dev/null
>> +++ b/ffmpeg_vaapi.c
>> @@ -0,0 +1,633 @@
>> +/*
>> + * VAAPI helper for hardware-accelerated decoding.
>> + *
>> + * 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 <string.h>
>> +
>> +#include <fcntl.h>
>> +#include <pthread.h>
>> +#include <unistd.h>
>> +
>> +#include "ffmpeg.h"
>> +
>> +#include "libavutil/avassert.h"
>> +#include "libavutil/avconfig.h"
>> +#include "libavutil/buffer.h"
>> +#include "libavutil/frame.h"
>> +#include "libavutil/imgutils.h"
>> +#include "libavutil/opt.h"
>> +#include "libavutil/pixfmt.h"
>> +#include "libavutil/vaapi.h"
>> +
>> +#include "libavcodec/vaapi.h"
>> +
>> +#include <va/va_x11.h>
>> +#include <va/va_drm.h>
>> +
>> +
>> +static AVClass vaapi_class = {
>> +    .class_name = "VAAPI/driver",
>> +    .item_name  = av_default_item_name,
>> +    .version    = LIBAVUTIL_VERSION_INT,
>> +};
>> +
>> +
>> +#define DEFAULT_SURFACES 20
>> +
>> +typedef struct VAAPIDecoderContext {
>> +    const AVClass *class;
>> +
>> +    AVVAAPIHardwareContext *hardware_context;
>> +    AVVAAPIPipelineConfig config;
>> +    AVVAAPIPipelineContext codec;
>> +    AVVAAPISurfaceConfig output;
>> +    AVVAAPISurfacePool pool;
>> +
>> +    int codec_initialised;
>> +} VAAPIDecoderContext;
>> +
>> +
>> +static int vaapi_get_buffer(AVCodecContext *s, AVFrame *frame, int flags)
>> +{
>> +    InputStream *ist = s->opaque;
>> +    VAAPIDecoderContext *ctx = ist->hwaccel_ctx;
>> +    AVFrame *new_frame;
>> +
>> +    av_assert0(frame->format == AV_PIX_FMT_VAAPI);
> 
> This can actually change on software decoding fallback, I think?

But this function won't be called in that case?

It works on the switching cases in FATE, at least: h264-reinit-small_420_8-to-large_444_10 and h264-reinit-small_420_9-to-small_420_8 both pass.

>> +
>> +    new_frame = av_frame_alloc();
>> +    if(!new_frame)
>> +        return AVERROR(ENOMEM);
>> +
>> +    av_vaapi_surface_pool_get(&ctx->pool, new_frame);
>> +
>> +    av_log(ctx, AV_LOG_DEBUG, "Decoder given reference to surface %#x.\n",
>> +           (VASurfaceID)new_frame->data[3]);
>> +
>> +    av_frame_copy_props(new_frame, frame);
>> +    av_frame_unref(frame);
>> +    av_frame_move_ref(frame, new_frame);
> 
> What are these acrobatics for? Why not use frame directly?

The AVFrame supplied by the decoder already has a lot of the metadata filled in, but the surface pool needs to make a new reference to its own AVFrame.

Without this, you get display order == decode order (that was fun to track down).

>> +    return 0;
>> +}
>> +
>> +static int vaapi_retrieve_data(AVCodecContext *avctx, AVFrame *input_frame)
>> +{
>> +    InputStream *ist = avctx->opaque;
>> +    VAAPIDecoderContext *ctx = ist->hwaccel_ctx;
>> +    AVVAAPISurfaceConfig *output = &ctx->output;
>> +    AVFrame *output_frame;
>> +    int err;
>> +
>> +    av_log(ctx, AV_LOG_DEBUG, "Decoder output in surface %#x.\n",
>> +           (VASurfaceID)input_frame->data[3]);
>> +
>> +    if(output->av_format == AV_PIX_FMT_VAAPI) {
>> +        // Nothing to do.
>> +        return 0;
>> +    }
>> +
>> +    output_frame = av_frame_alloc();
>> +    if(!output_frame)
>> +        return AVERROR(ENOMEM);
>> +
>> +    output_frame->format = output->av_format;
>> +    output_frame->width  = input_frame->width;
>> +    output_frame->height = input_frame->height;
>> +
>> +    err = av_frame_get_buffer(output_frame, 32);
>> +    if(err) {
>> +        av_log(ctx, AV_LOG_ERROR, "Failed to get buffer for output frame: "
>> +               "%d (%s).\n", err, av_err2str(err));
>> +        return err;
>> +    }
>> +
>> +    err = av_vaapi_copy_from_surface(output_frame, input_frame);
>> +    if(err) {
>> +        av_log(ctx, AV_LOG_ERROR, "Failed to copy from output surface: "
>> +               "%d (%s).\n", err, av_err2str(err));
>> +        return err;
>> +    }
>> +
>> +    av_frame_copy_props(output_frame, input_frame);
>> +    av_frame_unref(input_frame);
>> +    av_frame_move_ref(input_frame, output_frame);
>> +    return 0;
>> +}
>> +
>> +
>> +static const struct {
>> +    enum AVCodecID codec_id;
>> +    int codec_profile;
>> +    VAProfile va_profile;
>> +} vaapi_profile_map[] = {
>> +#define MAP(c, p, v) { AV_CODEC_ID_ ## c, FF_PROFILE_ ## p, VAProfile ## v }
>> +    MAP(MPEG2VIDEO,  MPEG2_SIMPLE,    MPEG2Simple ),
>> +    MAP(MPEG2VIDEO,  MPEG2_MAIN,      MPEG2Main   ),
>> +    MAP(H263,        UNKNOWN,         H263Baseline),
>> +    MAP(MPEG4,       MPEG4_SIMPLE,    MPEG4Simple ),
>> +    MAP(MPEG4,       MPEG4_ADVANCED_SIMPLE,
>> +                               MPEG4AdvancedSimple),
>> +    MAP(MPEG4,       MPEG4_MAIN,      MPEG4Main   ),
>> +    MAP(H264,        H264_CONSTRAINED_BASELINE,
>> +                           H264ConstrainedBaseline),
>> +    MAP(H264,        H264_BASELINE,   H264Baseline),
>> +    MAP(H264,        H264_MAIN,       H264Main    ),
>> +    MAP(H264,        H264_HIGH,       H264High    ),
>> +    MAP(HEVC,        HEVC_MAIN,       HEVCMain    ),
>> +    MAP(WMV3,        VC1_SIMPLE,      VC1Simple   ),
>> +    MAP(WMV3,        VC1_MAIN,        VC1Main     ),
>> +    MAP(WMV3,        VC1_COMPLEX,     VC1Advanced ),
>> +    MAP(WMV3,        VC1_ADVANCED,    VC1Advanced ),
>> +    MAP(VC1,         VC1_SIMPLE,      VC1Simple   ),
>> +    MAP(VC1,         VC1_MAIN,        VC1Main     ),
>> +    MAP(VC1,         VC1_COMPLEX,     VC1Advanced ),
>> +    MAP(VC1,         VC1_ADVANCED,    VC1Advanced ),
>> +    MAP(MJPEG,       UNKNOWN,         JPEGBaseline),
>> +    MAP(VP8,         UNKNOWN,        VP8Version0_3),
>> +    MAP(VP9,         VP9_0,           VP9Profile0 ),
>> +#undef MAP
>> +};
>> +
>> +static VAProfile vaapi_find_profile(const AVCodecContext *avctx)
>> +{
>> +    VAProfile result = VAProfileNone;
>> +    int i;
>> +    for(i = 0; i < FF_ARRAY_ELEMS(vaapi_profile_map); i++) {
>> +        if(avctx->codec_id != vaapi_profile_map[i].codec_id)
>> +            continue;
>> +        result = vaapi_profile_map[i].va_profile;
>> +        if(avctx->profile == vaapi_profile_map[i].codec_profile)
>> +            break;
>> +        // If there isn't an exact match, we will choose the last (highest)
>> +        // profile in the mapping table.  This is unfortunate because it
>> +        // could well be wrong, but it improves compatibility because streams
>> +        // and decoders do not necessarily conform to the requirements.
>> +        // (Without this, a lot of streams in FATE are discarded when then
>> +        // could have worked - H.264 extended profile which actually decodes
>> +        // with main, for example.)
> 
> I still think this should not be done by default, and instead a user
> option should enable it.

I guess that works if the error message suggests the option they could try.  "-lax-codec-profile-constraints"?  Other hwaccels might want it too.

>> +    }
>> +    return result;
>> +}
>> +
>> +static const struct {
>> +    enum AVPixelFormat pix_fmt;
>> +    unsigned int fourcc;
>> +} vaapi_image_formats[] = {
>> +    { AV_PIX_FMT_NV12,    VA_FOURCC_NV12 },
>> +    { AV_PIX_FMT_YUV420P, VA_FOURCC_YV12 },
>> +    { AV_PIX_FMT_GRAY8,   VA_FOURCC_Y800 },
>> +};
>> +
>> +static int vaapi_get_pix_fmt(unsigned int fourcc)
>> +{
>> +    int i;
>> +    for(i = 0; i < FF_ARRAY_ELEMS(vaapi_image_formats); i++)
>> +        if(vaapi_image_formats[i].fourcc == fourcc)
>> +            return vaapi_image_formats[i].pix_fmt;
>> +    return 0;
>> +}
> 
> Again wondering why there's not just a single mapping table in the
> libavutil code. (Just a suggestion; this is ok too.)

Ok, that does make sense to go with the mapping now.  I'll change it.

>> +
>> +static int vaapi_build_decoder_config(VAAPIDecoderContext *ctx,
>> +                                      AVVAAPIPipelineConfig *config,
>> +                                      AVVAAPISurfaceConfig *output,
>> +                                      AVCodecContext *avctx)
>> +{
>> +    VAStatus vas;
>> +    int i;
>> +
>> +    memset(config, 0, sizeof(*config));
>> +
>> +    // Pick codec profile to use.
>> +    {
>> +        VAProfile profile;
>> +        int profile_count;
>> +        VAProfile *profile_list;
>> +
>> +        profile = vaapi_find_profile(avctx);
>> +        if(profile == VAProfileNone) {
>> +            av_log(ctx, AV_LOG_ERROR, "VAAPI does not support codec %s.\n",
>> +                   avcodec_get_name(avctx->codec_id));
>> +            return AVERROR(EINVAL);
>> +        }
>> +
>> +        profile_count = vaMaxNumProfiles(ctx->hardware_context->display);
>> +        profile_list = av_calloc(profile_count, sizeof(VAProfile));
>> +        if(!profile_list)
>> +            return AVERROR(ENOMEM);
>> +
>> +        vas = vaQueryConfigProfiles(ctx->hardware_context->display,
>> +                                    profile_list, &profile_count);
>> +        if(vas != VA_STATUS_SUCCESS) {
>> +            av_log(ctx, AV_LOG_ERROR, "Failed to query profiles: %d (%s).\n",
>> +                   vas, vaErrorStr(vas));
>> +            av_free(profile_list);
>> +            return AVERROR(EINVAL);
>> +        }
>> +
>> +        while(1) {
>> +            for(i = 0; i < profile_count; i++) {
>> +                if(profile_list[i] == profile)
>> +                    break;
>> +            }
>> +            if(i < profile_count)
>> +                break;
>> +
>> +            if(profile == VAProfileH264Baseline ||
>> +               profile == VAProfileH264ConstrainedBaseline) {
>> +                // Promote both of these to improve compatibility.
>> +                profile = VAProfileH264Main;
>> +                continue;
>> +            }
>> +
>> +            profile = VAProfileNone;
>> +            break;
>> +        }
>> +
>> +        av_free(profile_list);
>> +
>> +        if(profile == VAProfileNone) {
>> +            av_log(ctx, AV_LOG_ERROR, "Hardware does not support codec: "
>> +                   "%s / %d.\n", avcodec_get_name(avctx->codec_id),
>> +                   avctx->profile);
>> +            return AVERROR(EINVAL);
>> +        } else {
>> +            av_log(ctx, AV_LOG_DEBUG, "Hardware supports codec: "
>> +                   "%s / %d -> VAProfile %d.\n",
>> +                   avcodec_get_name(avctx->codec_id), avctx->profile,
>> +                   profile);
>> +        }
>> +
>> +        config->profile = profile;
>> +        config->entrypoint = VAEntrypointVLD;
>> +    }
>> +
>> +    // Decide on the internal chroma format.
>> +    {
>> +        VAConfigAttrib attr;
>> +
>> +        // Currently the software only supports YUV420, so just make sure
>> +        // that the hardware we have does too.
>> +
>> +        memset(&attr, 0, sizeof(attr));
>> +        attr.type = VAConfigAttribRTFormat;
>> +        vas = vaGetConfigAttributes(ctx->hardware_context->display, config->profile,
>> +                                    VAEntrypointVLD, &attr, 1);
>> +        if(vas != VA_STATUS_SUCCESS) {
>> +            av_log(ctx, AV_LOG_ERROR, "Failed to fetch config attributes: "
>> +                   "%d (%s).\n", vas, vaErrorStr(vas));
>> +            return AVERROR(EINVAL);
>> +        }
>> +        if(!(attr.value & VA_RT_FORMAT_YUV420)) {
>> +            av_log(ctx, AV_LOG_ERROR, "Hardware does not support required "
>> +                   "chroma format (%#x).\n", attr.value);
>> +            return AVERROR(EINVAL);
>> +        }
>> +
>> +        output->rt_format = VA_RT_FORMAT_YUV420;
> 
> I'm confused, shouldn't this set it to something retrieved by the
> function above?

Read again - it's just a check, the code errors out on all other possibilities.

This section is only present because more could be added.  Currently the only other possibility in the Intel driver is greyscale for H.264 decoding only, so it didn't seem worth it now.

>> +    }
>> +
>> +    // Decide on the image format.
>> +    if(avctx->pix_fmt == AV_PIX_FMT_VAAPI) {
>> +        // We are going to be passing through a VAAPI surface directly:
>> +        // they will stay as whatever opaque internal format for that time,
>> +        // and we never need to make VAImages from them.
>> +
>> +        av_log(ctx, AV_LOG_DEBUG, "Using VAAPI opaque output format.\n");
>> +
>> +        output->av_format = AV_PIX_FMT_VAAPI;
>> +        memset(&output->image_format, 0, sizeof(output->image_format));
>> +
>> +    } else {
>> +        int image_format_count;
>> +        VAImageFormat *image_format_list;
>> +        int pix_fmt;
>> +
>> +        // We might want to force a change to the output format here
>> +        // if we are intending to use VADeriveImage?
>> +
>> +        image_format_count = vaMaxNumImageFormats(ctx->hardware_context->display);
>> +        image_format_list = av_calloc(image_format_count,
>> +                                      sizeof(VAImageFormat));
>> +        if(!image_format_list)
>> +            return AVERROR(ENOMEM);
>> +
>> +        vas = vaQueryImageFormats(ctx->hardware_context->display, image_format_list,
>> +                                  &image_format_count);
>> +        if(vas != VA_STATUS_SUCCESS) {
>> +            av_log(ctx, AV_LOG_ERROR, "Failed to query image formats: "
>> +                   "%d (%s).\n", vas, vaErrorStr(vas));
>> +            return AVERROR(EINVAL);
>> +        }
>> +
>> +        for(i = 0; i < image_format_count; i++) {
>> +            pix_fmt = vaapi_get_pix_fmt(image_format_list[i].fourcc);
>> +            if(pix_fmt == AV_PIX_FMT_NONE)
>> +                continue;
>> +            if(pix_fmt == avctx->pix_fmt)
>> +                break;
>> +        }
>> +        if(i < image_format_count) {
>> +            av_log(ctx, AV_LOG_DEBUG, "Using desired output format %s "
>> +                   "(%#x).\n", av_get_pix_fmt_name(pix_fmt),
>> +                   image_format_list[i].fourcc);
>> +        } else {
>> +            for(i = 0; i < image_format_count; i++) {
>> +                pix_fmt = vaapi_get_pix_fmt(image_format_list[i].fourcc);
>> +                if(pix_fmt != AV_PIX_FMT_NONE)
>> +                    break;
>> +            }
>> +            if(i >= image_format_count) {
>> +                av_log(ctx, AV_LOG_ERROR, "No supported output format found.\n");
>> +                av_free(image_format_list);
>> +                return AVERROR(EINVAL);
>> +            }
>> +            av_log(ctx, AV_LOG_DEBUG, "Using alternate output format %s "
>> +                   "(%#x).\n", av_get_pix_fmt_name(pix_fmt),
>> +                   image_format_list[i].fourcc);
>> +        }
>> +
>> +        output->av_format = pix_fmt;
>> +        memcpy(&output->image_format, &image_format_list[i],
>> +               sizeof(VAImageFormat));
>> +
>> +        av_free(image_format_list);
>> +    }
> 
> Seems to pick a format randomly (from the unordered image_format_list).
> Maybe it'd just be better to force NV12, and if that doesn't work,
> retry with yuv420p, or so.

It picks the format you asked for (avctx->pix_fmt), or if not present takes the first usable one on the list.  In practice you ask for YUV420P and probably get it, with NV12 otherwise.

>> +
>> +    // Decide how many reference frames we need.
>> +    {
>> +        // We should be able to do this in a more sensible way by looking
>> +        // at how many reference frames the input stream requires.
>> +        //output->count = DEFAULT_SURFACES;
>> +    }
>> +
>> +    // Test whether the width and height are within allowable limits.
>> +    {
>> +        // It would be nice if we could check this here before starting
>> +        // anything, but unfortunately we need an active VA config to test.
>> +        // Hence just copy.  If it isn't supproted, the pipeline
>> +        // initialisation below will fail below instead.
>> +        config->width  = output->width  = avctx->coded_width;
>> +        config->height = output->height = avctx->coded_height;
>> +    }
> 
> Forgot what the conclusion about this was last time.

Check later, and the pipeline initialisation will fail.  I forgot to add the explicit check (call vaQuerySurfaceAttributes()) to improve the message, I'll do that now.

>> +
>> +    return 0;
>> +}
>> +
>> +static void vaapi_decode_uninit(AVCodecContext *avctx)
>> +{
>> +    InputStream  *ist = avctx->opaque;
>> +    VAAPIDecoderContext *ctx = ist->hwaccel_ctx;
>> +
>> +    if(ctx) {
>> +        if(ctx->codec_initialised) {
>> +            av_vaapi_lock_hardware_context(ctx->hardware_context);
>> +            av_vaapi_surface_pool_uninit(&ctx->pool);
>> +            av_vaapi_pipeline_uninit(&ctx->codec);
>> +            av_vaapi_unlock_hardware_context(ctx->hardware_context);
>> +            ctx->codec_initialised = 0;
>> +        }
>> +
>> +        av_free(ctx);
>> +    }
>> +
>> +
>> +    ist->hwaccel_ctx           = 0;
>> +    ist->hwaccel_uninit        = 0;
>> +    ist->hwaccel_get_buffer    = 0;
>> +    ist->hwaccel_retrieve_data = 0;
>> +}
>> +
>> +
>> +static AVVAAPIHardwareContext *vaapi_context;
>> +
>> +int vaapi_decode_init(AVCodecContext *avctx)
>> +{
>> +    InputStream *ist = avctx->opaque;
>> +    VAAPIDecoderContext *ctx;
>> +    int err;
>> +
>> +    if(ist->hwaccel_id != HWACCEL_VAAPI)
>> +        return AVERROR(EINVAL);
>> +
>> +    // We have -hwaccel without -vaapi, so just initialise here with the
>> +    // device passed as -hwaccel_device (if -vaapi was passed, it will
>> +    // always have been called before now).
>> +    if(!vaapi_context) {
>> +        err = vaapi_hardware_init(ist->hwaccel_device);
>> +        if(err)
>> +            return err;
>> +    }
>> +
>> +    if(ist->hwaccel_ctx) {
>> +        ctx = ist->hwaccel_ctx;
>> +
>> +        av_vaapi_lock_hardware_context(ctx->hardware_context);
>> +
>> +        err = av_vaapi_pipeline_uninit(&ctx->codec);
>> +        if(err) {
>> +            av_log(ctx, AV_LOG_ERROR, "Unable to reinit; failed to uninit "
>> +                   "old codec context: %d (%s).\n", err, av_err2str(err));
>> +            goto fail;
>> +        }
>> +
>> +        err = av_vaapi_surface_pool_uninit(&ctx->pool);
>> +        if(err) {
>> +            av_log(ctx, AV_LOG_ERROR, "Unable to reinit; failed to uninit "
>> +                   "old surface pool: %d (%s).\n", err, av_err2str(err));
>> +            goto fail;
>> +        }
>> +
>> +    } else {
>> +        if(vaapi_context->decoder_pipeline_config_id != VA_INVALID_ID) {
>> +            av_log(0, AV_LOG_ERROR, "Multiple simultaneous VAAPI decode "
>> +                   "pipelines are not supported!\n");
>> +            return AVERROR(EINVAL);
>> +        }
>> +
>> +        ctx = av_mallocz(sizeof(*ctx));
>> +        if(!ctx)
>> +            return AVERROR(ENOMEM);
>> +        ctx->class = &vaapi_class;
>> +
>> +        ctx->hardware_context = vaapi_context;
>> +
>> +        av_vaapi_lock_hardware_context(ctx->hardware_context);
>> +    }
>> +
>> +    err = vaapi_build_decoder_config(ctx, &ctx->config, &ctx->output, avctx);
>> +    if(err) {
>> +        av_log(ctx, AV_LOG_ERROR, "No supported configuration for this codec.");
>> +        goto fail;
>> +    }
>> +
>> +    err = av_vaapi_surface_pool_init(&ctx->pool, ctx->hardware_context,
>> +                                     &ctx->output, DEFAULT_SURFACES);
>> +    if(err) {
>> +        av_log(ctx, AV_LOG_ERROR, "Failed to initialise surface pool: "
>> +               "%d (%s).\n", err, av_err2str(err));
>> +        goto fail;
>> +    }
>> +
>> +    err = av_vaapi_pipeline_init(&ctx->codec, ctx->hardware_context,
>> +                                 &ctx->config, &ctx->pool);
>> +    if(err) {
>> +        av_log(ctx, AV_LOG_ERROR, "Failed to initialise codec context: "
>> +               "%d (%s).\n", err, av_err2str(err));
>> +        goto fail;
>> +    }
>> +    ctx->codec_initialised = 1;
>> +
>> +    av_vaapi_unlock_hardware_context(ctx->hardware_context);
>> +
>> +    av_log(ctx, AV_LOG_DEBUG, "VAAPI decoder (re)init complete.\n");
>> +
>> +    ist->hwaccel_ctx           = ctx;
>> +    ist->hwaccel_uninit        = vaapi_decode_uninit;
>> +    ist->hwaccel_get_buffer    = vaapi_get_buffer;
>> +    ist->hwaccel_retrieve_data = vaapi_retrieve_data;
>> +
>> +    avctx->hwaccel_context = ctx->hardware_context;
>> +
>> +    ctx->hardware_context->decoder_pipeline_config_id  = ctx->codec.config_id;
>> +    ctx->hardware_context->decoder_pipeline_context_id = ctx->codec.context_id;
>> +
>> +    return 0;
>> +
>> + fail:
>> +    av_vaapi_unlock_hardware_context(ctx->hardware_context);
>> +    vaapi_decode_uninit(avctx);
>> +    return err;
>> +}
>> +
>> +int vaapi_hardware_set_options(AVDictionary **dict)
>> +{
>> +    av_log(0, AV_LOG_DEBUG, "Setting VAAPI hardware_context.\n");
>> +    av_dict_set_int(dict, "hardware_context", (int64_t)vaapi_context, 0);
>> +    return 0;
>> +}
>> +
>> +
>> +static pthread_mutex_t vaapi_mutex;
>> +
>> +static void vaapi_mutex_lock(void *ignored)
>> +{
>> +    pthread_mutex_lock(&vaapi_mutex);
>> +}
>> +
>> +static void vaapi_mutex_unlock(void *ignored)
>> +{
>> +    pthread_mutex_unlock(&vaapi_mutex);
>> +}
>> +
>> +int vaapi_hardware_init(const char *device)
>> +{
>> +    VADisplay display;
>> +    int drm_fd;
>> +    Display *x11_display;
>> +    VAStatus vas;
>> +    int major, minor, err;
>> +    pthread_mutexattr_t mutex_attr;
>> +
>> +    err = pthread_mutexattr_init(&mutex_attr);
>> +    if(!err)
>> +        err = pthread_mutexattr_settype(&mutex_attr, PTHREAD_MUTEX_RECURSIVE);
>> +    if(!err)
>> +        err = pthread_mutex_init(&vaapi_mutex, &mutex_attr);
>> +    if(err) {
>> +        err = AVERROR(err);
>> +        av_log(0, AV_LOG_ERROR, "Failed to initialise VAAPI mutex: %s.\n",
>> +               av_err2str(err));
>> +        return err;
>> +    }
> 
> (Lacks pthread_mutexattr_destroy.)

Sure.

>> +
>> +    display = 0;
>> +
>> +#if HAVE_VAAPI_X11
>> +    if(!display) {
>> +        // Try to open the device as an X11 display.
>> +        x11_display = XOpenDisplay(device);
>> +        if(!x11_display) {
>> +            av_log(0, AV_LOG_WARNING, "Cannot open X11 display %s.\n",
>> +                   XDisplayName(device));
>> +        } else {
>> +            display = vaGetDisplay(x11_display);
>> +            if(!display) {
>> +                av_log(0, AV_LOG_WARNING, "Cannot open a VA display "
>> +                       "from X11 display %s.\n", XDisplayName(device));
>> +                XCloseDisplay(x11_display);
>> +            } else {
>> +                av_log(0, AV_LOG_VERBOSE, "Opened VA display via X11 "
>> +                       "display %s.\n", XDisplayName(device));
>> +            }
>> +        }
>> +    }
>> +#endif
>> +
>> +#if HAVE_VAAPI_DRM
>> +    if(!display && device) {
>> +        // Try to open the device as a DRM path.
>> +        drm_fd = open(device, O_RDWR);
>> +        if(drm_fd < 0) {
>> +            err = errno;
>> +            av_log(0, AV_LOG_WARNING, "Cannot open DRM device %s.\n",
>> +                   device);
>> +        } else {
>> +            display = vaGetDisplayDRM(drm_fd);
>> +            if(!display) {
>> +                av_log(0, AV_LOG_WARNING, "Cannot open a VA display "
>> +                       "from DRM device %s.\n", device);
>> +                close(drm_fd);
>> +            } else {
>> +                av_log(0, AV_LOG_VERBOSE, "Opened VA display via DRM "
>> +                       "device %s.\n", device);
>> +            }
>> +        }
>> +    }
>> +#endif
>> +
>> +    if(!display) {
>> +        av_log(0, AV_LOG_ERROR, "No VA display found for device %s.\n",
>> +               device);
>> +        return AVERROR(EINVAL);
>> +    }
>> +
>> +    vas = vaInitialize(display, &major, &minor);
>> +    if(vas != VA_STATUS_SUCCESS) {
>> +        av_log(0, AV_LOG_ERROR, "Failed to initialise VAAPI "
>> +               "connection: %d (%s).\n", vas, vaErrorStr(vas));
>> +        return AVERROR(EINVAL);
>> +    }
>> +    av_log(0, AV_LOG_VERBOSE, "Initialised VAAPI connection: "
>> +           "version %d.%d\n", major, minor);
>> +
>> +    vaapi_context = av_vaapi_alloc_hardware_context();
>> +    if(!vaapi_context)
>> +        return AVERROR(ENOMEM);
>> +
>> +    vaapi_context->display = display;
>> +    vaapi_context->lock    = &vaapi_mutex_lock;
>> +    vaapi_context->unlock  = &vaapi_mutex_unlock;
>> +
>> +    vaapi_context->decoder_pipeline_config_id  = VA_INVALID_ID;
>> +    vaapi_context->decoder_pipeline_context_id = VA_INVALID_ID;
>> +
>> +    return 0;
>> +}

There is currently has no uninitialisation on any of this (vaTerminate, close X display or drm fd) because I don't have a sensible place to call it from.  I'm hoping that's ok in the ffmpeg application.


Thanks for the review :)

- Mark



More information about the ffmpeg-devel mailing list