On 12/01/17 08:01, wm4 wrote: > On Sun, 8 Jan 2017 19:12:47 +0000 > Mark Thompson <s...@jkqxz.net> wrote: > >> (cherry picked from commit ade370a4d7eab1866b6023c91c135d27c77ca465) >> --- >> One minor fixup for allocation due to differences in the lavfis, otherwise >> unchanged. >> >> configure | 1 + >> libavfilter/Makefile | 1 + >> libavfilter/allfilters.c | 1 + >> libavfilter/version.h | 2 +- >> libavfilter/vf_deinterlace_vaapi.c | 630 >> +++++++++++++++++++++++++++++++++++++ >> 5 files changed, 634 insertions(+), 1 deletion(-) >> create mode 100644 libavfilter/vf_deinterlace_vaapi.c >> ... >> + >> +static int deint_vaapi_config_output(AVFilterLink *outlink) >> +{ >> + AVFilterContext *avctx = outlink->src; >> + DeintVAAPIContext *ctx = avctx->priv; >> + AVVAAPIHWConfig *hwconfig = NULL; >> + AVHWFramesConstraints *constraints = NULL; >> + AVVAAPIFramesContext *va_frames; >> + VAStatus vas; >> + int err; >> + >> + deint_vaapi_pipeline_uninit(avctx); >> + >> + av_assert0(ctx->input_frames); >> + ctx->device_ref = av_buffer_ref(ctx->input_frames->device_ref); >> + ctx->hwctx = ((AVHWDeviceContext*)ctx->device_ref->data)->hwctx; >> + >> + ctx->output_width = ctx->input_frames->width; >> + ctx->output_height = ctx->input_frames->height; >> + >> + av_assert0(ctx->va_config == VA_INVALID_ID); >> + vas = vaCreateConfig(ctx->hwctx->display, VAProfileNone, >> + VAEntrypointVideoProc, 0, 0, &ctx->va_config); >> + if (vas != VA_STATUS_SUCCESS) { >> + av_log(avctx, AV_LOG_ERROR, "Failed to create processing pipeline " >> + "config: %d (%s).\n", vas, vaErrorStr(vas)); >> + err = AVERROR(EIO); >> + goto fail; >> + } >> + >> + hwconfig = av_hwdevice_hwconfig_alloc(ctx->device_ref); >> + if (!hwconfig) { >> + err = AVERROR(ENOMEM); >> + goto fail; >> + } >> + hwconfig->config_id = ctx->va_config; >> + >> + constraints = av_hwdevice_get_hwframe_constraints(ctx->device_ref, >> + hwconfig); >> + if (!constraints) { >> + err = AVERROR(ENOMEM); >> + goto fail; >> + } >> + >> + if (ctx->output_width < constraints->min_width || >> + ctx->output_height < constraints->min_height || >> + ctx->output_width > constraints->max_width || >> + ctx->output_height > constraints->max_height) { >> + av_log(avctx, AV_LOG_ERROR, "Hardware does not support " >> + "deinterlacing to size %dx%d " >> + "(constraints: width %d-%d height %d-%d).\n", >> + ctx->output_width, ctx->output_height, >> + constraints->min_width, constraints->max_width, >> + constraints->min_height, constraints->max_height); >> + err = AVERROR(EINVAL); >> + goto fail; >> + } >> + >> + err = deint_vaapi_build_filter_params(avctx); >> + if (err < 0) >> + goto fail; >> + >> + ctx->output_frames_ref = av_hwframe_ctx_alloc(ctx->device_ref); >> + if (!ctx->output_frames_ref) { >> + av_log(avctx, AV_LOG_ERROR, "Failed to create HW frame context " >> + "for output.\n"); >> + err = AVERROR(ENOMEM); >> + goto fail; >> + } >> + >> + ctx->output_frames = (AVHWFramesContext*)ctx->output_frames_ref->data; >> + >> + ctx->output_frames->format = AV_PIX_FMT_VAAPI; >> + ctx->output_frames->sw_format = ctx->input_frames->sw_format; >> + ctx->output_frames->width = ctx->output_width; >> + ctx->output_frames->height = ctx->output_height; >> + >> + // The number of output frames we need is determined by what follows >> + // the filter. If it's an encoder with complex frame reference >> + // structures then this could be very high. >> + ctx->output_frames->initial_pool_size = 10; > > This seems less than ideal. We should probably have some concept to > handle this issue. Until then, this should probably be a user > configurable option. It looks like this could waste GPU memory, so it > seems important enough.
Yes, we need a generic option somewhere - most hardware filters and decoders want it (not just VAAPI - QSV really needs it too, especially with the look-ahead options which can buffer arbitrarily many frames in the encoder). libav have talked about this recently, we should probably follow it up there. On the specific option, I don't much like the idea of adding that now because we would have to continue to support it later? > But is there really a need to allocate all the surfaces upfront? I > hoped this to be an issue with decoders only. > > (In my experience, I never needed this for VPP.) The API says it's required, so we do it. Not sure we can sanely do anything else? >> + err = av_hwframe_ctx_init(ctx->output_frames_ref); >> + if (err < 0) { >> + av_log(avctx, AV_LOG_ERROR, "Failed to initialise VAAPI frame " >> + "context for output: %d\n", err); >> + goto fail; >> + } >> + >> + va_frames = ctx->output_frames->hwctx; >> + >> + av_assert0(ctx->va_context == VA_INVALID_ID); >> + vas = vaCreateContext(ctx->hwctx->display, ctx->va_config, >> + ctx->output_width, ctx->output_height, 0, >> + va_frames->surface_ids, va_frames->nb_surfaces, >> + &ctx->va_context); >> + if (vas != VA_STATUS_SUCCESS) { >> + av_log(avctx, AV_LOG_ERROR, "Failed to create processing pipeline " >> + "context: %d (%s).\n", vas, vaErrorStr(vas)); >> + return AVERROR(EIO); >> + } >> + >> + outlink->w = ctx->output_width; >> + outlink->h = ctx->output_height; >> + >> + outlink->hw_frames_ctx = av_buffer_ref(ctx->output_frames_ref); >> + if (!outlink->hw_frames_ctx) { >> + err = AVERROR(ENOMEM); >> + goto fail; >> + } >> + >> + av_freep(&hwconfig); >> + av_hwframe_constraints_free(&constraints); >> + return 0; >> + >> +fail: >> + av_buffer_unref(&ctx->output_frames_ref); >> + av_freep(&hwconfig); >> + av_hwframe_constraints_free(&constraints); >> + return err; >> +} >> + >> +static int vaapi_proc_colour_standard(enum AVColorSpace av_cs) >> +{ >> + switch(av_cs) { >> +#define CS(av, va) case AVCOL_SPC_ ## av: return VAProcColorStandard ## va; >> + CS(BT709, BT709); >> + CS(BT470BG, BT470BG); >> + CS(SMPTE170M, SMPTE170M); >> + CS(SMPTE240M, SMPTE240M); >> +#undef CS >> + default: >> + return VAProcColorStandardNone; >> + } >> +} > > I bet there's something like this in the encoder code too? The encoder doesn't deal with it, because we write the headers ourselves. It is identical to a fragment in vf_scale_vaapi, though, yes. >> + >> +static int deint_vaapi_filter_frame(AVFilterLink *inlink, AVFrame >> *input_frame) >> +{ >> + AVFilterContext *avctx = inlink->dst; >> + AVFilterLink *outlink = avctx->outputs[0]; >> + DeintVAAPIContext *ctx = avctx->priv; >> + AVFrame *output_frame = NULL; >> + VASurfaceID input_surface, output_surface; >> + VASurfaceID backward_references[MAX_REFERENCES]; >> + VASurfaceID forward_references[MAX_REFERENCES]; >> + VAProcPipelineParameterBuffer params; >> + VAProcFilterParameterBufferDeinterlacing *filter_params; >> + VARectangle input_region; >> + VABufferID params_id; >> + VAStatus vas; >> + void *filter_params_addr = NULL; >> + int err, i; >> + >> + av_log(avctx, AV_LOG_DEBUG, "Filter input: %s, %ux%u (%"PRId64").\n", >> + av_get_pix_fmt_name(input_frame->format), >> + input_frame->width, input_frame->height, input_frame->pts); >> + >> + if (ctx->queue_count < ctx->queue_depth) { >> + ctx->frame_queue[ctx->queue_count++] = input_frame; >> + if (ctx->queue_count < ctx->queue_depth) { >> + // Need more reference surfaces before we can continue. >> + return 0; > > Does this handle close-to-EOF situations? The filter produces queue_depth fewer output frames than input, so yes. (If you only have a small number of frames and need more references than that, you get no output - sucks to be you.) >> + } >> + } else { >> + av_frame_free(&ctx->frame_queue[0]); >> + for (i = 0; i + 1 < ctx->queue_count; i++) >> + ctx->frame_queue[i] = ctx->frame_queue[i + 1]; >> + ctx->frame_queue[i] = input_frame; >> + } >> + >> + input_frame = >> + ctx->frame_queue[ctx->pipeline_caps.num_backward_references]; >> + input_surface = (VASurfaceID)(uintptr_t)input_frame->data[3]; >> + for (i = 0; i < ctx->pipeline_caps.num_backward_references; i++) >> + backward_references[i] = (VASurfaceID)(uintptr_t) >> + ctx->frame_queue[ctx->pipeline_caps.num_backward_references - >> + i - 1]->data[3]; >> + for (i = 0; i < ctx->pipeline_caps.num_forward_references; i++) >> + forward_references[i] = (VASurfaceID)(uintptr_t) >> + ctx->frame_queue[ctx->pipeline_caps.num_backward_references + >> + i + 1]->data[3]; >> + >> + av_log(avctx, AV_LOG_DEBUG, "Using surface %#x for " >> + "deinterlace input.\n", input_surface); >> + av_log(avctx, AV_LOG_DEBUG, "Backward references:"); >> + for (i = 0; i < ctx->pipeline_caps.num_backward_references; i++) >> + av_log(avctx, AV_LOG_DEBUG, " %#x", backward_references[i]); >> + av_log(avctx, AV_LOG_DEBUG, "\n"); >> + av_log(avctx, AV_LOG_DEBUG, "Forward references:"); >> + for (i = 0; i < ctx->pipeline_caps.num_forward_references; i++) >> + av_log(avctx, AV_LOG_DEBUG, " %#x", forward_references[i]); >> + av_log(avctx, AV_LOG_DEBUG, "\n"); >> + >> + output_frame = av_frame_alloc(); >> + if (!output_frame) { >> + err = AVERROR(ENOMEM); >> + goto fail; >> + } >> + >> + err = av_hwframe_get_buffer(ctx->output_frames_ref, >> + output_frame, 0); >> + if (err < 0) { >> + err = AVERROR(ENOMEM); >> + goto fail; >> + } >> + >> + output_surface = (VASurfaceID)(uintptr_t)output_frame->data[3]; >> + av_log(avctx, AV_LOG_DEBUG, "Using surface %#x for " >> + "deinterlace output.\n", output_surface); >> + >> + memset(¶ms, 0, sizeof(params)); >> + >> + input_region = (VARectangle) { >> + .x = 0, >> + .y = 0, >> + .width = input_frame->width, >> + .height = input_frame->height, >> + }; >> + >> + params.surface = input_surface; >> + params.surface_region = &input_region; >> + params.surface_color_standard = >> + vaapi_proc_colour_standard(input_frame->colorspace); >> + >> + params.output_region = NULL; >> + params.output_background_color = 0xff000000; >> + params.output_color_standard = params.surface_color_standard; >> + >> + params.pipeline_flags = 0; >> + params.filter_flags = VA_FRAME_PICTURE; >> + >> + vas = vaMapBuffer(ctx->hwctx->display, ctx->filter_buffer, >> + &filter_params_addr); >> + if (vas != VA_STATUS_SUCCESS) { >> + av_log(avctx, AV_LOG_ERROR, "Failed to map filter parameter " >> + "buffer: %d (%s).\n", vas, vaErrorStr(vas)); >> + err = AVERROR(EIO); >> + goto fail; >> + } >> + filter_params = filter_params_addr; >> + filter_params->flags = 0; >> + if (input_frame->interlaced_frame && !input_frame->top_field_first) >> + filter_params->flags |= VA_DEINTERLACING_BOTTOM_FIELD_FIRST; >> + filter_params_addr = NULL; >> + vas = vaUnmapBuffer(ctx->hwctx->display, ctx->filter_buffer); >> + if (vas != VA_STATUS_SUCCESS) >> + av_log(avctx, AV_LOG_ERROR, "Failed to unmap filter parameter " >> + "buffer: %d (%s).\n", vas, vaErrorStr(vas)); >> + >> + params.filters = &ctx->filter_buffer; >> + params.num_filters = 1; >> + >> + params.forward_references = forward_references; >> + params.num_forward_references = >> + ctx->pipeline_caps.num_forward_references; >> + params.backward_references = backward_references; >> + params.num_backward_references = >> + ctx->pipeline_caps.num_backward_references; >> + >> + vas = vaBeginPicture(ctx->hwctx->display, >> + ctx->va_context, output_surface); >> + if (vas != VA_STATUS_SUCCESS) { >> + av_log(avctx, AV_LOG_ERROR, "Failed to attach new picture: " >> + "%d (%s).\n", vas, vaErrorStr(vas)); >> + err = AVERROR(EIO); >> + goto fail; >> + } >> + >> + vas = vaCreateBuffer(ctx->hwctx->display, ctx->va_context, >> + VAProcPipelineParameterBufferType, >> + sizeof(params), 1, ¶ms, ¶ms_id); >> + if (vas != VA_STATUS_SUCCESS) { >> + av_log(avctx, AV_LOG_ERROR, "Failed to create parameter buffer: " >> + "%d (%s).\n", vas, vaErrorStr(vas)); >> + err = AVERROR(EIO); >> + goto fail_after_begin; >> + } >> + av_log(avctx, AV_LOG_DEBUG, "Pipeline parameter buffer is %#x.\n", >> + params_id); >> + >> + vas = vaRenderPicture(ctx->hwctx->display, ctx->va_context, >> + ¶ms_id, 1); >> + if (vas != VA_STATUS_SUCCESS) { >> + av_log(avctx, AV_LOG_ERROR, "Failed to render parameter buffer: " >> + "%d (%s).\n", vas, vaErrorStr(vas)); >> + err = AVERROR(EIO); >> + goto fail_after_begin; >> + } >> + >> + vas = vaEndPicture(ctx->hwctx->display, ctx->va_context); >> + if (vas != VA_STATUS_SUCCESS) { >> + av_log(avctx, AV_LOG_ERROR, "Failed to start picture processing: " >> + "%d (%s).\n", vas, vaErrorStr(vas)); >> + err = AVERROR(EIO); >> + goto fail_after_render; >> + } >> + >> + if (ctx->hwctx->driver_quirks & >> + AV_VAAPI_DRIVER_QUIRK_RENDER_PARAM_BUFFERS) { >> + vas = vaDestroyBuffer(ctx->hwctx->display, params_id); >> + if (vas != VA_STATUS_SUCCESS) { >> + av_log(avctx, AV_LOG_ERROR, "Failed to free parameter buffer: " >> + "%d (%s).\n", vas, vaErrorStr(vas)); >> + // And ignore. >> + } >> + } >> + >> + err = av_frame_copy_props(output_frame, input_frame); >> + if (err < 0) >> + goto fail; >> + >> + av_log(avctx, AV_LOG_DEBUG, "Filter output: %s, %ux%u (%"PRId64").\n", >> + av_get_pix_fmt_name(output_frame->format), >> + output_frame->width, output_frame->height, output_frame->pts); >> + >> + return ff_filter_frame(outlink, output_frame); >> + >> +fail_after_begin: >> + vaRenderPicture(ctx->hwctx->display, ctx->va_context, ¶ms_id, 1); >> +fail_after_render: >> + vaEndPicture(ctx->hwctx->display, ctx->va_context); >> +fail: >> + if (filter_params_addr) >> + vaUnmapBuffer(ctx->hwctx->display, ctx->filter_buffer); >> + av_frame_free(&output_frame); >> + return err; >> +} >> + >> ... >> + >> +AVFilter ff_vf_deinterlace_vaapi = { >> + .name = "deinterlace_vaapi", >> + .description = NULL_IF_CONFIG_SMALL("Deinterlacing of VAAPI >> surfaces"), >> + .priv_size = sizeof(DeintVAAPIContext), >> + .init = &deint_vaapi_init, >> + .uninit = &deint_vaapi_uninit, >> + .query_formats = &deint_vaapi_query_formats, >> + .inputs = deint_vaapi_inputs, >> + .outputs = deint_vaapi_outputs, >> + .priv_class = &deint_vaapi_class, >> +}; > > Would there be any benefit in conflating this filter with the scale > filter, or is that strictly unnecessary? I know vdpau's API conflates > them, but maybe that was done for simplicity. There is some common code, but I don't think they should be the same filter - the structure and options are sufficiently different that it would be kindof annoying. I could see making libavfilter/vaapi.[ch] files with common stuff in, though. (Also for the "miscellaneous other processing" filter (denoise, sharpen, etc.), if anyone wanted to chase that up.) Thanks, - Mark _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel