[PATCHv2,2/3] vb2: keep track of timestamp status

Message ID 20190204101134.56283-3-hverkuil-cisco@xs4all.nl (mailing list archive)
State Accepted, archived
Delegated to: Hans Verkuil
Headers

Commit Message

Hans Verkuil Feb. 4, 2019, 10:11 a.m. UTC
  From: Hans Verkuil <hverkuil-cisco@xs4all.nl>

If a stream is stopped, or if a USERPTR/DMABUF buffer is queued
backed by a different user address or dmabuf fd, then the timestamp
should be skipped by vb2_find_timestamp since the memory it refers
to is no longer valid.

So keep track of a 'copied_timestamp' state: it is set when the
timestamp is copied from an output to a capture buffer, and is
cleared when it is no longer valid.

Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
---
 drivers/media/common/videobuf2/videobuf2-core.c | 3 +++
 drivers/media/common/videobuf2/videobuf2-v4l2.c | 3 ++-
 drivers/media/v4l2-core/v4l2-mem2mem.c          | 1 +
 include/media/videobuf2-core.h                  | 3 +++
 4 files changed, 9 insertions(+), 1 deletion(-)
  

Comments

Paul Kocialkowski Feb. 13, 2019, 9:22 a.m. UTC | #1
Hi,

On Mon, 2019-02-04 at 11:11 +0100, hverkuil-cisco@xs4all.nl wrote:
> From: Hans Verkuil <hverkuil-cisco@xs4all.nl>
> 
> If a stream is stopped, or if a USERPTR/DMABUF buffer is queued
> backed by a different user address or dmabuf fd, then the timestamp
> should be skipped by vb2_find_timestamp since the memory it refers
> to is no longer valid.
> 
> So keep track of a 'copied_timestamp' state: it is set when the
> timestamp is copied from an output to a capture buffer, and is
> cleared when it is no longer valid.

Reviewed-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>

Cheers,

Paul

> Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
> ---
>  drivers/media/common/videobuf2/videobuf2-core.c | 3 +++
>  drivers/media/common/videobuf2/videobuf2-v4l2.c | 3 ++-
>  drivers/media/v4l2-core/v4l2-mem2mem.c          | 1 +
>  include/media/videobuf2-core.h                  | 3 +++
>  4 files changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/media/common/videobuf2/videobuf2-core.c b/drivers/media/common/videobuf2/videobuf2-core.c
> index 35cf36686e20..dc29bd01d6c5 100644
> --- a/drivers/media/common/videobuf2/videobuf2-core.c
> +++ b/drivers/media/common/videobuf2/videobuf2-core.c
> @@ -1041,6 +1041,7 @@ static int __prepare_userptr(struct vb2_buffer *vb)
>  		if (vb->planes[plane].mem_priv) {
>  			if (!reacquired) {
>  				reacquired = true;
> +				vb->copied_timestamp = 0;
>  				call_void_vb_qop(vb, buf_cleanup, vb);
>  			}
>  			call_void_memop(vb, put_userptr, vb->planes[plane].mem_priv);
> @@ -1165,6 +1166,7 @@ static int __prepare_dmabuf(struct vb2_buffer *vb)
>  
>  		if (!reacquired) {
>  			reacquired = true;
> +			vb->copied_timestamp = 0;
>  			call_void_vb_qop(vb, buf_cleanup, vb);
>  		}
>  
> @@ -1943,6 +1945,7 @@ static void __vb2_queue_cancel(struct vb2_queue *q)
>  		if (vb->request)
>  			media_request_put(vb->request);
>  		vb->request = NULL;
> +		vb->copied_timestamp = 0;
>  	}
>  }
>  
> diff --git a/drivers/media/common/videobuf2/videobuf2-v4l2.c b/drivers/media/common/videobuf2/videobuf2-v4l2.c
> index 3aeaea3af42a..55277370c313 100644
> --- a/drivers/media/common/videobuf2/videobuf2-v4l2.c
> +++ b/drivers/media/common/videobuf2/videobuf2-v4l2.c
> @@ -604,7 +604,8 @@ int vb2_find_timestamp(const struct vb2_queue *q, u64 timestamp,
>  	unsigned int i;
>  
>  	for (i = start_idx; i < q->num_buffers; i++)
> -		if (q->bufs[i]->timestamp == timestamp)
> +		if (q->bufs[i]->copied_timestamp &&
> +		    q->bufs[i]->timestamp == timestamp)
>  			return i;
>  	return -1;
>  }
> diff --git a/drivers/media/v4l2-core/v4l2-mem2mem.c b/drivers/media/v4l2-core/v4l2-mem2mem.c
> index 631f4e2aa942..64b19ee1c847 100644
> --- a/drivers/media/v4l2-core/v4l2-mem2mem.c
> +++ b/drivers/media/v4l2-core/v4l2-mem2mem.c
> @@ -992,6 +992,7 @@ void v4l2_m2m_buf_copy_data(const struct vb2_v4l2_buffer *out_vb,
>  	cap_vb->field = out_vb->field;
>  	cap_vb->flags &= ~mask;
>  	cap_vb->flags |= out_vb->flags & mask;
> +	cap_vb->vb2_buf.copied_timestamp = 1;
>  }
>  EXPORT_SYMBOL_GPL(v4l2_m2m_buf_copy_data);
>  
> diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
> index 2757d1902609..62488d901747 100644
> --- a/include/media/videobuf2-core.h
> +++ b/include/media/videobuf2-core.h
> @@ -262,6 +262,8 @@ struct vb2_buffer {
>  	 * prepared:		this buffer has been prepared, i.e. the
>  	 *			buf_prepare op was called. It is cleared again
>  	 *			after the 'buf_finish' op is called.
> +	 * copied_timestamp:	the timestamp of this capture buffer was copied
> +	 *			from an output buffer.
>  	 * queued_entry:	entry on the queued buffers list, which holds
>  	 *			all buffers queued from userspace
>  	 * done_entry:		entry on the list that stores all buffers ready
> @@ -271,6 +273,7 @@ struct vb2_buffer {
>  	enum vb2_buffer_state	state;
>  	unsigned int		synced:1;
>  	unsigned int		prepared:1;
> +	unsigned int		copied_timestamp:1;
>  
>  	struct vb2_plane	planes[VB2_MAX_PLANES];
>  	struct list_head	queued_entry;
  

Patch

diff --git a/drivers/media/common/videobuf2/videobuf2-core.c b/drivers/media/common/videobuf2/videobuf2-core.c
index 35cf36686e20..dc29bd01d6c5 100644
--- a/drivers/media/common/videobuf2/videobuf2-core.c
+++ b/drivers/media/common/videobuf2/videobuf2-core.c
@@ -1041,6 +1041,7 @@  static int __prepare_userptr(struct vb2_buffer *vb)
 		if (vb->planes[plane].mem_priv) {
 			if (!reacquired) {
 				reacquired = true;
+				vb->copied_timestamp = 0;
 				call_void_vb_qop(vb, buf_cleanup, vb);
 			}
 			call_void_memop(vb, put_userptr, vb->planes[plane].mem_priv);
@@ -1165,6 +1166,7 @@  static int __prepare_dmabuf(struct vb2_buffer *vb)
 
 		if (!reacquired) {
 			reacquired = true;
+			vb->copied_timestamp = 0;
 			call_void_vb_qop(vb, buf_cleanup, vb);
 		}
 
@@ -1943,6 +1945,7 @@  static void __vb2_queue_cancel(struct vb2_queue *q)
 		if (vb->request)
 			media_request_put(vb->request);
 		vb->request = NULL;
+		vb->copied_timestamp = 0;
 	}
 }
 
diff --git a/drivers/media/common/videobuf2/videobuf2-v4l2.c b/drivers/media/common/videobuf2/videobuf2-v4l2.c
index 3aeaea3af42a..55277370c313 100644
--- a/drivers/media/common/videobuf2/videobuf2-v4l2.c
+++ b/drivers/media/common/videobuf2/videobuf2-v4l2.c
@@ -604,7 +604,8 @@  int vb2_find_timestamp(const struct vb2_queue *q, u64 timestamp,
 	unsigned int i;
 
 	for (i = start_idx; i < q->num_buffers; i++)
-		if (q->bufs[i]->timestamp == timestamp)
+		if (q->bufs[i]->copied_timestamp &&
+		    q->bufs[i]->timestamp == timestamp)
 			return i;
 	return -1;
 }
diff --git a/drivers/media/v4l2-core/v4l2-mem2mem.c b/drivers/media/v4l2-core/v4l2-mem2mem.c
index 631f4e2aa942..64b19ee1c847 100644
--- a/drivers/media/v4l2-core/v4l2-mem2mem.c
+++ b/drivers/media/v4l2-core/v4l2-mem2mem.c
@@ -992,6 +992,7 @@  void v4l2_m2m_buf_copy_data(const struct vb2_v4l2_buffer *out_vb,
 	cap_vb->field = out_vb->field;
 	cap_vb->flags &= ~mask;
 	cap_vb->flags |= out_vb->flags & mask;
+	cap_vb->vb2_buf.copied_timestamp = 1;
 }
 EXPORT_SYMBOL_GPL(v4l2_m2m_buf_copy_data);
 
diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
index 2757d1902609..62488d901747 100644
--- a/include/media/videobuf2-core.h
+++ b/include/media/videobuf2-core.h
@@ -262,6 +262,8 @@  struct vb2_buffer {
 	 * prepared:		this buffer has been prepared, i.e. the
 	 *			buf_prepare op was called. It is cleared again
 	 *			after the 'buf_finish' op is called.
+	 * copied_timestamp:	the timestamp of this capture buffer was copied
+	 *			from an output buffer.
 	 * queued_entry:	entry on the queued buffers list, which holds
 	 *			all buffers queued from userspace
 	 * done_entry:		entry on the list that stores all buffers ready
@@ -271,6 +273,7 @@  struct vb2_buffer {
 	enum vb2_buffer_state	state;
 	unsigned int		synced:1;
 	unsigned int		prepared:1;
+	unsigned int		copied_timestamp:1;
 
 	struct vb2_plane	planes[VB2_MAX_PLANES];
 	struct list_head	queued_entry;