linux-media.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] [media] coda: implement encoder stop command
@ 2017-03-02  9:51 Philipp Zabel
  2017-03-02 10:02 ` Jean-Michel Hautbois
  2017-03-02 16:30 ` Jean-Michel Hautbois
  0 siblings, 2 replies; 5+ messages in thread
From: Philipp Zabel @ 2017-03-02  9:51 UTC (permalink / raw)
  To: linux-media; +Cc: kernel, Philipp Zabel

There is no need to call v4l2_m2m_try_schedule to kick off draining the
bitstream buffer for the encoder, but we have to wake up the destination
queue in case there are no new OUTPUT buffers to be encoded and userspace
is already polling for new CAPTURE buffers.

Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
---
 drivers/media/platform/coda/coda-common.c | 47 +++++++++++++++++++++++++++++++
 1 file changed, 47 insertions(+)

diff --git a/drivers/media/platform/coda/coda-common.c b/drivers/media/platform/coda/coda-common.c
index e1a2e8c70db01..085bbdb0d361b 100644
--- a/drivers/media/platform/coda/coda-common.c
+++ b/drivers/media/platform/coda/coda-common.c
@@ -881,6 +881,47 @@ static int coda_g_selection(struct file *file, void *fh,
 	return 0;
 }
 
+static int coda_try_encoder_cmd(struct file *file, void *fh,
+				struct v4l2_encoder_cmd *ec)
+{
+	if (ec->cmd != V4L2_ENC_CMD_STOP)
+		return -EINVAL;
+
+	if (ec->flags & V4L2_ENC_CMD_STOP_AT_GOP_END)
+		return -EINVAL;
+
+	return 0;
+}
+
+static int coda_encoder_cmd(struct file *file, void *fh,
+			    struct v4l2_encoder_cmd *ec)
+{
+	struct coda_ctx *ctx = fh_to_ctx(fh);
+	struct vb2_queue *dst_vq;
+	int ret;
+
+	ret = coda_try_encoder_cmd(file, fh, ec);
+	if (ret < 0)
+		return ret;
+
+	/* Ignore encoder stop command silently in decoder context */
+	if (ctx->inst_type != CODA_INST_ENCODER)
+		return 0;
+
+	/* Set the stream-end flag on this context */
+	ctx->bit_stream_param |= CODA_BIT_STREAM_END_FLAG;
+
+	/* If there is no buffer in flight, wake up */
+	if (ctx->qsequence == ctx->osequence) {
+		dst_vq = v4l2_m2m_get_vq(ctx->fh.m2m_ctx,
+					 V4L2_BUF_TYPE_VIDEO_CAPTURE);
+		dst_vq->last_buffer_dequeued = true;
+		wake_up(&dst_vq->done_wq);
+	}
+
+	return 0;
+}
+
 static int coda_try_decoder_cmd(struct file *file, void *fh,
 				struct v4l2_decoder_cmd *dc)
 {
@@ -1054,6 +1095,8 @@ static const struct v4l2_ioctl_ops coda_ioctl_ops = {
 
 	.vidioc_g_selection	= coda_g_selection,
 
+	.vidioc_try_encoder_cmd	= coda_try_encoder_cmd,
+	.vidioc_encoder_cmd	= coda_encoder_cmd,
 	.vidioc_try_decoder_cmd	= coda_try_decoder_cmd,
 	.vidioc_decoder_cmd	= coda_decoder_cmd,
 
@@ -1330,9 +1373,13 @@ static void coda_buf_queue(struct vb2_buffer *vb)
 		mutex_lock(&ctx->bitstream_mutex);
 		v4l2_m2m_buf_queue(ctx->fh.m2m_ctx, vbuf);
 		if (vb2_is_streaming(vb->vb2_queue))
+			/* This set buf->sequence = ctx->qsequence++ */
 			coda_fill_bitstream(ctx, true);
 		mutex_unlock(&ctx->bitstream_mutex);
 	} else {
+		if (ctx->inst_type == CODA_INST_ENCODER &&
+		    vq->type == V4L2_BUF_TYPE_VIDEO_OUTPUT)
+			vbuf->sequence = ctx->qsequence++;
 		v4l2_m2m_buf_queue(ctx->fh.m2m_ctx, vbuf);
 	}
 }
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] [media] coda: implement encoder stop command
  2017-03-02  9:51 [PATCH] [media] coda: implement encoder stop command Philipp Zabel
@ 2017-03-02 10:02 ` Jean-Michel Hautbois
  2017-03-02 10:16   ` Philipp Zabel
  2017-03-02 16:30 ` Jean-Michel Hautbois
  1 sibling, 1 reply; 5+ messages in thread
From: Jean-Michel Hautbois @ 2017-03-02 10:02 UTC (permalink / raw)
  To: Philipp Zabel; +Cc: Linux Media Mailing List, Sascha Hauer

Hi Philipp,

2017-03-02 10:51 GMT+01:00 Philipp Zabel <p.zabel@pengutronix.de>:
> There is no need to call v4l2_m2m_try_schedule to kick off draining the
> bitstream buffer for the encoder, but we have to wake up the destination
> queue in case there are no new OUTPUT buffers to be encoded and userspace
> is already polling for new CAPTURE buffers.
>
> Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
> ---
>  drivers/media/platform/coda/coda-common.c | 47 +++++++++++++++++++++++++++++++
>  1 file changed, 47 insertions(+)
>
> diff --git a/drivers/media/platform/coda/coda-common.c b/drivers/media/platform/coda/coda-common.c
> index e1a2e8c70db01..085bbdb0d361b 100644
> --- a/drivers/media/platform/coda/coda-common.c
> +++ b/drivers/media/platform/coda/coda-common.c
> @@ -881,6 +881,47 @@ static int coda_g_selection(struct file *file, void *fh,
>         return 0;
>  }
>
> +static int coda_try_encoder_cmd(struct file *file, void *fh,
> +                               struct v4l2_encoder_cmd *ec)
> +{
> +       if (ec->cmd != V4L2_ENC_CMD_STOP)
> +               return -EINVAL;
> +
> +       if (ec->flags & V4L2_ENC_CMD_STOP_AT_GOP_END)
> +               return -EINVAL;
> +
> +       return 0;
> +}
> +
> +static int coda_encoder_cmd(struct file *file, void *fh,
> +                           struct v4l2_encoder_cmd *ec)
> +{
> +       struct coda_ctx *ctx = fh_to_ctx(fh);
> +       struct vb2_queue *dst_vq;
> +       int ret;
> +
> +       ret = coda_try_encoder_cmd(file, fh, ec);
> +       if (ret < 0)
> +               return ret;
> +
> +       /* Ignore encoder stop command silently in decoder context */
> +       if (ctx->inst_type != CODA_INST_ENCODER)
> +               return 0;
> +
> +       /* Set the stream-end flag on this context */
> +       ctx->bit_stream_param |= CODA_BIT_STREAM_END_FLAG;

Why aren't you calling coda_bit_stream_end_flag() ?

> +       /* If there is no buffer in flight, wake up */
> +       if (ctx->qsequence == ctx->osequence) {
> +               dst_vq = v4l2_m2m_get_vq(ctx->fh.m2m_ctx,
> +                                        V4L2_BUF_TYPE_VIDEO_CAPTURE);
> +               dst_vq->last_buffer_dequeued = true;
> +               wake_up(&dst_vq->done_wq);
> +       }
> +
> +       return 0;
> +}
> +
>  static int coda_try_decoder_cmd(struct file *file, void *fh,
>                                 struct v4l2_decoder_cmd *dc)
>  {
> @@ -1054,6 +1095,8 @@ static const struct v4l2_ioctl_ops coda_ioctl_ops = {
>
>         .vidioc_g_selection     = coda_g_selection,
>
> +       .vidioc_try_encoder_cmd = coda_try_encoder_cmd,
> +       .vidioc_encoder_cmd     = coda_encoder_cmd,
>         .vidioc_try_decoder_cmd = coda_try_decoder_cmd,
>         .vidioc_decoder_cmd     = coda_decoder_cmd,
>
> @@ -1330,9 +1373,13 @@ static void coda_buf_queue(struct vb2_buffer *vb)
>                 mutex_lock(&ctx->bitstream_mutex);
>                 v4l2_m2m_buf_queue(ctx->fh.m2m_ctx, vbuf);
>                 if (vb2_is_streaming(vb->vb2_queue))
> +                       /* This set buf->sequence = ctx->qsequence++ */
>                         coda_fill_bitstream(ctx, true);
>                 mutex_unlock(&ctx->bitstream_mutex);
>         } else {
> +               if (ctx->inst_type == CODA_INST_ENCODER &&
> +                   vq->type == V4L2_BUF_TYPE_VIDEO_OUTPUT)
> +                       vbuf->sequence = ctx->qsequence++;
>                 v4l2_m2m_buf_queue(ctx->fh.m2m_ctx, vbuf);
>         }
>  }
> --
> 2.11.0
>

JM

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] [media] coda: implement encoder stop command
  2017-03-02 10:02 ` Jean-Michel Hautbois
