On 2016/9/10 3:29, Mark Thompson wrote:
> On 09/09/16 04:31, Jun Zhao wrote:
>> v4 : - fix sharpless typo(sharpless -> sharpness)
>> - when don't support denoise/sharpness, report the error and return
>> - fix denoise/sharpness params buffer leak in error handle
>> v3 : fix sharpless mapping issue
>> v2 : fix filter support flag check logic issue
>>
>> From d1f0b556bdd6be4dffcd3b1b93cba5cd1098908a Mon Sep 17 00:00:00 2001
>> From: Jun Zhao <mypopy...@gmail.com>
>> Date: Tue, 30 Aug 2016 14:36:00 +0800
>> Subject: [PATCH v4] lavf : scale_vaapi : add denoise/sharpless support.
>
> I think libavfilter is generally "lavfi" ("lavf" is libavformat).
Thanks for you correct me, will re-submit the patch with correct prefix "lavfi"
>
>> add denoise/sharpless support, used scope [-1, 100] as the input
>> scope.
>>
>> Signed-off-by: Jun Zhao <jun.z...@intel.com>
>> ---
>> libavfilter/vf_scale_vaapi.c | 112
>> +++++++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 112 insertions(+)
>>
>> diff --git a/libavfilter/vf_scale_vaapi.c b/libavfilter/vf_scale_vaapi.c
>> index 8dd5acf..e48e201 100644
>> --- a/libavfilter/vf_scale_vaapi.c
>> +++ b/libavfilter/vf_scale_vaapi.c
>> @@ -53,6 +53,14 @@ typedef struct ScaleVAAPIContext {
>> int output_width;
>> int output_height;
>>
>> + VAProcFilterCap denoise_caps;
>> + int denoise; // enable denoise algo. level is the optional
>> + // value from the interval [-1, 100], -1 means
>> disabled
>> +
>> + VAProcFilterCap sharpness_caps;
>> + int sharpness; // enable sharpness. level is the optional value
>> + // from the interval [-1, 100], -1 means disabled
>> +
>> } ScaleVAAPIContext;
>>
>>
>> @@ -117,6 +125,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_sharpness_caps = 1;
>>
>> scale_vaapi_pipeline_uninit(ctx);
>>
>> @@ -225,6 +235,28 @@ static int scale_vaapi_config_output(AVFilterLink
>> *outlink)
>> goto fail;
>> }
>>
>> + if (ctx->denoise != -1) {
>> + 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_ERROR, "Failed to query denoise caps "
>> + "context: %d (%s).\n", vas, vaErrorStr(vas));
>> + return AVERROR(EIO);
>> + }
>> + }
>> +
>> + if (ctx->sharpness != -1) {
>> + vas = vaQueryVideoProcFilterCaps(ctx->hwctx->display,
>> ctx->va_context,
>> + VAProcFilterSharpening,
>> + &ctx->sharpness_caps,
>> &num_sharpness_caps);
>> + if (vas != VA_STATUS_SUCCESS) {
>> + av_log(ctx, AV_LOG_ERROR, "Failed to query sharpness caps "
>> + "context: %d (%s).\n", vas, vaErrorStr(vas));
>> + return AVERROR(EIO);
>> + }
>> + }
>> +
>> av_freep(&hwconfig);
>> av_hwframe_constraints_free(&constraints);
>> return 0;
>> @@ -250,6 +282,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 +299,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 sharpness_id;
>> + VABufferID filter_bufs[8];
>
> VAProcFilterCount might be a better number than 8 as an upper bound on the
> number of filters which could ever be added to this.
agree
>
>> + int num_filter_bufs = 0;
>> VAStatus vas;
>> int err;
>>
>> @@ -290,6 +334,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) {
>> + 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->sharpness != -1) {
>> + VAProcFilterParameterBuffer sharpness;
>> + sharpness.type = VAProcFilterSharpening;
>> + sharpness.value = map_to_range(ctx->sharpness,
>> + 0, 100,
>> + ctx->sharpness_caps.range.min_value,
>> + ctx->sharpness_caps.range.max_value);
>> + vas = vaCreateBuffer(ctx->hwctx->display, ctx->va_context,
>> + VAProcFilterParameterBufferType, sizeof(sharpness),
>> 1,
>> + &sharpness, &sharpness_id);
>> + if (vas != VA_STATUS_SUCCESS) {
>> + av_log(ctx, AV_LOG_ERROR, "Failed to create sharpness
>> parameter buffer: "
>> + "%d (%s).\n", vas, vaErrorStr(vas));
>> + err = AVERROR(EIO);
>> + goto fail;
>> + }
>> + filter_bufs[num_filter_bufs++] = sharpness_id;
>> + }
>> +
>> memset(¶ms, 0, sizeof(params));
>>
>> params.surface = input_surface;
>> @@ -304,6 +385,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 +437,26 @@ 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) {
>> + 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);
>> + }
>> + }
>> +
>> + // This doesn't get freed automatically for some reason.
>> + if (ctx->sharpness != -1) {
>> + vas = vaDestroyBuffer(ctx->hwctx->display, sharpness_id);
>> + if (vas != VA_STATUS_SUCCESS) {
>> + av_log(ctx, AV_LOG_ERROR, "Failed to free sharpness parameter
>> buffer: "
>> + "%d (%s).\n", vas, vaErrorStr(vas));
>> + err = AVERROR(EIO);
>> + }
>> + }
>
> Did you decide on anything wrt the create/destroy point last time? In any
> case, the comment shouldn't be copied like this.
Do you think can we change it like this?
for (int i = 0; i < num_filter_bufs; i++)
vaDestroyBuffer(ctx->hwctx->display, filter_bufs[i]);
>
>> +
>> av_frame_copy_props(output_frame, input_frame);
>> av_frame_free(&input_frame);
>>
>> @@ -369,6 +475,8 @@ fail_after_begin:
>> fail_after_render:
>> vaEndPicture(ctx->hwctx->display, ctx->va_context);
>> fail:
>> + for (int i = 0; i < num_filter_bufs; i++)
>> + vaDestroyBuffer(ctx->hwctx->display, filter_bufs[i]);
>> av_frame_free(&input_frame);
>> av_frame_free(&output_frame);
>> return err;
>> @@ -418,6 +526,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 },
>> + { "sharpness", "sharpness level [-1, 100], -1 means disabled",
>> + OFFSET(sharpness), AV_OPT_TYPE_INT, { .i64 = -1 }, -1, 100, .flags =
>> FLAGS },
>> { NULL },
>> };
>>
>> --
>> 2.1.4
>>
>
> Some testing, then: all on a Skylake 6300 with latest git libva and
> intel-driver - thanks for the pointer to the fix for the crash in the noise
> reduction filter. Input is the 1080p version of big buck bunny.
>
> ./ffmpeg_g -y -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' -c:v h264_vaapi out.mp4
>
> -> 340fps.
>
> ./ffmpeg_g -y -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:sharpness=50' -c:v
> h264_vaapi out.mp4
>
> -> 82fps. The output is wrong: it only includes the top-left four-ninths of
> the input, as if the scale hasn't happened. Can you reproduce this? Also,
> is this the expected performance level - it seems significantly slower than I
> would expect.
>
After talk with Intel-driver guys, the sharpness can't work with the other vpp
features, and I find a bug about this issue:
https://bugs.freedesktop.org/show_bug.cgi?id=96988
> ./ffmpeg_g -y -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 out.mp4
>
> -> 222fps. Output looks plausible.
>
> ./ffmpeg_g -y -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:sharpness=50:denoise=50'
> -c:v h264_vaapi out.mp4
>
> Aborts with:
>
> Do not support multiply filters outside vebox pipeline
> ffmpeg_g: ../../git/src/gen75_picture_process.c:358: gen75_proc_picture:
> Assertion `0' failed.
> Aborted
>
> I guess that means multiple filters aren't supported in the driver like this,
> though they certainly could be with the API design so excluding the
> possibility completely doesn't seem right. Can we query for this case to
> avoid hitting the abort?
>
Good idea, will add a check in next version patch.
> Thanks,
>
> - Mark
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
