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.s...@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 _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel