All of lore.kernel.org
 help / color / mirror / Atom feed
From: Hans Verkuil <hverkuil@xs4all.nl>
To: Linux Media Mailing List <linux-media@vger.kernel.org>,
	Tomasz Figa <tfiga@chromium.org>,
	Alexandre Courbot <acourbot@chromium.org>,
	Paul Kocialkowski <paul.kocialkowski@bootlin.com>
Subject: [PATCH] vb2: clear timestamp if buffer mem is reacquired
Date: Sat, 2 Feb 2019 18:03:47 +0100	[thread overview]
Message-ID: <59fc9777-41f3-5f87-2d84-f9375d8a2895@xs4all.nl> (raw)

Stateless codecs have to find buffers based on a timestamp (vb2_find_timestamp).
The timestamp is set to 0 initially, so prohibit finding timestamp 0 since it
could find unused buffers without associated memory (userptr or dmabuf).

The memory associated with a buffer will also disappear if the same buffer was
requeued with a different userptr address or dmabuf fd. Detect this and set the
timestamp of that buffer to 0 if this happens.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
Note: I think it is still necessary to lock a buffer when it is in use as
a reference frame, otherwise a userspace application can queue it again with
a different dmabuf fd, which could free the memory of the old dmabuf.

vb2_find_buffer should probably do that.
---
diff --git a/drivers/media/common/videobuf2/videobuf2-core.c b/drivers/media/common/videobuf2/videobuf2-core.c
index e07b6bdb6982..b664d9790330 100644
--- a/drivers/media/common/videobuf2/videobuf2-core.c
+++ b/drivers/media/common/videobuf2/videobuf2-core.c
@@ -1043,6 +1043,8 @@ static int __prepare_userptr(struct vb2_buffer *vb)
 				reacquired = true;
 				call_void_vb_qop(vb, buf_cleanup, vb);
 			}
+			if (!q->is_output)
+				vb->timestamp = 0;
 			call_void_memop(vb, put_userptr, vb->planes[plane].mem_priv);
 		}

@@ -1157,6 +1159,8 @@ static int __prepare_dmabuf(struct vb2_buffer *vb)
 		/* Skip the plane if already verified */
 		if (dbuf == vb->planes[plane].dbuf &&
 			vb->planes[plane].length == planes[plane].length) {
+			if (!q->is_output)
+				vb->timestamp = 0;
 			dma_buf_put(dbuf);
 			continue;
 		}
diff --git a/drivers/media/common/videobuf2/videobuf2-v4l2.c b/drivers/media/common/videobuf2/videobuf2-v4l2.c
index 3aeaea3af42a..8e966fa81b7e 100644
--- a/drivers/media/common/videobuf2/videobuf2-v4l2.c
+++ b/drivers/media/common/videobuf2/videobuf2-v4l2.c
@@ -603,6 +603,9 @@ int vb2_find_timestamp(const struct vb2_queue *q, u64 timestamp,
 {
 	unsigned int i;

+	if (!timestamp)
+		return -1;
+
 	for (i = start_idx; i < q->num_buffers; i++)
 		if (q->bufs[i]->timestamp == timestamp)
 			return i;
diff --git a/include/media/videobuf2-v4l2.h b/include/media/videobuf2-v4l2.h
index 8a10889dc2fd..01bf4b2199c7 100644
--- a/include/media/videobuf2-v4l2.h
+++ b/include/media/videobuf2-v4l2.h
@@ -59,14 +59,14 @@ struct vb2_v4l2_buffer {
  * vb2_find_timestamp() - Find buffer with given timestamp in the queue
  *
  * @q:		pointer to &struct vb2_queue with videobuf2 queue.
- * @timestamp:	the timestamp to find.
+ * @timestamp:	the timestamp to find. Must be > 0.
  * @start_idx:	the start index (usually 0) in the buffer array to start
  *		searching from. Note that there may be multiple buffers
  *		with the same timestamp value, so you can restart the search
  *		by setting @start_idx to the previously found index + 1.
  *
  * Returns the buffer index of the buffer with the given @timestamp, or
- * -1 if no buffer with @timestamp was found.
+ * -1 if no buffer with @timestamp was found or if @timestamp was 0.
  */
 int vb2_find_timestamp(const struct vb2_queue *q, u64 timestamp,
 		       unsigned int start_idx);

             reply	other threads:[~2019-02-02 17:03 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-02-02 17:03 Hans Verkuil [this message]
2019-02-02 20:59 ` [PATCH] vb2: clear timestamp if buffer mem is reacquired Nicolas Dufresne
2019-02-03  8:23   ` Hans Verkuil

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=59fc9777-41f3-5f87-2d84-f9375d8a2895@xs4all.nl \
    --to=hverkuil@xs4all.nl \
    --cc=acourbot@chromium.org \
    --cc=linux-media@vger.kernel.org \
    --cc=paul.kocialkowski@bootlin.com \
    --cc=tfiga@chromium.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.