From 7ae6c203cb98982e62e13ea33df5db26a2b6d565 Mon Sep 17 00:00:00 2001
From: Philipp Zabel
Date: Fri, 5 Apr 2024 14:09:18 +0200
Subject: [PATCH] v4l2: bufferpool: Drop writable check on output pool process
Output buffers don't have to be writable. Accepting read-only buffers
from the V4L2 buffer pool allows upstream elements to write directly
into the V4L2 buffers without triggering a CPU copy into a new buffer
from the same V4L2 buffer pool every time.
Tested with the vivid output device:
GST_DEBUG=GST_PERFORMANCE:7 gst-launch-1.0 videotestsrc ! v4l2sink device=/dev/video5
With this change, gst_v4l2_buffer_pool_dqbuf() must be allowed to not
resize read-only memories of output buffers.
Part-of:
---
.../sys/v4l2/gstv4l2bufferpool.c | 45 ++++++++++---------
1 file changed, 25 insertions(+), 20 deletions(-)
diff --git a/subprojects/gst-plugins-good/sys/v4l2/gstv4l2bufferpool.c b/subprojects/gst-plugins-good/sys/v4l2/gstv4l2bufferpool.c
index f0dd1bf31e..49e6a5de3e 100644
--- a/subprojects/gst-plugins-good/sys/v4l2/gstv4l2bufferpool.c
+++ b/subprojects/gst-plugins-good/sys/v4l2/gstv4l2bufferpool.c
@@ -84,7 +84,8 @@ static void gst_v4l2_buffer_pool_complete_release_buffer (GstBufferPool * bpool,
GstBuffer * buffer, gboolean queued);
static gboolean
-gst_v4l2_is_buffer_valid (GstBuffer * buffer, GstV4l2MemoryGroup ** out_group)
+gst_v4l2_is_buffer_valid (GstBuffer * buffer, GstV4l2MemoryGroup ** out_group,
+ gboolean check_writable)
{
GstMemory *mem = gst_buffer_peek_memory (buffer, 0);
gboolean valid = FALSE;
@@ -108,7 +109,7 @@ gst_v4l2_is_buffer_valid (GstBuffer * buffer, GstV4l2MemoryGroup ** out_group)
if (group->mem[i] != gst_buffer_peek_memory (buffer, i))
goto done;
- if (!gst_memory_is_writable (group->mem[i]))
+ if (check_writable && !gst_memory_is_writable (group->mem[i]))
goto done;
}
@@ -127,7 +128,7 @@ gst_v4l2_buffer_pool_resize_buffer (GstBufferPool * bpool, GstBuffer * buffer)
GstV4l2BufferPool *pool = GST_V4L2_BUFFER_POOL (bpool);
GstV4l2MemoryGroup *group;
- if (gst_v4l2_is_buffer_valid (buffer, &group)) {
+ if (gst_v4l2_is_buffer_valid (buffer, &group, TRUE)) {
gst_v4l2_allocator_reset_group (pool->vallocator, group);
} else {
GST_BUFFER_FLAG_SET (buffer, GST_BUFFER_FLAG_TAG_MEMORY);
@@ -238,7 +239,7 @@ gst_v4l2_buffer_pool_import_userptr (GstV4l2BufferPool * pool,
GST_LOG_OBJECT (pool, "importing userptr");
/* get the group */
- if (!gst_v4l2_is_buffer_valid (dest, &group))
+ if (!gst_v4l2_is_buffer_valid (dest, &group, TRUE))
goto not_our_buffer;
if (V4L2_TYPE_IS_OUTPUT (pool->obj->type))
@@ -355,7 +356,7 @@ gst_v4l2_buffer_pool_import_dmabuf (GstV4l2BufferPool * pool,
GST_LOG_OBJECT (pool, "importing dmabuf");
- if (!gst_v4l2_is_buffer_valid (dest, &group))
+ if (!gst_v4l2_is_buffer_valid (dest, &group, TRUE))
goto not_our_buffer;
if (n_mem > GST_VIDEO_MAX_PLANES)
@@ -1312,15 +1313,18 @@ gst_v4l2_buffer_pool_dqbuf (GstV4l2BufferPool * pool, GstBuffer ** buffer,
if (GST_VIDEO_INFO_FORMAT (&pool->caps_info) == GST_VIDEO_FORMAT_ENCODED)
break;
- /* Ensure our offset matches the expected plane size, or image size if
- * there is only one memory */
- if (group->n_mem == 1) {
- gst_memory_resize (group->mem[0], 0, info->size + info->offset[0]);
- break;
- }
+ if (obj->type == V4L2_BUF_TYPE_VIDEO_CAPTURE ||
+ obj->type == V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE) {
+ /* Ensure our offset matches the expected plane size, or image size if
+ * there is only one memory */
+ if (group->n_mem == 1) {
+ gst_memory_resize (group->mem[0], 0, info->size + info->offset[0]);
+ break;
+ }
- if (!GST_VIDEO_FORMAT_INFO_IS_TILED (finfo))
- gst_memory_resize (group->mem[i], 0, obj->plane_size[i]);
+ if (!GST_VIDEO_FORMAT_INFO_IS_TILED (finfo))
+ gst_memory_resize (group->mem[i], 0, obj->plane_size[i]);
+ }
}
/* Ignore timestamp and field for OUTPUT device */
@@ -1520,7 +1524,7 @@ done:
/* Mark buffer as outstanding */
if (ret == GST_FLOW_OK) {
GstV4l2MemoryGroup *group;
- if (gst_v4l2_is_buffer_valid (*buffer, &group)) {
+ if (gst_v4l2_is_buffer_valid (*buffer, &group, TRUE)) {
GST_LOG_OBJECT (pool, "mark buffer %u outstanding", group->buffer.index);
g_atomic_int_or (&pool->buffer_state[group->buffer.index],
BUFFER_STATE_OUTSTANDING);
@@ -1571,7 +1575,7 @@ gst_v4l2_buffer_pool_complete_release_buffer (GstBufferPool * bpool,
case GST_V4L2_IO_DMABUF_IMPORT:
{
GstV4l2MemoryGroup *group;
- if (gst_v4l2_is_buffer_valid (buffer, &group)) {
+ if (gst_v4l2_is_buffer_valid (buffer, &group, TRUE)) {
GstFlowReturn ret = GST_FLOW_OK;
gst_v4l2_allocator_reset_group (pool->vallocator, group);
@@ -1612,7 +1616,7 @@ gst_v4l2_buffer_pool_complete_release_buffer (GstBufferPool * bpool,
GstV4l2MemoryGroup *group;
guint index;
- if (!gst_v4l2_is_buffer_valid (buffer, &group)) {
+ if (!gst_v4l2_is_buffer_valid (buffer, &group, TRUE)) {
/* Simply release invalid/modified buffer, the allocator will
* give it back later */
GST_BUFFER_FLAG_SET (buffer, GST_BUFFER_FLAG_TAG_MEMORY);
@@ -1663,7 +1667,7 @@ gst_v4l2_buffer_pool_release_buffer (GstBufferPool * bpool, GstBuffer * buffer)
GstV4l2MemoryGroup *group;
gboolean queued = FALSE;
- if (gst_v4l2_is_buffer_valid (buffer, &group)) {
+ if (gst_v4l2_is_buffer_valid (buffer, &group, TRUE)) {
gint old_buffer_state =
g_atomic_int_and (&pool->buffer_state[group->buffer.index],
~BUFFER_STATE_OUTSTANDING);
@@ -2065,7 +2069,8 @@ gst_v4l2_buffer_pool_process (GstV4l2BufferPool * pool, GstBuffer ** buf,
if ((*buf)->pool != bpool)
goto copying;
- if (!gst_v4l2_is_buffer_valid (*buf, &group))
+ /* Output buffers don't have to be writable */
+ if (!gst_v4l2_is_buffer_valid (*buf, &group, FALSE))
goto copying;
index = group->buffer.index;
@@ -2102,7 +2107,7 @@ gst_v4l2_buffer_pool_process (GstV4l2BufferPool * pool, GstBuffer ** buf,
}
/* retrieve the group */
- gst_v4l2_is_buffer_valid (to_queue, &group);
+ gst_v4l2_is_buffer_valid (to_queue, &group, TRUE);
}
if ((ret =
@@ -2115,7 +2120,7 @@ gst_v4l2_buffer_pool_process (GstV4l2BufferPool * pool, GstBuffer ** buf,
* streaming now */
if (!gst_v4l2_buffer_pool_streamon (pool)) {
/* don't check return value because qbuf would have failed */
- gst_v4l2_is_buffer_valid (to_queue, &group);
+ gst_v4l2_is_buffer_valid (to_queue, &group, TRUE);
/* qbuf has stored to_queue buffer but we are not in
* streaming state, so the flush logic won't be performed.