[FFmpeg-devel] [PATCH 1/2] lavfi/vf_hwmap: make hwunmap from software frame work.
Mark Thompson
sw at jkqxz.net
Tue Dec 18 00:33:03 EET 2018
13/12/2018 01:50, Ruiling Song wrote:
> This patch was used to fix the second hwmap filter issue:
> [vaapi_frame] hwmap [software filters] hwmap [vaapi_frame]
> For such case, we also need to allocate the hardware frame
> and map it back to software.
>
> Signed-off-by: Ruiling Song <ruiling.song at intel.com>
> ---
> libavfilter/vf_hwmap.c | 125 +++++++++++++++++++++++++++++--------------------
> 1 file changed, 75 insertions(+), 50 deletions(-)
>
> diff --git a/libavfilter/vf_hwmap.c b/libavfilter/vf_hwmap.c
> index 290559a..03cb325 100644
> --- a/libavfilter/vf_hwmap.c
> +++ b/libavfilter/vf_hwmap.c
> @@ -50,6 +50,36 @@ static int hwmap_query_formats(AVFilterContext *avctx)
> return 0;
> }
>
> +static int create_hwframe_context(HWMapContext *ctx, AVFilterContext *avctx,
> + AVBufferRef *device, int format,
> + int sw_format, int width, int height)
> +{
> + int err;
> + AVHWFramesContext *frames;
> +
> + ctx->hwframes_ref = av_hwframe_ctx_alloc(device);
> + if (!ctx->hwframes_ref) {
> + return AVERROR(ENOMEM);
> + }
> + frames = (AVHWFramesContext*)ctx->hwframes_ref->data;
> +
> + frames->format = format;
> + frames->sw_format = sw_format;
> + frames->width = width;
> + frames->height = height;
> +
> + if (avctx->extra_hw_frames >= 0)
> + frames->initial_pool_size = 2 + avctx->extra_hw_frames;
> +
> + err = av_hwframe_ctx_init(ctx->hwframes_ref);
> + if (err < 0) {
> + av_log(avctx, AV_LOG_ERROR, "Failed to initialise "
> + "target frames context: %d.\n", err);
> + return err;
> + }
> + return 0;
> +}
> +
> static int hwmap_config_output(AVFilterLink *outlink)
> {
> AVFilterContext *avctx = outlink->src;
> @@ -130,29 +160,11 @@ static int hwmap_config_output(AVFilterLink *outlink)
> // overwrite the input hwframe context with a derived context
> // mapped from that back to the source type.
> AVBufferRef *source;
> - AVHWFramesContext *frames;
> -
> - ctx->hwframes_ref = av_hwframe_ctx_alloc(device);
> - if (!ctx->hwframes_ref) {
> - err = AVERROR(ENOMEM);
> + err = create_hwframe_context(ctx, avctx, device, outlink->format,
> + hwfc->sw_format, hwfc->width,
> + hwfc->height);
> + if (err < 0)
> goto fail;
> - }
> - frames = (AVHWFramesContext*)ctx->hwframes_ref->data;
> -
> - frames->format = outlink->format;
> - frames->sw_format = hwfc->sw_format;
> - frames->width = hwfc->width;
> - frames->height = hwfc->height;
> -
> - if (avctx->extra_hw_frames >= 0)
> - frames->initial_pool_size = 2 + avctx->extra_hw_frames;
> -
> - err = av_hwframe_ctx_init(ctx->hwframes_ref);
> - if (err < 0) {
> - av_log(avctx, AV_LOG_ERROR, "Failed to initialise "
> - "target frames context: %d.\n", err);
> - goto fail;
> - }
>
> err = av_hwframe_ctx_create_derived(&source,
> inlink->format,
> @@ -175,10 +187,20 @@ static int hwmap_config_output(AVFilterLink *outlink)
> inlink->hw_frames_ctx = source;
>
> } else if ((outlink->format == hwfc->format &&
> - inlink->format == hwfc->sw_format) ||
> - inlink->format == hwfc->format) {
> - // Map from a hardware format to a software format, or
> - // undo an existing such mapping.
> + inlink->format == hwfc->sw_format)) {
> + // unmap a software frame back to hardware
> + ctx->reverse = 1;
> + // incase user does not provide filter device, use the device_ref
> + // from inlink
> + if (!device)
> + device = hwfc->device_ref;
> +
> + err = create_hwframe_context(ctx, avctx, device, outlink->format,
> + inlink->format, inlink->w, inlink->h);
> + if (err < 0)
> + goto fail;
I don't think the unmap case here wants to make a new hardware frames context? You have a software frame which is actually a mapping of a hardware frame and want to retrieve the original hardware frame, so the frames context is that of the original frame.
This happens when making writeable mappings to make changes to a frame. Consider, for example:
./ffmpeg_g -y -hwaccel vaapi -hwaccel_device /dev/dri/renderD128 -hwaccel_output_format vaapi -i in.mp4 -an -vf 'scale_vaapi=format=nv12,hwmap=mode=read+write,format=nv12,drawtext=fontfile=font.ttf:text=Hello World!,format=nv12,hwmap' -c:v h264_vaapi out.mp4
> + } else if (inlink->format == hwfc->format) {
> + // Map from a hardware format to a software format
>
> ctx->hwframes_ref = av_buffer_ref(inlink->hw_frames_ctx);
> if (!ctx->hwframes_ref) {
> @@ -212,29 +234,10 @@ static int hwmap_config_output(AVFilterLink *outlink)
> }
>
> ctx->reverse = 1;
> -
> - ctx->hwframes_ref = av_hwframe_ctx_alloc(device);
> - if (!ctx->hwframes_ref) {
> - err = AVERROR(ENOMEM);
> - goto fail;
> - }
> - hwfc = (AVHWFramesContext*)ctx->hwframes_ref->data;
> -
> - hwfc->format = outlink->format;
> - hwfc->sw_format = inlink->format;
> - hwfc->width = inlink->w;
> - hwfc->height = inlink->h;
> -
> - if (avctx->extra_hw_frames >= 0)
> - hwfc->initial_pool_size = 2 + avctx->extra_hw_frames;
> -
> - err = av_hwframe_ctx_init(ctx->hwframes_ref);
> - if (err < 0) {
> - av_log(avctx, AV_LOG_ERROR, "Failed to create frame "
> - "context for reverse mapping: %d.\n", err);
> + err = create_hwframe_context(ctx, avctx, device, outlink->format,
> + inlink->format, inlink->w, inlink->h);
> + if (err < 0)
> goto fail;
> - }
> -
> } else {
> av_log(avctx, AV_LOG_ERROR, "Mapping requires a hardware "
> "context (a device, or frames on input).\n");
Merging the new context creation in the other two paths looks right to me.
> @@ -266,8 +269,17 @@ static AVFrame *hwmap_get_buffer(AVFilterLink *inlink, int w, int h)
> AVFilterContext *avctx = inlink->dst;
> AVFilterLink *outlink = avctx->outputs[0];
> HWMapContext *ctx = avctx->priv;
> + const AVPixFmtDescriptor *desc;
> +
> + desc = av_pix_fmt_desc_get(inlink->format);
> + if (!desc) {
> + av_log(avctx, AV_LOG_ERROR, "Unknown pix format %d\n", inlink->format);
> + return NULL;
> + }
>
> - if (ctx->reverse && !inlink->hw_frames_ctx) {
> + // if we are asking for software formats, we currently always
> + // allocate a hardware frame and map it reversely to software format.
> + if (!(desc->flags & AV_PIX_FMT_FLAG_HWACCEL)) {
> AVFrame *src, *dst;
> int err;
>
> @@ -306,12 +318,20 @@ static int hwmap_filter_frame(AVFilterLink *link, AVFrame *input)
> AVFilterLink *outlink = avctx->outputs[0];
> HWMapContext *ctx = avctx->priv;
> AVFrame *map = NULL;
> + const AVPixFmtDescriptor *desc;
> int err;
>
> av_log(ctx, AV_LOG_DEBUG, "Filter input: %s, %ux%u (%"PRId64").\n",
> av_get_pix_fmt_name(input->format),
> input->width, input->height, input->pts);
>
> + desc = av_pix_fmt_desc_get(input->format);
> + if (!desc) {
> + av_log(avctx, AV_LOG_ERROR, "Unknown pix format %d\n", input->format);
> + err = AVERROR(EINVAL);
> + goto fail;
> + }
> +
> map = av_frame_alloc();
> if (!map) {
> err = AVERROR(ENOMEM);
> @@ -319,7 +339,12 @@ static int hwmap_filter_frame(AVFilterLink *link, AVFrame *input)
> }
>
> map->format = outlink->format;
> - map->hw_frames_ctx = av_buffer_ref(ctx->hwframes_ref);
> + // The software frame may be mapped in another frame context,
> + // so we also do the unmap in that frame context.
> + if (!(desc->flags & AV_PIX_FMT_FLAG_HWACCEL) && input->hw_frames_ctx)
> + map->hw_frames_ctx = av_buffer_ref(input->hw_frames_ctx);
> + else
> + map->hw_frames_ctx = av_buffer_ref(ctx->hwframes_ref);
I'm not sure when this is hit. Maybe it is exactly the case where you made the unnecessary frames context above?
> if (!map->hw_frames_ctx) {
> err = AVERROR(ENOMEM);
> goto fail;
>
Thanks,
- Mark
More information about the ffmpeg-devel
mailing list