> -----Original Message-----
> From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On Behalf Of
> Mark Thompson
> Sent: Tuesday, December 18, 2018 6:33 AM
> To: ffmpeg-devel@ffmpeg.org
> Subject: Re: [FFmpeg-devel] [PATCH 1/2] lavfi/vf_hwmap: make hwunmap from
> software frame work.
>
> 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.
The drawtext filter does directly write to that mapped frame, thank you for
showing me that.
But I think still there are many other filters that do not directly write to
the input frame.
I am creating the frames context for the latter case that ask for a new frame
from output link. Consider a simple transpose.
./ffmpeg_g -y -loglevel error -init_hw_device vaapi=va:/dev/dri/renderD128
-hwaccel vaapi -hwaccel_device va -hwaccel_output_format vaapi -i input.mp4
-an -filter_complex 'hwmap,transpose=dir=1,format=nv12,hwmap' -c:v h264_vaapi
out.mp4
>
> 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=fo
> ntfile=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?
This is hit when a new frames context was created for this hwmap filter but the
input frame was mapped in a previous hwmap, consider the passthrough mode in
transpose filter. Or the drawtext filter you mentioned, now that I am creating
a new frames context, this ctx->hwframes_ref is not the same as the
input->hw_frames_ctx.
I am writing the patch so that user could freely use software filters with
vappi codec pipeline.
Thanks!
Ruiling
>
> > 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
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