@ 2017-03-02 10:16   ` Philipp Zabel
  0 siblings, 0 replies; 5+ messages in thread
From: Philipp Zabel @ 2017-03-02 10:16 UTC (permalink / raw)
  To: Jean-Michel Hautbois; +Cc: Linux Media Mailing List, Sascha Hauer

Hi Jean-Michel,

On Thu, 2017-03-02 at 11:02 +0100, Jean-Michel Hautbois wrote:
> Hi Philipp,
> 
> 2017-03-02 10:51 GMT+01:00 Philipp Zabel <p.zabel@pengutronix.de>:
> > There is no need to call v4l2_m2m_try_schedule to kick off draining the
> > bitstream buffer for the encoder, but we have to wake up the destination
> > queue in case there are no new OUTPUT buffers to be encoded and userspace
> > is already polling for new CAPTURE buffers.
> >
> > Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
> > ---
> >  drivers/media/platform/coda/coda-common.c | 47 +++++++++++++++++++++++++++++++
> >  1 file changed, 47 insertions(+)
> >
> > diff --git a/drivers/media/platform/coda/coda-common.c b/drivers/media/platform/coda/coda-common.c
> > index e1a2e8c70db01..085bbdb0d361b 100644
> > --- a/drivers/media/platform/coda/coda-common.c
> > +++ b/drivers/media/platform/coda/coda-common.c
> > @@ -881,6 +881,47 @@ static int coda_g_selection(struct file *file, void *fh,
> >         return 0;
> >  }
> >
> > +static int coda_try_encoder_cmd(struct file *file, void *fh,
> > +                               struct v4l2_encoder_cmd *ec)
> > +{
> > +       if (ec->cmd != V4L2_ENC_CMD_STOP)
> > +               return -EINVAL;
> > +
> > +       if (ec->flags & V4L2_ENC_CMD_STOP_AT_GOP_END)
> > +               return -EINVAL;
> > +
> > +       return 0;
> > +}
> > +
> > +static int coda_encoder_cmd(struct file *file, void *fh,
> > +                           struct v4l2_encoder_cmd *ec)
> > +{
> > +       struct coda_ctx *ctx = fh_to_ctx(fh);
> > +       struct vb2_queue *dst_vq;
> > +       int ret;
> > +
> > +       ret = coda_try_encoder_cmd(file, fh, ec);
> > +       if (ret < 0)
> > +               return ret;
> > +
> > +       /* Ignore encoder stop command silently in decoder context */
> > +       if (ctx->inst_type != CODA_INST_ENCODER)
> > +               return 0;
> > +
> > +       /* Set the stream-end flag on this context */
> > +       ctx->bit_stream_param |= CODA_BIT_STREAM_END_FLAG;
> 
> Why aren't you calling coda_bit_stream_end_flag() ?

Because that additionally does:

        /* If this context is currently running, update the hardware flag */
        if ((dev->devtype->product == CODA_960) &&
            coda_isbusy(dev) &&
            (ctx->idx == coda_read(dev, CODA_REG_BIT_RUN_INDEX))) {
                coda_write(dev, ctx->bit_stream_param,
                           CODA_REG_BIT_BIT_STREAM_PARAM);
        }

to kick a potentially hanging decode picture run. This is unnecessary in the
encoder case.
We only need the flag set to make coda_buf_is_end_of_stream return true
and thereby make coda_m2m_buf_done set the V4L2_BUF_FLAG_LAST on the
buffer and emit the EOS event.

regards
Philipp

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] [media] coda: implement encoder stop command
  2017-03-02  9:51 [PATCH] [media] coda: implement encoder stop command Philipp Zabel
  2017-03-02 10:02 ` Jean-Michel Hautbois
