Re: [FFmpeg-devel] [PATCH v3] lavu/hwcontext_vaapi: Use vaMapBuffer2 for mapping image buffers

[David Rosca <nowrep@gmail.com>]

On Fri, Oct 27, 2023 at 7:14 PM Mark Thompson <sw@jkqxz.net> wrote:
>
> On 27/10/2023 11:00, David Rosca wrote:
> > This allows some optimizations in driver, such as not having to read
> > back the data if write-only mapping is requested.
> > ---
> > v3: Fix another warning
> >
> >   libavutil/hwcontext_vaapi.c | 11 +++++++++++
> >   1 file changed, 11 insertions(+)
> >
> > diff --git a/libavutil/hwcontext_vaapi.c b/libavutil/hwcontext_vaapi.c
> > index 558fed94c6..86b0852c12 100644
> > --- a/libavutil/hwcontext_vaapi.c
> > +++ b/libavutil/hwcontext_vaapi.c
> > @@ -799,6 +799,9 @@ static int vaapi_map_frame(AVHWFramesContext *hwfc,
> >       VAStatus vas;
> >       void *address = NULL;
> >       int err, i;
> > +#if VA_CHECK_VERSION(1, 21, 0)
> > +    uint32_t vaflags = 0;
> > +#endif
> >
> >       surface_id = (VASurfaceID)(uintptr_t)src->data[3];
> >       av_log(hwfc, AV_LOG_DEBUG, "Map surface %#x.\n", surface_id);
> > @@ -882,7 +885,15 @@ static int vaapi_map_frame(AVHWFramesContext *hwfc,
> >           }
> >       }
> >
> > +#if VA_CHECK_VERSION(1, 21, 0)
> > +    if (flags & AV_HWFRAME_MAP_READ || !(flags & AV_HWFRAME_MAP_OVERWRITE))
> > +        vaflags |= VA_MAPBUFFER_FLAG_READ;
>
> I don't understand where the !overwrite has come from in this condition?

This logic is a couple lines ahead in the vaCreateImage path. If
AV_HWFRAME_MAP_OVERWRITE isn't set, it will call vaGetImage to read
the image data. And as vaDeriveImage + vaMapBuffer is read+write
mapping, I think the same logic needs to be applied to vaMapBuffer2
too.

>
> If the user requested write-only but not overwrite then they're expecting to write some pixels within the image (such as adding an overlay), but don't want to read anything.

Exactly for this case the read is needed. If the user writes only some
(not all) pixels of the image, then the rest of the image will be
invalid if a driver implements the mapping using staging texture
(which is what Mesa does).

>
> > +    if (flags & AV_HWFRAME_MAP_WRITE)
> > +        vaflags |= VA_MAPBUFFER_FLAG_WRITE;
> > +    vas = vaMapBuffer2(hwctx->display, map->image.buf, &address, vaflags);
> > +#else
> >       vas = vaMapBuffer(hwctx->display, map->image.buf, &address);
> > +#endif
> >       if (vas != VA_STATUS_SUCCESS) {
> >           av_log(hwfc, AV_LOG_ERROR, "Failed to map image from surface "
> >                  "%#x: %d (%s).\n", surface_id, vas, vaErrorStr(vas));
>
> Please add a note that there is a compatibility layer in libva so that MapBuffer2 calls MapBuffer if the driver doesn't expose it directly, so this does work with older drivers.  (The patch looked wrong before I realised that.)
>
> Thanks,
>
> - Mark
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".