On 27/10/2023 19:46, David Rosca wrote:
On Fri, Oct 27, 2023 at 7:14 PM Mark Thompson <s...@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.
The case is not the same as the one earlier, because ...
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).
... that is not what the documentation says the function has to do:
"""
/**
* Map data store of the buffer into the client's address space
* this interface could be used to convey the operation hint
* backend driver could use these hint to optimize the implementations
*/
/** \brief VA_MAPBUFFER_FLAG_DEFAULT is used when there are no flag specified
* same as VA_MAPBUFFER_FLAG_READ | VA_MAPBUFFER_FLAG_WRITE.
*/
/** \brief application will read the surface after map */
#define VA_MAPBUFFER_FLAG_READ 1
/** \brief application will write the surface after map */
#define VA_MAPBUFFER_FLAG_WRITE 2
VAStatus vaMapBuffer2(
VADisplay dpy,
VABufferID buf_id, /* in */
void **pbuf, /* out */
uint32_t flags /* in */
);
"""
There is no suggestion here that setting WRITE & !READ implies that the user
has to completely overwrite the surface.
A user reading this would reasonably set write-only in the case where they want
to add an overlay, but your interpretation would make the driver discard the
rest of the image and give an unexpected result.
If you believe the function is intended to work this way then can you submit a
patch to libva to update the documentation to say what you expect? (Which can
also be seen by other driver implementers, so that they can comment on whether
the additional meaning you assign to the flags is something they would support
as well.)
Alternatively, it sounds like you would want to add an OVERWRITE flag to libva,
for exactly the same reasons that it exists already in ffmpeg.
+ 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-requ...@ffmpeg.org with subject "unsubscribe".