@ 2017-03-02 16:30 ` Jean-Michel Hautbois
  2017-03-03 11:50   ` Philipp Zabel
  1 sibling, 1 reply; 5+ messages in thread
From: Jean-Michel Hautbois @ 2017-03-02 16:30 UTC (permalink / raw)
  To: Philipp Zabel; +Cc: Linux Media Mailing List, Sascha Hauer

<snip>

> +       /* If there is no buffer in flight, wake up */
> +       if (ctx->qsequence == ctx->osequence) {

Not sure about this one, I would have done something like :
if (!(ctx->fh.m2m_ctx->job_flags)) {

> +               dst_vq = v4l2_m2m_get_vq(ctx->fh.m2m_ctx,
> +                                        V4L2_BUF_TYPE_VIDEO_CAPTURE);
> +               dst_vq->last_buffer_dequeued = true;
> +               wake_up(&dst_vq->done_wq);
> +       }
> +
> +       return 0;
> +}
> +
>  static int coda_try_decoder_cmd(struct file *file, void *fh,
>                                 struct v4l2_decoder_cmd *dc)
>  {
> @@ -1054,6 +1095,8 @@ static const struct v4l2_ioctl_ops coda_ioctl_ops = {
>
>         .vidioc_g_selection     = coda_g_selection,
>
> +       .vidioc_try_encoder_cmd = coda_try_encoder_cmd,
> +       .vidioc_encoder_cmd     = coda_encoder_cmd,
>         .vidioc_try_decoder_cmd = coda_try_decoder_cmd,
>         .vidioc_decoder_cmd     = coda_decoder_cmd,
>
> @@ -1330,9 +1373,13 @@ static void coda_buf_queue(struct vb2_buffer *vb)
>                 mutex_lock(&ctx->bitstream_mutex);
>                 v4l2_m2m_buf_queue(ctx->fh.m2m_ctx, vbuf);
>                 if (vb2_is_streaming(vb->vb2_queue))
> +                       /* This set buf->sequence = ctx->qsequence++ */
>                         coda_fill_bitstream(ctx, true);
>                 mutex_unlock(&ctx->bitstream_mutex);
>         } else {
> +               if (ctx->inst_type == CODA_INST_ENCODER &&
> +                   vq->type == V4L2_BUF_TYPE_VIDEO_OUTPUT)
> +                       vbuf->sequence = ctx->qsequence++;
>                 v4l2_m2m_buf_queue(ctx->fh.m2m_ctx, vbuf);
>         }
>  }
> --
> 2.11.0
>

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] [media] coda: implement encoder stop command
  2017-03-02 16:30 ` Jean-Michel Hautbois
@ 2017-03-03 11:50   ` Philipp Zabel
  0 siblings, 0 replies; 5+ messages in thread
From: Philipp Zabel @ 2017-03-03 11:50 UTC (permalink / raw)
  To: Jean-Michel Hautbois; +Cc: Linux Media Mailing List, Sascha Hauer

On Thu, 2017-03-02 at 17:30 +0100, Jean-Michel Hautbois wrote:
> <snip>
> 
> > +       /* If there is no buffer in flight, wake up */
> > +       if (ctx->qsequence == ctx->osequence) {
> 
> Not sure about this one, I would have done something like :
> if (!(ctx->fh.m2m_ctx->job_flags)) {

This field is documented as "used internally", though.

regards
Philipp

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2017-03-03 14:11 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-03-02  9:51 [PATCH] [media] coda: implement encoder stop command Philipp Zabel
2017-03-02 10:02 ` Jean-Michel Hautbois
2017-03-02 10:16   ` Philipp Zabel
2017-03-02 16:30 ` Jean-Michel Hautbois
2017-03-03 11:50   ` Philipp Zabel

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).