Re: [PATCH] [media] coda: implement encoder stop command
On Thu, 2017-03-02 at 17:30 +0100, Jean-Michel Hautbois wrote:
>
>
> > + /* 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
Re: [PATCH] [media] coda: implement encoder stop command
> + /* 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
>
Re: [PATCH] [media] coda: implement encoder stop command
Hi Philipp,
2017-03-02 10:51 GMT+01:00 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
> ---
> 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
Re: [PATCH] [media] coda: implement encoder stop command
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 :
> > 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
> > ---
> > 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
