From patchwork Mon May 6 15:21:29 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Mark Thompson X-Patchwork-Id: 13013 Return-Path: X-Original-To: patchwork@ffaux-bg.ffmpeg.org Delivered-To: patchwork@ffaux-bg.ffmpeg.org Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org [79.124.17.100]) by ffaux.localdomain (Postfix) with ESMTP id 0AAF2449A0D for ; Mon, 6 May 2019 18:21:39 +0300 (EEST) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id DBA2C6800D8; Mon, 6 May 2019 18:21:38 +0300 (EEST) X-Original-To: ffmpeg-devel@ffmpeg.org Delivered-To: ffmpeg-devel@ffmpeg.org Received: from mail-wm1-f67.google.com (mail-wm1-f67.google.com [209.85.128.67]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 3DC5368AB8A for ; Mon, 6 May 2019 18:21:31 +0300 (EEST) Received: by mail-wm1-f67.google.com with SMTP id p21so16268160wmc.0 for ; Mon, 06 May 2019 08:21:31 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=jkqxz-net.20150623.gappssmtp.com; s=20150623; h=to:from:subject:message-id:date:user-agent:mime-version :content-language:content-transfer-encoding; bh=nfpNGRdLNj0YyHHfxdikZUtDEGSM84HXUK8cbn8NObQ=; b=IRErIuEZmCneH3+QtiZpYRuGe7veluTutpvWhRGYPUG8yYnhFUmfdIms24gwzPLFwV 3cIzBa3Uk/qvdRhSJ2LyBB4WLg/RDj2okHO1mE0qioUqhGxoqF8Khnt1+Z2ujZDl1dLk Ukv4Q8Xg825bfbtpEGooA6Dxgx/MUgEv1D8DfxiJnMPep+s3iRlbKZPi2eukSs5I8Fkb KL4GnJYcHUzthATFBYelgsyedlHg3Nl/2LZF7HRzzJGCeRg+vCqOTMRXH0SJdCYoOADQ KgPzEL9VZB1vimJ6/GmODkwRycklr9haUL6scdDEeLRjCGev/XYOot5WreslPjoHRyPU NTwQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:to:from:subject:message-id:date:user-agent :mime-version:content-language:content-transfer-encoding; bh=nfpNGRdLNj0YyHHfxdikZUtDEGSM84HXUK8cbn8NObQ=; b=apfSdnTbPhqgucy74JuwWtypBZPr6H85DYcCRV5JdLUyVGHJ9QjbXfmQsURsxmaKlK TK5JuGT7AOuriwjVz/KZkXNu8Xf65Bku0lST7nZOQLVXtA+HZi2AuAJHU5Xtqp3Ks7h7 cG4hQb4OAr5t7rq64rn8wfrCSW1XNsdZQP9bdU9zikx//VUNGrlNem3pARv8TELroko7 nT658GLJLHBUFdtNgDVUqkVUWbLqsdjJ1kXeinsu9lRBsniQYdE7UmnBP5YsbrsYMDKS 0zSMWl1XbYJf1tRAEIjKJjdJ89iQbh6pdkXPaUA3flvJ6+E9suDYCDIevA2L8cWXMi2j BdSQ== X-Gm-Message-State: APjAAAUemgraf0IwvEImQSefA8XLb5aB5pglw6AwGc+NBCOhYVx9yhJ+ CyMpv9elkxoEa8949O49xskq6Qzl5/8= X-Google-Smtp-Source: APXvYqzqJa+AJcEWSsTiUdngjcWhWi3ORkQ4pWasQ9av5cMzglz4BfmELmYz/H0DSASSZ/tLY5lQoA== X-Received: by 2002:a1c:a893:: with SMTP id r141mr16633079wme.7.1557156091100; Mon, 06 May 2019 08:21:31 -0700 (PDT) Received: from [192.168.0.13] (cpc91242-cmbg18-2-0-cust650.5-4.cable.virginm.net. [82.8.130.139]) by smtp.gmail.com with ESMTPSA id f7sm10248825wmb.28.2019.05.06.08.21.29 for (version=TLS1_3 cipher=AEAD-AES128-GCM-SHA256 bits=128/128); Mon, 06 May 2019 08:21:29 -0700 (PDT) To: FFmpeg development discussions and patches From: Mark Thompson Message-ID: <16133da8-ce0a-10da-c32c-dc43fdc4e10c@jkqxz.net> Date: Mon, 6 May 2019 16:21:29 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.6.1 MIME-Version: 1.0 Content-Language: en-US Subject: [FFmpeg-devel] [PATCH] vaapi_encode: Refactor encode misc parameter buffer creation X-BeenThere: ffmpeg-devel@ffmpeg.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: FFmpeg development discussions and patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: FFmpeg development discussions and patches Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" This removes the use of the nonstandard combined structures, which generated some warnings with clang and will cause alignment problems with some parameter buffer types. --- On 27/03/2019 14:18, Carl Eugen Hoyos wrote: > Attached patch fixes many warnings when compiling vaapi with clang. > Also tested with clang-3.4. > ... How about this approach instead? I think something like it is going to be needed anyway because of alignment problems with parameter structures which aren't yet used. - Mark libavcodec/vaapi_encode.c | 71 ++++++++++++++++++++++++---------- libavcodec/vaapi_encode.h | 23 +++-------- libavcodec/vaapi_encode_h264.c | 8 ++-- 3 files changed, 60 insertions(+), 42 deletions(-) diff --git a/libavcodec/vaapi_encode.c b/libavcodec/vaapi_encode.c index 2dda451882..95031187df 100644 --- a/libavcodec/vaapi_encode.c +++ b/libavcodec/vaapi_encode.c @@ -103,6 +103,29 @@ static int vaapi_encode_make_param_buffer(AVCodecContext *avctx, return 0; } +static int vaapi_encode_make_misc_param_buffer(AVCodecContext *avctx, + VAAPIEncodePicture *pic, + int type, + const void *data, size_t len) +{ + // Construct the buffer on the stack - 1KB is much larger than any + // current misc parameter buffer type (the largest is EncQuality at + // 224 bytes). + uint8_t buffer[1024]; + VAEncMiscParameterBuffer header = { + .type = type, + }; + size_t buffer_size = sizeof(header) + len; + av_assert0(buffer_size <= sizeof(buffer)); + + memcpy(buffer, &header, sizeof(header)); + memcpy(buffer + sizeof(header), data, len); + + return vaapi_encode_make_param_buffer(avctx, pic, + VAEncMiscParameterBufferType, + buffer, buffer_size); +} + static int vaapi_encode_wait(AVCodecContext *avctx, VAAPIEncodePicture *pic) { @@ -212,10 +235,10 @@ static int vaapi_encode_issue(AVCodecContext *avctx, if (pic->type == PICTURE_TYPE_IDR) { for (i = 0; i < ctx->nb_global_params; i++) { - err = vaapi_encode_make_param_buffer(avctx, pic, - VAEncMiscParameterBufferType, - (char*)ctx->global_params[i], - ctx->global_params_size[i]); + err = vaapi_encode_make_misc_param_buffer(avctx, pic, + ctx->global_params_type[i], + ctx->global_params[i], + ctx->global_params_size[i]); if (err < 0) goto fail; } @@ -1034,14 +1057,14 @@ int ff_vaapi_encode2(AVCodecContext *avctx, AVPacket *pkt, return AVERROR(ENOSYS); } -static av_cold void vaapi_encode_add_global_param(AVCodecContext *avctx, - VAEncMiscParameterBuffer *buffer, - size_t size) +static av_cold void vaapi_encode_add_global_param(AVCodecContext *avctx, int type, + void *buffer, size_t size) { VAAPIEncodeContext *ctx = avctx->priv_data; av_assert0(ctx->nb_global_params < MAX_GLOBAL_PARAMS); + ctx->global_params_type[ctx->nb_global_params] = type; ctx->global_params [ctx->nb_global_params] = buffer; ctx->global_params_size[ctx->nb_global_params] = size; @@ -1575,8 +1598,7 @@ rc_mode_found: rc_bits_per_second, rc_window_size); } - ctx->rc_params.misc.type = VAEncMiscParameterTypeRateControl; - ctx->rc_params.rc = (VAEncMiscParameterRateControl) { + ctx->rc_params = (VAEncMiscParameterRateControl) { .bits_per_second = rc_bits_per_second, .target_percentage = rc_target_percentage, .window_size = rc_window_size, @@ -1591,7 +1613,9 @@ rc_mode_found: .quality_factor = rc_quality, #endif }; - vaapi_encode_add_global_param(avctx, &ctx->rc_params.misc, + vaapi_encode_add_global_param(avctx, + VAEncMiscParameterTypeRateControl, + &ctx->rc_params, sizeof(ctx->rc_params)); } @@ -1600,12 +1624,13 @@ rc_mode_found: "initial fullness %"PRId64" bits.\n", hrd_buffer_size, hrd_initial_buffer_fullness); - ctx->hrd_params.misc.type = VAEncMiscParameterTypeHRD; - ctx->hrd_params.hrd = (VAEncMiscParameterHRD) { + ctx->hrd_params = (VAEncMiscParameterHRD) { .initial_buffer_fullness = hrd_initial_buffer_fullness, .buffer_size = hrd_buffer_size, }; - vaapi_encode_add_global_param(avctx, &ctx->hrd_params.misc, + vaapi_encode_add_global_param(avctx, + VAEncMiscParameterTypeHRD, + &ctx->hrd_params, sizeof(ctx->hrd_params)); } @@ -1619,11 +1644,13 @@ rc_mode_found: av_log(avctx, AV_LOG_VERBOSE, "RC framerate: %d/%d (%.2f fps).\n", fr_num, fr_den, (double)fr_num / fr_den); - ctx->fr_params.misc.type = VAEncMiscParameterTypeFrameRate; - ctx->fr_params.fr.framerate = (unsigned int)fr_den << 16 | fr_num; - + ctx->fr_params = (VAEncMiscParameterFrameRate) { + .framerate = (unsigned int)fr_den << 16 | fr_num, + }; #if VA_CHECK_VERSION(0, 40, 0) - vaapi_encode_add_global_param(avctx, &ctx->fr_params.misc, + vaapi_encode_add_global_param(avctx, + VAEncMiscParameterTypeFrameRate, + &ctx->fr_params, sizeof(ctx->fr_params)); #endif @@ -1885,10 +1912,12 @@ static av_cold int vaapi_encode_init_quality(AVCodecContext *avctx) quality = attr.value; } - ctx->quality_params.misc.type = VAEncMiscParameterTypeQualityLevel; - ctx->quality_params.quality.quality_level = quality; - - vaapi_encode_add_global_param(avctx, &ctx->quality_params.misc, + ctx->quality_params = (VAEncMiscParameterBufferQualityLevel) { + .quality_level = quality, + }; + vaapi_encode_add_global_param(avctx, + VAEncMiscParameterTypeQualityLevel, + &ctx->quality_params, sizeof(ctx->quality_params)); } #else diff --git a/libavcodec/vaapi_encode.h b/libavcodec/vaapi_encode.h index 44a8db566e..80f6c680b4 100644 --- a/libavcodec/vaapi_encode.h +++ b/libavcodec/vaapi_encode.h @@ -244,28 +244,17 @@ typedef struct VAAPIEncodeContext { // Global parameters which will be applied at the start of the // sequence (includes rate control parameters below). - VAEncMiscParameterBuffer *global_params[MAX_GLOBAL_PARAMS]; + int global_params_type[MAX_GLOBAL_PARAMS]; + const void *global_params [MAX_GLOBAL_PARAMS]; size_t global_params_size[MAX_GLOBAL_PARAMS]; int nb_global_params; // Rate control parameters. - struct { - VAEncMiscParameterBuffer misc; - VAEncMiscParameterRateControl rc; - } rc_params; - struct { - VAEncMiscParameterBuffer misc; - VAEncMiscParameterHRD hrd; - } hrd_params; - struct { - VAEncMiscParameterBuffer misc; - VAEncMiscParameterFrameRate fr; - } fr_params; + VAEncMiscParameterRateControl rc_params; + VAEncMiscParameterHRD hrd_params; + VAEncMiscParameterFrameRate fr_params; #if VA_CHECK_VERSION(0, 36, 0) - struct { - VAEncMiscParameterBuffer misc; - VAEncMiscParameterBufferQualityLevel quality; - } quality_params; + VAEncMiscParameterBufferQualityLevel quality_params; #endif // Per-sequence parameter structure (VAEncSequenceParameterBuffer*). diff --git a/libavcodec/vaapi_encode_h264.c b/libavcodec/vaapi_encode_h264.c index 4cf99d7c78..d1427112ea 100644 --- a/libavcodec/vaapi_encode_h264.c +++ b/libavcodec/vaapi_encode_h264.c @@ -471,9 +471,9 @@ static int vaapi_encode_h264_init_sequence_params(AVCodecContext *avctx) (ctx->va_bit_rate >> hrd->bit_rate_scale + 6) - 1; hrd->cpb_size_scale = - av_clip_uintp2(av_log2(ctx->hrd_params.hrd.buffer_size) - 15 - 4, 4); + av_clip_uintp2(av_log2(ctx->hrd_params.buffer_size) - 15 - 4, 4); hrd->cpb_size_value_minus1[0] = - (ctx->hrd_params.hrd.buffer_size >> hrd->cpb_size_scale + 4) - 1; + (ctx->hrd_params.buffer_size >> hrd->cpb_size_scale + 4) - 1; // CBR mode as defined for the HRD cannot be achieved without filler // data, so this flag cannot be set even with VAAPI CBR modes. @@ -488,8 +488,8 @@ static int vaapi_encode_h264_init_sequence_params(AVCodecContext *avctx) // This calculation can easily overflow 32 bits. bp->nal.initial_cpb_removal_delay[0] = 90000 * - (uint64_t)ctx->hrd_params.hrd.initial_buffer_fullness / - ctx->hrd_params.hrd.buffer_size; + (uint64_t)ctx->hrd_params.initial_buffer_fullness / + ctx->hrd_params.buffer_size; bp->nal.initial_cpb_removal_delay_offset[0] = 0; } else { sps->vui.nal_hrd_parameters_present_flag = 0;