[FFmpeg-devel] [PATCH v3] lavf : scale_vaapi : add denoise/sharpless support
Jun Zhao
mypopydev at gmail.com
Mon Sep 5 04:52:22 EEST 2016
On 2016/8/31 6:48, Mark Thompson wrote:
> On 30/08/16 09:00, Jun Zhao wrote:
>> v3 : fix sharpless mapping issue
>> v2 : fix filter support flag check logic issue
>
> Hi,
>
> A general remark to start: vf_scale_vaapi is named to be a scaling filter (i.e. it replaces vf_scale/swscale for AV_PIX_FMT_VAAPI) - is this therefore really the right place to be adding other operations unrelated to scaling?
>
> Do use-cases for these operations actually make sense to add here rather than in a separate filter? (I'm not sure what the answer to this should be - I would definitely argue that the deinterlacer should be a separate filter, but these other operations are unclear.)
>
>
As you know, VPP use the pipeline mode, split the scale/denoise/sharpness/... in
different filter maybe is not good idea, I guess nobody want to call vaRenderPicture()/
vaEndpicture/... again and again in vf_scale_vaapi.c/vf_denosie_vaapi.c/vf_sharpness_vaapi.c/...
>> From 415b00c6157d8311cc18713e6347400895f7333c Mon Sep 17 00:00:00 2001
>> From: Jun Zhao <mypopydev at gmail.com>
>> Date: Tue, 30 Aug 2016 14:36:00 +0800
>> Subject: [PATCH v3] lavf : scale_vaapi : add denoise/sharpless support.
>>
>> add denoise/sharpless support, used scope [-1, 100] as the input
>> scope.
>>
>> Signed-off-by: Jun Zhao <jun.zhao at intel.com>
>> ---
>> libavfilter/vf_scale_vaapi.c | 115 +++++++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 115 insertions(+)
>>
>> diff --git a/libavfilter/vf_scale_vaapi.c b/libavfilter/vf_scale_vaapi.c
>> index 8dd5acf..5658746 100644
>> --- a/libavfilter/vf_scale_vaapi.c
>> +++ b/libavfilter/vf_scale_vaapi.c
>> @@ -53,6 +53,16 @@ typedef struct ScaleVAAPIContext {
>> int output_width;
>> int output_height;
>>
>> + VAProcFilterCap denoise_caps;
>> + int support_denoise;
>> + int denoise; // enable denoise algo. level is the optional
>> + // value from the interval [-1, 100], -1 means disabled
>> +
>> + VAProcFilterCap sharpless_caps;
>> + int support_sharpless;
>> + int sharpless; // enable sharpless. level is the optional value
>> + // from the interval [-1, 100], -1 means disabled
>
> "sharpless"? "sharpness" or "sharpen", might be better. (The filter is called "sharpening", though maybe it can also blur the image?)
>
>> +
>> } ScaleVAAPIContext;
>>
>>
>> @@ -117,6 +127,8 @@ static int scale_vaapi_config_output(AVFilterLink *outlink)
>> AVVAAPIFramesContext *va_frames;
>> VAStatus vas;
>> int err, i;
>> + unsigned int num_denoise_caps = 1;
>> + unsigned int num_sharpless_caps = 1;
>>
>> scale_vaapi_pipeline_uninit(ctx);
>>
>> @@ -225,6 +237,29 @@ static int scale_vaapi_config_output(AVFilterLink *outlink)
>> goto fail;
>> }
>>
>> + vas = vaQueryVideoProcFilterCaps(ctx->hwctx->display, ctx->va_context,
>> + VAProcFilterNoiseReduction,
>> + &ctx->denoise_caps, &num_denoise_caps);
>> + if (vas != VA_STATUS_SUCCESS) {
>> + av_log(ctx, AV_LOG_WARNING, "Failed to query denoise caps "
>> + "context: %d (%s).\n", vas, vaErrorStr(vas));
>> + ctx->support_denoise = 0;
>> + } else {
>> + ctx->support_denoise = 1;
>> + }
>> +
>> + vas = vaQueryVideoProcFilterCaps(ctx->hwctx->display, ctx->va_context,
>> + VAProcFilterSharpening,
>> + &ctx->sharpless_caps, &num_sharpless_caps);
>> + if (vas != VA_STATUS_SUCCESS) {
>> + av_log(ctx, AV_LOG_WARNING, "Failed to query sharpless caps "
>> + "context: %d (%s).\n", vas, vaErrorStr(vas));
>> + ctx->support_sharpless = 0;
>> + } else {
>> + ctx->support_sharpless = 1;
>> + }
>
> If the user explicitly requests these filters that can still be ignored if they not supported? Maybe it would be better to just give up with an error message at that point.
>
> Similarly, the warning that they are not supported is unhelpful if the user didn't ask for them.
>
>> +
>> +
>> av_freep(&hwconfig);
>> av_hwframe_constraints_free(&constraints);
>> return 0;
>> @@ -250,6 +285,14 @@ static int vaapi_proc_colour_standard(enum AVColorSpace av_cs)
>> }
>> }
>>
>> +static float map_to_range(
>> + int input, int in_min, int in_max,
>> + float out_min, float out_max)
>> +{
>> + return (input - in_min) * (out_max - out_min) / (in_max - in_min) + out_min;
>> +}
>> +
>> +
>> static int scale_vaapi_filter_frame(AVFilterLink *inlink, AVFrame *input_frame)
>> {
>> AVFilterContext *avctx = inlink->dst;
>> @@ -259,6 +302,10 @@ static int scale_vaapi_filter_frame(AVFilterLink *inlink, AVFrame *input_frame)
>> VASurfaceID input_surface, output_surface;
>> VAProcPipelineParameterBuffer params;
>> VABufferID params_id;
>> + VABufferID denoise_id;
>> + VABufferID sharpless_id;
>> + VABufferID filter_bufs[8];
>> + int num_filter_bufs = 0;
>> VAStatus vas;
>> int err;
>>
>> @@ -290,6 +337,43 @@ static int scale_vaapi_filter_frame(AVFilterLink *inlink, AVFrame *input_frame)
>> av_log(ctx, AV_LOG_DEBUG, "Using surface %#x for scale output.\n",
>> output_surface);
>>
>> + if (ctx->denoise != -1 && ctx->support_denoise) {
>> + VAProcFilterParameterBuffer denoise;
>> + denoise.type = VAProcFilterNoiseReduction;
>> + denoise.value = map_to_range(ctx->denoise, 0, 100,
>> + ctx->denoise_caps.range.min_value,
>> + ctx->denoise_caps.range.max_value);
>> + vas = vaCreateBuffer(ctx->hwctx->display, ctx->va_context,
>> + VAProcFilterParameterBufferType, sizeof(denoise), 1,
>> + &denoise, &denoise_id);
>> + if (vas != VA_STATUS_SUCCESS) {
>> + av_log(ctx, AV_LOG_ERROR, "Failed to create denoise parameter buffer: "
>> + "%d (%s).\n", vas, vaErrorStr(vas));
>> + err = AVERROR(EIO);
>> + goto fail;
>> + }
>> + filter_bufs[num_filter_bufs++] = denoise_id;
>> + }
>> +
>> + if (ctx->sharpless != -1 && ctx->support_sharpless) {
>> + VAProcFilterParameterBuffer sharpless;
>> + sharpless.type = VAProcFilterSharpening;
>> + sharpless.value = map_to_range(ctx->sharpless,
>> + 0, 100,
>> + ctx->sharpless_caps.range.min_value,
>> + ctx->sharpless_caps.range.max_value);
>> + vas = vaCreateBuffer(ctx->hwctx->display, ctx->va_context,
>> + VAProcFilterParameterBufferType, sizeof(sharpless), 1,
>> + &sharpless, &sharpless_id);
>> + if (vas != VA_STATUS_SUCCESS) {
>> + av_log(ctx, AV_LOG_ERROR, "Failed to create sharpless parameter buffer: "
>> + "%d (%s).\n", vas, vaErrorStr(vas));
>> + err = AVERROR(EIO);
>> + goto fail;
>> + }
>> + filter_bufs[num_filter_bufs++] = sharpless_id;
>> + }
>> +
>> memset(¶ms, 0, sizeof(params));
>>
>> params.surface = input_surface;
>> @@ -304,6 +388,11 @@ static int scale_vaapi_filter_frame(AVFilterLink *inlink, AVFrame *input_frame)
>> params.pipeline_flags = 0;
>> params.filter_flags = VA_FILTER_SCALING_HQ;
>>
>> + if (num_filter_bufs) {
>> + params.filters = filter_bufs;
>> + params.num_filters = num_filter_bufs;
>> + }
>> +
>> vas = vaBeginPicture(ctx->hwctx->display,
>> ctx->va_context, output_surface);
>> if (vas != VA_STATUS_SUCCESS) {
>> @@ -351,6 +440,28 @@ static int scale_vaapi_filter_frame(AVFilterLink *inlink, AVFrame *input_frame)
>> goto fail;
>> }
>>
>> + // This doesn't get freed automatically for some reason.
>> + if (ctx->denoise != -1 && ctx->support_denoise) {
>> + vas = vaDestroyBuffer(ctx->hwctx->display, denoise_id);
>> + if (vas != VA_STATUS_SUCCESS) {
>> + av_log(ctx, AV_LOG_ERROR, "Failed to free denoise parameter buffer: "
>> + "%d (%s).\n", vas, vaErrorStr(vas));
>> + err = AVERROR(EIO);
>> + goto fail;
>> + }
>> + }
>> +
>> + // This doesn't get freed automatically for some reason.
>> + if (ctx->sharpless != -1 && ctx->support_sharpless) {
>> + vas = vaDestroyBuffer(ctx->hwctx->display, sharpless_id);
>> + if (vas != VA_STATUS_SUCCESS) {
>> + av_log(ctx, AV_LOG_ERROR, "Failed to free sharpless parameter buffer: "
>> + "%d (%s).\n", vas, vaErrorStr(vas));
>> + err = AVERROR(EIO);
>> + goto fail;
>> + }
>> + }
>
> See <https://git.libav.org/?p=libav.git;a=commit;h=582d4211e00015b68626f77ce4af53161e2b1713> for some later context to the comment. Might we be better off without the repeated create/destroy here, given that vaRenderPicture() isn't going to free these subsidiary buffers in the general case?
>
>> +
>> av_frame_copy_props(output_frame, input_frame);
>> av_frame_free(&input_frame);
>>
>
> The filter parameter buffers should be freed on failure (assuming they don't persist).
>
>> @@ -418,6 +529,10 @@ static const AVOption scale_vaapi_options[] = {
>> OFFSET(output_height), AV_OPT_TYPE_INT, { .i64 = 0 }, 0, INT_MAX, .flags = FLAGS },
>> { "format", "Output video format (software format of hardware frames)",
>> OFFSET(output_format_string), AV_OPT_TYPE_STRING, .flags = FLAGS },
>> + { "denoise", "denoise level [-1, 100], -1 means disabled",
>> + OFFSET(denoise), AV_OPT_TYPE_INT, { .i64 = -1 }, -1, 100, .flags = FLAGS },
>> + { "sharpless", "sharpless level [-1, 100], -1 means disabled",
>> + OFFSET(sharpless), AV_OPT_TYPE_INT, { .i64 = -1 }, -1, 100, .flags = FLAGS },
>> { NULL },
>> };
>>
>> --
>> 2.1.4
>>
>
> Finally, I tried to test this on a Skylake with recent git libva. I took a 1080p input video and ran:
>
> ./ffmpeg_g -y -threads 1 -vaapi_device /dev/dri/renderD128 -hwaccel vaapi -hwaccel_output_format vaapi -i in.mp4 -an -vf 'format=vaapi|nv12,hwupload,scale_vaapi=w=1280:h=720:sharpless=50' -c:v h264_vaapi -qp 20 out.mp4
>
> but it doesn't appear to be working as one might expect. I end up with a 720p output video consisting of the top left four-ninths of the input video, unscaled? It also has a very large slowdown: that command runs at ~80fps, but without the sharpness setting in it runs at ~300fps.
>
> I also tried:
>
> ./ffmpeg_g -y -threads 1 -vaapi_device /dev/dri/renderD128 -hwaccel vaapi -hwaccel_output_format vaapi -i in.mp4 -an -vf 'format=vaapi|nv12,hwupload,scale_vaapi=w=1280:h=720:denoise=50' -c:v h264_vaapi -qp 20 out.mp4
>
> and got:
>
> Thread 1 "ffmpeg_g" received signal SIGSEGV, Segmentation fault.
> hsw_veb_pre_format_convert (ctx=ctx at entry=0x234b8e0, proc_ctx=proc_ctx at entry=0x23ab170) at ../../git/src/gen75_vpp_vebox.c:1382
> 1382 proc_ctx->width_input = proc_ctx->pipeline_param->surface_region->width;
> (gdb) bt
> #0 hsw_veb_pre_format_convert (ctx=ctx at entry=0x234b8e0, proc_ctx=proc_ctx at entry=0x23ab170) at ../../git/src/gen75_vpp_vebox.c:1382
> #1 0x00007ffff4438bc7 in gen9_vebox_process_picture (ctx=ctx at entry=0x234b8e0, proc_ctx=0x23ab170) at ../../git/src/gen75_vpp_vebox.c:2384
> #2 0x00007ffff443279b in gen75_vpp_vebox (ctx=ctx at entry=0x234b8e0, proc_ctx=proc_ctx at entry=0x23a75a0) at ../../git/src/gen75_picture_process.c:89
> #3 0x00007ffff4432e91 in gen75_proc_picture (ctx=0x234b8e0, profile=VAProfileNone, codec_state=<optimized out>, hw_context=0x23a75a0) at ../../git/src/gen75_picture_process.c:328
> #4 0x00007ffff705de5f in vaEndPicture (dpy=0x234b800, context=33554432) at ../../git/va/va.c:1232
> #5 0x0000000000593133 in scale_vaapi_filter_frame (inlink=0x23e8d20, input_frame=0x23aab20) at src/libavfilter/vf_scale_vaapi.c:426
> #6 0x0000000000455a6c in ff_filter_frame_framed (link=0x23e8d20, frame=0x23aab20) at src/libavfilter/avfilter.c:1134
> #7 0x0000000000455f62 in ff_filter_frame (link=0x23e8d20, frame=0x23aab20) at src/libavfilter/avfilter.c:1232
> #8 0x00000000005078d7 in hwupload_filter_frame (link=0x237cc00, input=0x23aab20) at src/libavfilter/vf_hwupload.c:162
> #9 0x0000000000455a6c in ff_filter_frame_framed (link=0x237cc00, frame=0x23aab20) at src/libavfilter/avfilter.c:1134
> #10 0x0000000000455f62 in ff_filter_frame (link=0x237cc00, frame=0x23aab20) at src/libavfilter/avfilter.c:1232
> #11 0x00000000004555a4 in default_filter_frame (link=0x237b180, frame=0x23aab20) at src/libavfilter/avfilter.c:1046
> #12 0x0000000000455a6c in ff_filter_frame_framed (link=0x237b180, frame=0x23aab20) at src/libavfilter/avfilter.c:1134
> #13 0x0000000000455f62 in ff_filter_frame (link=0x237b180, frame=0x23aab20) at src/libavfilter/avfilter.c:1232
> #14 0x000000000045bde8 in request_frame (link=0x237b180) at src/libavfilter/buffersrc.c:450
> #15 0x000000000045b675 in av_buffersrc_add_frame_internal (ctx=0x237b080, frame=0x23ba000, flags=4) at src/libavfilter/buffersrc.c:239
> #16 0x000000000045b344 in av_buffersrc_add_frame_flags (ctx=0x237b080, frame=0x23ba000, flags=4) at src/libavfilter/buffersrc.c:164
> #17 0x00000000004257b2 in decode_video (ist=0x244a2e0, pkt=0x7fffffffdc50, got_output=0x7fffffffdcac) at src/ffmpeg.c:2196
> #18 0x000000000042606e in process_input_packet (ist=0x244a2e0, pkt=0x7fffffffdda0, no_eof=0) at src/ffmpeg.c:2340
> #19 0x000000000042ca11 in process_input (file_index=0) at src/ffmpeg.c:4020
> #20 0x000000000042cd06 in transcode_step () at src/ffmpeg.c:4108
> #21 0x000000000042ce23 in transcode () at src/ffmpeg.c:4162
> #22 0x000000000042d474 in main (argc=20, argv=0x7fffffffe468) at src/ffmpeg.c:4357
> (gdb) p proc_ctx
> $1 = (struct intel_vebox_context *) 0x23ab170
> (gdb) p proc_ctx->pipeline_param
> $2 = (VAProcPipelineParameterBuffer *) 0x23ab100
> (gdb) p proc_ctx->pipeline_param->surface_region
> $3 = (const VARectangle *) 0x0
> (gdb) p proc_ctx->pipeline_param->surface_region->width
> Cannot access memory at address 0x4
>
>
> So, can you share how you are running this, and what the expected results are?
>
>
> Thanks
>
> - Mark
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
More information about the ffmpeg-devel
mailing list