diff mbox

[v4,2/2] media: staging/imx: fill vb2_v4l2_buffer sequence entry

Message ID 20180315191323.6028-2-ps.report@gmx.net (mailing list archive)
State Accepted, archived
Delegated to: Hans Verkuil
Headers

Commit Message

Peter Seiderer March 15, 2018, 7:13 p.m. UTC 
- enables gstreamer v4l2src lost frame detection, e.g:

  0:00:08.685185668  348  0x54f520 WARN  v4l2src gstv4l2src.c:970:gst_v4l2src_create:<v4l2src0> lost frames detected: count = 141 - ts: 0:00:08.330177332

- fixes v4l2-compliance test failure:

  Streaming ioctls:
          test read/write: OK (Not Supported)
              Video Capture:
                  Buffer: 0 Sequence: 0 Field: None Timestamp: 92.991450s
                  Buffer: 1 Sequence: 0 Field: None Timestamp: 93.008135s
                  fail: v4l2-test-buffers.cpp(294): (int)g_sequence() < seq.last_seq + 1
                  fail: v4l2-test-buffers.cpp(707): buf.check(q, last_seq)

Signed-off-by: Peter Seiderer <ps.report@gmx.net>
---
Changes in v2:
  - fill vb2_v4l2_buffer sequence entry in imx-ic-prpencvf too
    (suggested by Steve Longerbeam)

Changes in v3:
  - add changelog (suggested by Greg Kroah-Hartman, Fabio Estevam
    and Dan Carpenter) and patch history
  - use u32 (instead of __u32) (suggested by Dan Carpenter)
  - let sequence counter start with zero, keeping v4l2-compliance
    testing happy (needs additional setting of field to a valid
    value, patch will follow soon)

Changes in v4:
  - add v4l2-compliance test failure to changelog
  - reorder frame_sequence increment and assignement to
    avoid -1 as start value (suggeted by Steve Longerbeam)
---
 drivers/staging/media/imx/imx-ic-prpencvf.c | 4 ++++
 drivers/staging/media/imx/imx-media-csi.c   | 4 ++++
 2 files changed, 8 insertions(+)

Comments

Dan Carpenter March 16, 2018, 8:12 a.m. UTC | #1 
MUCH BETTER!111!Exclamationmark!  Thanks!  :)

regards,
dan carpenter
Steve Longerbeam March 16, 2018, 5:05 p.m. UTC | #2 
Reviewed-by: Steve Longerbeam <steve_longerbeam@mentor.com>


On 03/15/2018 12:13 PM, Peter Seiderer wrote:
> - enables gstreamer v4l2src lost frame detection, e.g:
>
>    0:00:08.685185668  348  0x54f520 WARN  v4l2src gstv4l2src.c:970:gst_v4l2src_create:<v4l2src0> lost frames detected: count = 141 - ts: 0:00:08.330177332
>
> - fixes v4l2-compliance test failure:
>
>    Streaming ioctls:
>            test read/write: OK (Not Supported)
>                Video Capture:
>                    Buffer: 0 Sequence: 0 Field: None Timestamp: 92.991450s
>                    Buffer: 1 Sequence: 0 Field: None Timestamp: 93.008135s
>                    fail: v4l2-test-buffers.cpp(294): (int)g_sequence() < seq.last_seq + 1
>                    fail: v4l2-test-buffers.cpp(707): buf.check(q, last_seq)
>
> Signed-off-by: Peter Seiderer <ps.report@gmx.net>
> ---
> Changes in v2:
>    - fill vb2_v4l2_buffer sequence entry in imx-ic-prpencvf too
>      (suggested by Steve Longerbeam)
>
> Changes in v3:
>    - add changelog (suggested by Greg Kroah-Hartman, Fabio Estevam
>      and Dan Carpenter) and patch history
>    - use u32 (instead of __u32) (suggested by Dan Carpenter)
>    - let sequence counter start with zero, keeping v4l2-compliance
>      testing happy (needs additional setting of field to a valid
>      value, patch will follow soon)
>
> Changes in v4:
>    - add v4l2-compliance test failure to changelog
>    - reorder frame_sequence increment and assignement to
>      avoid -1 as start value (suggeted by Steve Longerbeam)
> ---
>   drivers/staging/media/imx/imx-ic-prpencvf.c | 4 ++++
>   drivers/staging/media/imx/imx-media-csi.c   | 4 ++++
>   2 files changed, 8 insertions(+)
>
> diff --git a/drivers/staging/media/imx/imx-ic-prpencvf.c b/drivers/staging/media/imx/imx-ic-prpencvf.c
> index ffeb017c73b2..28f41caba05d 100644
> --- a/drivers/staging/media/imx/imx-ic-prpencvf.c
> +++ b/drivers/staging/media/imx/imx-ic-prpencvf.c
> @@ -103,6 +103,7 @@ struct prp_priv {
>   	int nfb4eof_irq;
>   
>   	int stream_count;
> +	u32 frame_sequence; /* frame sequence counter */
>   	bool last_eof;  /* waiting for last EOF at stream off */
>   	bool nfb4eof;    /* NFB4EOF encountered during streaming */
>   	struct completion last_eof_comp;
> @@ -211,12 +212,14 @@ static void prp_vb2_buf_done(struct prp_priv *priv, struct ipuv3_channel *ch)
>   	done = priv->active_vb2_buf[priv->ipu_buf_num];
>   	if (done) {
>   		done->vbuf.field = vdev->fmt.fmt.pix.field;
> +		done->vbuf.sequence = priv->frame_sequence;
>   		vb = &done->vbuf.vb2_buf;
>   		vb->timestamp = ktime_get_ns();
>   		vb2_buffer_done(vb, priv->nfb4eof ?
>   				VB2_BUF_STATE_ERROR : VB2_BUF_STATE_DONE);
>   	}
>   
> +	priv->frame_sequence++;
>   	priv->nfb4eof = false;
>   
>   	/* get next queued buffer */
> @@ -638,6 +641,7 @@ static int prp_start(struct prp_priv *priv)
>   
>   	/* init EOF completion waitq */
>   	init_completion(&priv->last_eof_comp);
> +	priv->frame_sequence = 0;
>   	priv->last_eof = false;
>   	priv->nfb4eof = false;
>   
> diff --git a/drivers/staging/media/imx/imx-media-csi.c b/drivers/staging/media/imx/imx-media-csi.c
> index 5f69117b5811..3f2ce05848f3 100644
> --- a/drivers/staging/media/imx/imx-media-csi.c
> +++ b/drivers/staging/media/imx/imx-media-csi.c
> @@ -111,6 +111,7 @@ struct csi_priv {
>   	struct v4l2_ctrl_handler ctrl_hdlr;
>   
>   	int stream_count; /* streaming counter */
> +	u32 frame_sequence; /* frame sequence counter */
>   	bool last_eof;   /* waiting for last EOF at stream off */
>   	bool nfb4eof;    /* NFB4EOF encountered during streaming */
>   	struct completion last_eof_comp;
> @@ -237,12 +238,14 @@ static void csi_vb2_buf_done(struct csi_priv *priv)
>   	done = priv->active_vb2_buf[priv->ipu_buf_num];
>   	if (done) {
>   		done->vbuf.field = vdev->fmt.fmt.pix.field;
> +		done->vbuf.sequence = priv->frame_sequence;
>   		vb = &done->vbuf.vb2_buf;
>   		vb->timestamp = ktime_get_ns();
>   		vb2_buffer_done(vb, priv->nfb4eof ?
>   				VB2_BUF_STATE_ERROR : VB2_BUF_STATE_DONE);
>   	}
>   
> +	priv->frame_sequence++;
>   	priv->nfb4eof = false;
>   
>   	/* get next queued buffer */
> @@ -544,6 +547,7 @@ static int csi_idmac_start(struct csi_priv *priv)
>   
>   	/* init EOF completion waitq */
>   	init_completion(&priv->last_eof_comp);
> +	priv->frame_sequence = 0;
>   	priv->last_eof = false;
>   	priv->nfb4eof = false;
>
Peter Seiderer June 11, 2018, 8:48 p.m. UTC | #3 
Hello *,

On Fri, 16 Mar 2018 10:05:44 -0700, Steve Longerbeam <slongerbeam@gmail.com> wrote:

> Reviewed-by: Steve Longerbeam <steve_longerbeam@mentor.com>

Ping? Anybody taking this one?

Regards,
Peter

> 
> 
> On 03/15/2018 12:13 PM, Peter Seiderer wrote:
> > - enables gstreamer v4l2src lost frame detection, e.g:
> >
> >    0:00:08.685185668  348  0x54f520 WARN  v4l2src gstv4l2src.c:970:gst_v4l2src_create:<v4l2src0> lost frames detected: count = 141 - ts: 0:00:08.330177332
> >
> > - fixes v4l2-compliance test failure:
> >
> >    Streaming ioctls:
> >            test read/write: OK (Not Supported)
> >                Video Capture:
> >                    Buffer: 0 Sequence: 0 Field: None Timestamp: 92.991450s
> >                    Buffer: 1 Sequence: 0 Field: None Timestamp: 93.008135s
> >                    fail: v4l2-test-buffers.cpp(294): (int)g_sequence() < seq.last_seq + 1
> >                    fail: v4l2-test-buffers.cpp(707): buf.check(q, last_seq)
> >
> > Signed-off-by: Peter Seiderer <ps.report@gmx.net>
> > ---
> > Changes in v2:
> >    - fill vb2_v4l2_buffer sequence entry in imx-ic-prpencvf too
> >      (suggested by Steve Longerbeam)
> >
> > Changes in v3:
> >    - add changelog (suggested by Greg Kroah-Hartman, Fabio Estevam
> >      and Dan Carpenter) and patch history
> >    - use u32 (instead of __u32) (suggested by Dan Carpenter)
> >    - let sequence counter start with zero, keeping v4l2-compliance
> >      testing happy (needs additional setting of field to a valid
> >      value, patch will follow soon)
> >
> > Changes in v4:
> >    - add v4l2-compliance test failure to changelog
> >    - reorder frame_sequence increment and assignement to
> >      avoid -1 as start value (suggeted by Steve Longerbeam)
> > ---
> >   drivers/staging/media/imx/imx-ic-prpencvf.c | 4 ++++
> >   drivers/staging/media/imx/imx-media-csi.c   | 4 ++++
> >   2 files changed, 8 insertions(+)
> >
> > diff --git a/drivers/staging/media/imx/imx-ic-prpencvf.c b/drivers/staging/media/imx/imx-ic-prpencvf.c
> > index ffeb017c73b2..28f41caba05d 100644
> > --- a/drivers/staging/media/imx/imx-ic-prpencvf.c
> > +++ b/drivers/staging/media/imx/imx-ic-prpencvf.c
> > @@ -103,6 +103,7 @@ struct prp_priv {
> >   	int nfb4eof_irq;
> >   
> >   	int stream_count;
> > +	u32 frame_sequence; /* frame sequence counter */
> >   	bool last_eof;  /* waiting for last EOF at stream off */
> >   	bool nfb4eof;    /* NFB4EOF encountered during streaming */
> >   	struct completion last_eof_comp;
> > @@ -211,12 +212,14 @@ static void prp_vb2_buf_done(struct prp_priv *priv, struct ipuv3_channel *ch)
> >   	done = priv->active_vb2_buf[priv->ipu_buf_num];
> >   	if (done) {
> >   		done->vbuf.field = vdev->fmt.fmt.pix.field;
> > +		done->vbuf.sequence = priv->frame_sequence;
> >   		vb = &done->vbuf.vb2_buf;
> >   		vb->timestamp = ktime_get_ns();
> >   		vb2_buffer_done(vb, priv->nfb4eof ?
> >   				VB2_BUF_STATE_ERROR : VB2_BUF_STATE_DONE);
> >   	}
> >   
> > +	priv->frame_sequence++;
> >   	priv->nfb4eof = false;
> >   
> >   	/* get next queued buffer */
> > @@ -638,6 +641,7 @@ static int prp_start(struct prp_priv *priv)
> >   
> >   	/* init EOF completion waitq */
> >   	init_completion(&priv->last_eof_comp);
> > +	priv->frame_sequence = 0;
> >   	priv->last_eof = false;
> >   	priv->nfb4eof = false;
> >   
> > diff --git a/drivers/staging/media/imx/imx-media-csi.c b/drivers/staging/media/imx/imx-media-csi.c
> > index 5f69117b5811..3f2ce05848f3 100644
> > --- a/drivers/staging/media/imx/imx-media-csi.c
> > +++ b/drivers/staging/media/imx/imx-media-csi.c
> > @@ -111,6 +111,7 @@ struct csi_priv {
> >   	struct v4l2_ctrl_handler ctrl_hdlr;
> >   
> >   	int stream_count; /* streaming counter */
> > +	u32 frame_sequence; /* frame sequence counter */
> >   	bool last_eof;   /* waiting for last EOF at stream off */
> >   	bool nfb4eof;    /* NFB4EOF encountered during streaming */
> >   	struct completion last_eof_comp;
> > @@ -237,12 +238,14 @@ static void csi_vb2_buf_done(struct csi_priv *priv)
> >   	done = priv->active_vb2_buf[priv->ipu_buf_num];
> >   	if (done) {
> >   		done->vbuf.field = vdev->fmt.fmt.pix.field;
> > +		done->vbuf.sequence = priv->frame_sequence;
> >   		vb = &done->vbuf.vb2_buf;
> >   		vb->timestamp = ktime_get_ns();
> >   		vb2_buffer_done(vb, priv->nfb4eof ?
> >   				VB2_BUF_STATE_ERROR : VB2_BUF_STATE_DONE);
> >   	}
> >   
> > +	priv->frame_sequence++;
> >   	priv->nfb4eof = false;
> >   
> >   	/* get next queued buffer */
> > @@ -544,6 +547,7 @@ static int csi_idmac_start(struct csi_priv *priv)
> >   
> >   	/* init EOF completion waitq */
> >   	init_completion(&priv->last_eof_comp);
> > +	priv->frame_sequence = 0;
> >   	priv->last_eof = false;
> >   	priv->nfb4eof = false;
> >     
>
Philipp Zabel June 12, 2018, 8:41 a.m. UTC | #4 
Hi,

On Mon, 2018-06-11 at 22:48 +0200, Peter Seiderer wrote:
> Hello *,
> 
> On Fri, 16 Mar 2018 10:05:44 -0700, Steve Longerbeam <slongerbeam@gmail.com> wrote:
> 
> > Reviewed-by: Steve Longerbeam <steve_longerbeam@mentor.com>
> 
> Ping? Anybody taking this one?

The patches have been marked not applicable in patchwork [1][2].
They do apply on media_tree master.

[1] https://patchwork.linuxtv.org/patch/47946/
[2] https://patchwork.linuxtv.org/patch/47947/

regards
Philipp
Hans Verkuil June 12, 2018, 8:59 a.m. UTC | #5 
On 12/06/18 10:41, Philipp Zabel wrote:
> Hi,
> 
> On Mon, 2018-06-11 at 22:48 +0200, Peter Seiderer wrote:
>> Hello *,
>>
>> On Fri, 16 Mar 2018 10:05:44 -0700, Steve Longerbeam <slongerbeam@gmail.com> wrote:
>>
>>> Reviewed-by: Steve Longerbeam <steve_longerbeam@mentor.com>
>>
>> Ping? Anybody taking this one?
> 
> The patches have been marked not applicable in patchwork [1][2].
> They do apply on media_tree master.
> 
> [1] https://patchwork.linuxtv.org/patch/47946/
> [2] https://patchwork.linuxtv.org/patch/47947/
> 
> regards
> Philipp
> 

I changed the state to NEW and delegated it to me, so the next time I'm processing
patches I'll handled them. I suspect the state was changed by mistake by someone
(quite possibly me!).

Regards,

	Hans
diff mbox

Patch

diff --git a/drivers/staging/media/imx/imx-ic-prpencvf.c b/drivers/staging/media/imx/imx-ic-prpencvf.c
index ffeb017c73b2..28f41caba05d 100644
--- a/drivers/staging/media/imx/imx-ic-prpencvf.c
+++ b/drivers/staging/media/imx/imx-ic-prpencvf.c
@@ -103,6 +103,7 @@  struct prp_priv {
 	int nfb4eof_irq;
 
 	int stream_count;
+	u32 frame_sequence; /* frame sequence counter */
 	bool last_eof;  /* waiting for last EOF at stream off */
 	bool nfb4eof;    /* NFB4EOF encountered during streaming */
 	struct completion last_eof_comp;
@@ -211,12 +212,14 @@  static void prp_vb2_buf_done(struct prp_priv *priv, struct ipuv3_channel *ch)
 	done = priv->active_vb2_buf[priv->ipu_buf_num];
 	if (done) {
 		done->vbuf.field = vdev->fmt.fmt.pix.field;
+		done->vbuf.sequence = priv->frame_sequence;
 		vb = &done->vbuf.vb2_buf;
 		vb->timestamp = ktime_get_ns();
 		vb2_buffer_done(vb, priv->nfb4eof ?
 				VB2_BUF_STATE_ERROR : VB2_BUF_STATE_DONE);
 	}
 
+	priv->frame_sequence++;
 	priv->nfb4eof = false;
 
 	/* get next queued buffer */
@@ -638,6 +641,7 @@  static int prp_start(struct prp_priv *priv)
 
 	/* init EOF completion waitq */
 	init_completion(&priv->last_eof_comp);
+	priv->frame_sequence = 0;
 	priv->last_eof = false;
 	priv->nfb4eof = false;
 
diff --git a/drivers/staging/media/imx/imx-media-csi.c b/drivers/staging/media/imx/imx-media-csi.c
index 5f69117b5811..3f2ce05848f3 100644
--- a/drivers/staging/media/imx/imx-media-csi.c
+++ b/drivers/staging/media/imx/imx-media-csi.c
@@ -111,6 +111,7 @@  struct csi_priv {
 	struct v4l2_ctrl_handler ctrl_hdlr;
 
 	int stream_count; /* streaming counter */
+	u32 frame_sequence; /* frame sequence counter */
 	bool last_eof;   /* waiting for last EOF at stream off */
 	bool nfb4eof;    /* NFB4EOF encountered during streaming */
 	struct completion last_eof_comp;
@@ -237,12 +238,14 @@  static void csi_vb2_buf_done(struct csi_priv *priv)
 	done = priv->active_vb2_buf[priv->ipu_buf_num];
 	if (done) {
 		done->vbuf.field = vdev->fmt.fmt.pix.field;
+		done->vbuf.sequence = priv->frame_sequence;
 		vb = &done->vbuf.vb2_buf;
 		vb->timestamp = ktime_get_ns();
 		vb2_buffer_done(vb, priv->nfb4eof ?
 				VB2_BUF_STATE_ERROR : VB2_BUF_STATE_DONE);
 	}
 
+	priv->frame_sequence++;
 	priv->nfb4eof = false;
 
 	/* get next queued buffer */
@@ -544,6 +547,7 @@  static int csi_idmac_start(struct csi_priv *priv)
 
 	/* init EOF completion waitq */
 	init_completion(&priv->last_eof_comp);
+	priv->frame_sequence = 0;
 	priv->last_eof = false;
 	priv->nfb4eof = false;