linux-media.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/9] vicodec: fixes and sync with v4 of codec spec
@ 2019-06-06 12:11 Hans Verkuil
  2019-06-06 12:11 ` [PATCH 1/9] vicodec: move v4l2_ctrl_request_complete after spin_unlock Hans Verkuil
                   ` (8 more replies)
  0 siblings, 9 replies; 10+ messages in thread
From: Hans Verkuil @ 2019-06-06 12:11 UTC (permalink / raw)
  To: linux-media

This series fixes a number of bugs and compliance failures in the
vicodec driver. Combined with new/fixed tests in v4l2-compliance
(see https://git.linuxtv.org/hverkuil/v4l-utils.git/log/?h=vicodec)
vicodec can now be used in regression testing.

There will probably be a few more changes once the codec spec is
finalized, but I'm hopeful that those changes will be minor.

Having this merged will make it easier to work on improved codec
core support in v4l2-mem2mem.c.

Regards,

	Hans

Hans Verkuil (9):
  vicodec: move v4l2_ctrl_request_complete after spin_unlock
  vicodec: always return a valid format.
  vicodec: fix initial stateless sizeimage value
  vicodec: pass on enc output format to capture side
  vicodec: add V4L2_CID_MIN_BUFFERS_FOR_OUTPUT
  vicodec: set KEY/PFRAME flag when decoding
  vicodec: use correct sizeimage value when draining
  vicodec: stateless codecs do not have EOS and SOURCE_CHANGE events
  vicodec: improve handling of ENC_CMD_STOP/START

 drivers/media/platform/vicodec/vicodec-core.c | 257 ++++++++++++++----
 1 file changed, 199 insertions(+), 58 deletions(-)

-- 
2.20.1


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

* [PATCH 1/9] vicodec: move v4l2_ctrl_request_complete after spin_unlock
  2019-06-06 12:11 [PATCH 0/9] vicodec: fixes and sync with v4 of codec spec Hans Verkuil
@ 2019-06-06 12:11 ` Hans Verkuil
  2019-06-06 12:11 ` [PATCH 2/9] vicodec: always return a valid format Hans Verkuil
                   ` (7 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Hans Verkuil @ 2019-06-06 12:11 UTC (permalink / raw)
  To: linux-media; +Cc: Hans Verkuil

v4l2_ctrl_request_complete can sleep, so can't be called while
a spinlock is held.

Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
---
 drivers/media/platform/vicodec/vicodec-core.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/media/platform/vicodec/vicodec-core.c b/drivers/media/platform/vicodec/vicodec-core.c
index 72c56756e45b..358469f23191 100644
--- a/drivers/media/platform/vicodec/vicodec-core.c
+++ b/drivers/media/platform/vicodec/vicodec-core.c
@@ -442,14 +442,14 @@ static void device_run(void *priv)
 		ctx->comp_has_next_frame = false;
 	}
 	v4l2_m2m_buf_done(dst_buf, state);
-	if (ctx->is_stateless && src_req)
-		v4l2_ctrl_request_complete(src_req, &ctx->hdl);
 
 	ctx->comp_size = 0;
 	ctx->header_size = 0;
 	ctx->comp_magic_cnt = 0;
 	ctx->comp_has_frame = false;
 	spin_unlock(ctx->lock);
+	if (ctx->is_stateless && src_req)
+		v4l2_ctrl_request_complete(src_req, &ctx->hdl);
 
 	if (ctx->is_enc)
 		v4l2_m2m_job_finish(dev->stateful_enc.m2m_dev, ctx->fh.m2m_ctx);
-- 
2.20.1


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

* [PATCH 2/9] vicodec: always return a valid format.
  2019-06-06 12:11 [PATCH 0/9] vicodec: fixes and sync with v4 of codec spec Hans Verkuil
  2019-06-06 12:11 ` [PATCH 1/9] vicodec: move v4l2_ctrl_request_complete after spin_unlock Hans Verkuil
@ 2019-06-06 12:11 ` Hans Verkuil
  2019-06-06 12:11 ` [PATCH 3/9] vicodec: fix initial stateless sizeimage value Hans Verkuil
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Hans Verkuil @ 2019-06-06 12:11 UTC (permalink / raw)
  To: linux-media; +Cc: Hans Verkuil

Rather than returning width/height values of 0, just default to
a format. Formats in V4L2 are always supposed to be valid, there
is no concept of an invalid format.

Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
---
 drivers/media/platform/vicodec/vicodec-core.c | 26 ++++++++++---------
 1 file changed, 14 insertions(+), 12 deletions(-)

diff --git a/drivers/media/platform/vicodec/vicodec-core.c b/drivers/media/platform/vicodec/vicodec-core.c
index 358469f23191..b23d57f50c94 100644
--- a/drivers/media/platform/vicodec/vicodec-core.c
+++ b/drivers/media/platform/vicodec/vicodec-core.c
@@ -1742,11 +1742,13 @@ static const struct v4l2_ctrl_config vicodec_ctrl_stateless_state = {
  */
 static int vicodec_open(struct file *file)
 {
+	const struct v4l2_fwht_pixfmt_info *info = v4l2_fwht_get_pixfmt(0);
 	struct video_device *vfd = video_devdata(file);
 	struct vicodec_dev *dev = video_drvdata(file);
 	struct vicodec_ctx *ctx = NULL;
 	struct v4l2_ctrl_handler *hdl;
-	unsigned int size;
+	unsigned int raw_size;
+	unsigned int comp_size;
 	int rc = 0;
 
 	if (mutex_lock_interruptible(vfd->lock))
@@ -1785,7 +1787,7 @@ static int vicodec_open(struct file *file)
 	v4l2_ctrl_handler_setup(hdl);
 
 	if (ctx->is_enc)
-		ctx->q_data[V4L2_M2M_SRC].info = v4l2_fwht_get_pixfmt(0);
+		ctx->q_data[V4L2_M2M_SRC].info = info;
 	else if (ctx->is_stateless)
 		ctx->q_data[V4L2_M2M_SRC].info = &pixfmt_stateless_fwht;
 	else
@@ -1794,22 +1796,22 @@ static int vicodec_open(struct file *file)
 	ctx->q_data[V4L2_M2M_SRC].coded_height = 720;
 	ctx->q_data[V4L2_M2M_SRC].visible_width = 1280;
 	ctx->q_data[V4L2_M2M_SRC].visible_height = 720;
-	size = 1280 * 720 * ctx->q_data[V4L2_M2M_SRC].info->sizeimage_mult /
-		ctx->q_data[V4L2_M2M_SRC].info->sizeimage_div;
+	raw_size = 1280 * 720 * info->sizeimage_mult / info->sizeimage_div;
+	comp_size = 1280 * 720 * pixfmt_fwht.sizeimage_mult /
+				 pixfmt_fwht.sizeimage_div;
 	if (ctx->is_enc || ctx->is_stateless)
-		ctx->q_data[V4L2_M2M_SRC].sizeimage = size;
+		ctx->q_data[V4L2_M2M_SRC].sizeimage = raw_size;
 	else
 		ctx->q_data[V4L2_M2M_SRC].sizeimage =
-			size + sizeof(struct fwht_cframe_hdr);
+			comp_size + sizeof(struct fwht_cframe_hdr);
+	ctx->q_data[V4L2_M2M_DST] = ctx->q_data[V4L2_M2M_SRC];
 	if (ctx->is_enc) {
-		ctx->q_data[V4L2_M2M_DST] = ctx->q_data[V4L2_M2M_SRC];
 		ctx->q_data[V4L2_M2M_DST].info = &pixfmt_fwht;
-		ctx->q_data[V4L2_M2M_DST].sizeimage = 1280 * 720 *
-			ctx->q_data[V4L2_M2M_DST].info->sizeimage_mult /
-			ctx->q_data[V4L2_M2M_DST].info->sizeimage_div +
-			sizeof(struct fwht_cframe_hdr);
+		ctx->q_data[V4L2_M2M_DST].sizeimage =
+			comp_size + sizeof(struct fwht_cframe_hdr);
 	} else {
-		ctx->q_data[V4L2_M2M_DST].info = NULL;
+		ctx->q_data[V4L2_M2M_DST].info = info;
+		ctx->q_data[V4L2_M2M_DST].sizeimage = raw_size;
 	}
 
 	ctx->state.colorspace = V4L2_COLORSPACE_REC709;
-- 
2.20.1


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

* [PATCH 3/9] vicodec: fix initial stateless sizeimage value
  2019-06-06 12:11 [PATCH 0/9] vicodec: fixes and sync with v4 of codec spec Hans Verkuil
  2019-06-06 12:11 ` [PATCH 1/9] vicodec: move v4l2_ctrl_request_complete after spin_unlock Hans Verkuil
  2019-06-06 12:11 ` [PATCH 2/9] vicodec: always return a valid format Hans Verkuil
@ 2019-06-06 12:11 ` Hans Verkuil
  2019-06-06 12:11 ` [PATCH 4/9] vicodec: pass on enc output format to capture side Hans Verkuil
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Hans Verkuil @ 2019-06-06 12:11 UTC (permalink / raw)
  To: linux-media; +Cc: Hans Verkuil

The initial sizeimage value was wrong for the stateless decoder.

Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
---
 drivers/media/platform/vicodec/vicodec-core.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/media/platform/vicodec/vicodec-core.c b/drivers/media/platform/vicodec/vicodec-core.c
index b23d57f50c94..7a7082808a23 100644
--- a/drivers/media/platform/vicodec/vicodec-core.c
+++ b/drivers/media/platform/vicodec/vicodec-core.c
@@ -1799,8 +1799,10 @@ static int vicodec_open(struct file *file)
 	raw_size = 1280 * 720 * info->sizeimage_mult / info->sizeimage_div;
 	comp_size = 1280 * 720 * pixfmt_fwht.sizeimage_mult /
 				 pixfmt_fwht.sizeimage_div;
-	if (ctx->is_enc || ctx->is_stateless)
+	if (ctx->is_enc)
 		ctx->q_data[V4L2_M2M_SRC].sizeimage = raw_size;
+	else if (ctx->is_stateless)
+		ctx->q_data[V4L2_M2M_SRC].sizeimage = comp_size;
 	else
 		ctx->q_data[V4L2_M2M_SRC].sizeimage =
 			comp_size + sizeof(struct fwht_cframe_hdr);
-- 
2.20.1


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

* [PATCH 4/9] vicodec: pass on enc output format to capture side
  2019-06-06 12:11 [PATCH 0/9] vicodec: fixes and sync with v4 of codec spec Hans Verkuil
                   ` (2 preceding siblings ...)
  2019-06-06 12:11 ` [PATCH 3/9] vicodec: fix initial stateless sizeimage value Hans Verkuil
@ 2019-06-06 12:11 ` Hans Verkuil
  2019-06-06 12:11 ` [PATCH 5/9] vicodec: add V4L2_CID_MIN_BUFFERS_FOR_OUTPUT Hans Verkuil
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Hans Verkuil @ 2019-06-06 12:11 UTC (permalink / raw)
  To: linux-media; +Cc: Hans Verkuil

Setting the encoder output format to e.g. 1920x1080 will set the
crop rectangle to 1920x1088, the coded resolution to 1920x1088 and
the capture coded resolution and sizeimage to 1920x1088 as well.

Note that this might change, since the encoder spec is still in
flux with respect to how this should behave.

Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
---
 drivers/media/platform/vicodec/vicodec-core.c | 53 +++++++++++++++----
 1 file changed, 43 insertions(+), 10 deletions(-)

diff --git a/drivers/media/platform/vicodec/vicodec-core.c b/drivers/media/platform/vicodec/vicodec-core.c
index 7a7082808a23..7a78d044072d 100644
--- a/drivers/media/platform/vicodec/vicodec-core.c
+++ b/drivers/media/platform/vicodec/vicodec-core.c
@@ -1032,16 +1032,10 @@ static int vidioc_s_fmt(struct vicodec_ctx *ctx, struct v4l2_format *f)
 	default:
 		return -EINVAL;
 	}
-	if (q_data->visible_width > q_data->coded_width)
-		q_data->visible_width = q_data->coded_width;
-	if (q_data->visible_height > q_data->coded_height)
-		q_data->visible_height = q_data->coded_height;
-
 
 	dprintk(ctx->dev,
-		"Setting format for type %d, coded wxh: %dx%d, visible wxh: %dx%d, fourcc: %08x\n",
+		"Setting format for type %d, coded wxh: %dx%d, fourcc: 0x%08x\n",
 		f->type, q_data->coded_width, q_data->coded_height,
-		q_data->visible_width, q_data->visible_height,
 		q_data->info->id);
 
 	return 0;
@@ -1063,18 +1057,58 @@ static int vidioc_s_fmt_vid_out(struct file *file, void *priv,
 				struct v4l2_format *f)
 {
 	struct vicodec_ctx *ctx = file2ctx(file);
-	struct v4l2_pix_format_mplane *pix_mp;
+	struct vicodec_q_data *q_data;
+	struct vicodec_q_data *q_data_cap;
 	struct v4l2_pix_format *pix;
+	struct v4l2_pix_format_mplane *pix_mp;
+	u32 coded_w = 0, coded_h = 0;
+	unsigned int size = 0;
 	int ret;
 
+	q_data = get_q_data(ctx, f->type);
+	q_data_cap = get_q_data(ctx, V4L2_BUF_TYPE_VIDEO_CAPTURE);
+
 	ret = vidioc_try_fmt_vid_out(file, priv, f);
 	if (ret)
 		return ret;
 
+	if (ctx->is_enc) {
+		struct vb2_queue *vq = v4l2_m2m_get_vq(ctx->fh.m2m_ctx, f->type);
+		struct vb2_queue *vq_cap = v4l2_m2m_get_vq(ctx->fh.m2m_ctx,
+							   V4L2_BUF_TYPE_VIDEO_CAPTURE);
+		const struct v4l2_fwht_pixfmt_info *info = ctx->is_stateless ?
+			&pixfmt_stateless_fwht : &pixfmt_fwht;
+
+		if (f->type == V4L2_BUF_TYPE_VIDEO_OUTPUT) {
+			coded_w = f->fmt.pix.width;
+			coded_h = f->fmt.pix.height;
+		} else {
+			coded_w = f->fmt.pix_mp.width;
+			coded_h = f->fmt.pix_mp.height;
+		}
+		if (vb2_is_busy(vq) && (coded_w != q_data->coded_width ||
+					coded_h != q_data->coded_height))
+			return -EBUSY;
+		size = coded_w * coded_h *
+			info->sizeimage_mult / info->sizeimage_div;
+		if (!ctx->is_stateless)
+			size += sizeof(struct fwht_cframe_hdr);
+
+		if (vb2_is_busy(vq_cap) && size > q_data_cap->sizeimage)
+			return -EBUSY;
+	}
+
 	ret = vidioc_s_fmt(file2ctx(file), f);
 	if (!ret) {
+		if (ctx->is_enc) {
+			q_data->visible_width = coded_w;
+			q_data->visible_height = coded_h;
+			q_data_cap->coded_width = coded_w;
+			q_data_cap->coded_height = coded_h;
+			q_data_cap->sizeimage = size;
+		}
+
 		switch (f->type) {
-		case V4L2_BUF_TYPE_VIDEO_CAPTURE:
 		case V4L2_BUF_TYPE_VIDEO_OUTPUT:
 			pix = &f->fmt.pix;
 			ctx->state.colorspace = pix->colorspace;
@@ -1082,7 +1116,6 @@ static int vidioc_s_fmt_vid_out(struct file *file, void *priv,
 			ctx->state.ycbcr_enc = pix->ycbcr_enc;
 			ctx->state.quantization = pix->quantization;
 			break;
-		case V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE:
 		case V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE:
 			pix_mp = &f->fmt.pix_mp;
 			ctx->state.colorspace = pix_mp->colorspace;
-- 
2.20.1


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

* [PATCH 5/9] vicodec: add V4L2_CID_MIN_BUFFERS_FOR_OUTPUT
  2019-06-06 12:11 [PATCH 0/9] vicodec: fixes and sync with v4 of codec spec Hans Verkuil
                   ` (3 preceding siblings ...)
  2019-06-06 12:11 ` [PATCH 4/9] vicodec: pass on enc output format to capture side Hans Verkuil
@ 2019-06-06 12:11 ` Hans Verkuil
  2019-06-06 12:11 ` [PATCH 6/9] vicodec: set KEY/PFRAME flag when decoding Hans Verkuil
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Hans Verkuil @ 2019-06-06 12:11 UTC (permalink / raw)
  To: linux-media; +Cc: Hans Verkuil

The stateful encoder requires the presence of this control.
Since a single buffer is sufficient for vicodec, we just
set this control to 1.

Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
---
 drivers/media/platform/vicodec/vicodec-core.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/media/platform/vicodec/vicodec-core.c b/drivers/media/platform/vicodec/vicodec-core.c
index 7a78d044072d..4bea4a57386d 100644
--- a/drivers/media/platform/vicodec/vicodec-core.c
+++ b/drivers/media/platform/vicodec/vicodec-core.c
@@ -1801,13 +1801,16 @@ static int vicodec_open(struct file *file)
 	file->private_data = &ctx->fh;
 	ctx->dev = dev;
 	hdl = &ctx->hdl;
-	v4l2_ctrl_handler_init(hdl, 4);
+	v4l2_ctrl_handler_init(hdl, 5);
 	v4l2_ctrl_new_std(hdl, &vicodec_ctrl_ops, V4L2_CID_MPEG_VIDEO_GOP_SIZE,
 			  1, 16, 1, 10);
 	v4l2_ctrl_new_std(hdl, &vicodec_ctrl_ops, V4L2_CID_FWHT_I_FRAME_QP,
 			  1, 31, 1, 20);
 	v4l2_ctrl_new_std(hdl, &vicodec_ctrl_ops, V4L2_CID_FWHT_P_FRAME_QP,
 			  1, 31, 1, 20);
+	if (ctx->is_enc)
+		v4l2_ctrl_new_std(hdl, &vicodec_ctrl_ops,
+				  V4L2_CID_MIN_BUFFERS_FOR_OUTPUT, 1, 1, 1, 1);
 	if (ctx->is_stateless)
 		v4l2_ctrl_new_custom(hdl, &vicodec_ctrl_stateless_state, NULL);
 	if (hdl->error) {
-- 
2.20.1


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

* [PATCH 6/9] vicodec: set KEY/PFRAME flag when decoding
  2019-06-06 12:11 [PATCH 0/9] vicodec: fixes and sync with v4 of codec spec Hans Verkuil
                   ` (4 preceding siblings ...)
  2019-06-06 12:11 ` [PATCH 5/9] vicodec: add V4L2_CID_MIN_BUFFERS_FOR_OUTPUT Hans Verkuil
@ 2019-06-06 12:11 ` Hans Verkuil
  2019-06-06 12:11 ` [PATCH 7/9] vicodec: use correct sizeimage value when draining Hans Verkuil
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Hans Verkuil @ 2019-06-06 12:11 UTC (permalink / raw)
  To: linux-media; +Cc: Hans Verkuil

Set V4L2_BUF_FLAG_P/KEYFRAME after decoding a frame.

Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
---
 drivers/media/platform/vicodec/vicodec-core.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/media/platform/vicodec/vicodec-core.c b/drivers/media/platform/vicodec/vicodec-core.c
index 4bea4a57386d..4b0062ac880c 100644
--- a/drivers/media/platform/vicodec/vicodec-core.c
+++ b/drivers/media/platform/vicodec/vicodec-core.c
@@ -329,6 +329,10 @@ static int device_process(struct vicodec_ctx *ctx,
 			copy_cap_to_ref(p_dst, ctx->state.info, &ctx->state);
 
 		vb2_set_plane_payload(&dst_vb->vb2_buf, 0, q_dst->sizeimage);
+		if (ntohl(ctx->state.header.flags) & FWHT_FL_I_FRAME)
+			dst_vb->flags |= V4L2_BUF_FLAG_KEYFRAME;
+		else
+			dst_vb->flags |= V4L2_BUF_FLAG_PFRAME;
 	}
 	return ret;
 }
@@ -407,7 +411,6 @@ static void device_run(void *priv)
 	u32 state;
 	struct media_request *src_req;
 
-
 	src_buf = v4l2_m2m_next_src_buf(ctx->fh.m2m_ctx);
 	dst_buf = v4l2_m2m_dst_buf_remove(ctx->fh.m2m_ctx);
 	src_req = src_buf->vb2_buf.req_obj.req;
@@ -421,7 +424,7 @@ static void device_run(void *priv)
 	else
 		dst_buf->sequence = q_dst->sequence++;
 	dst_buf->flags &= ~V4L2_BUF_FLAG_LAST;
-	v4l2_m2m_buf_copy_metadata(src_buf, dst_buf, !ctx->is_enc);
+	v4l2_m2m_buf_copy_metadata(src_buf, dst_buf, false);
 
 	ctx->last_dst_buf = dst_buf;
 
-- 
2.20.1


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

* [PATCH 7/9] vicodec: use correct sizeimage value when draining
  2019-06-06 12:11 [PATCH 0/9] vicodec: fixes and sync with v4 of codec spec Hans Verkuil
                   ` (5 preceding siblings ...)
  2019-06-06 12:11 ` [PATCH 6/9] vicodec: set KEY/PFRAME flag when decoding Hans Verkuil
@ 2019-06-06 12:11 ` Hans Verkuil
  2019-06-06 12:11 ` [PATCH 8/9] vicodec: stateless codecs do not have EOS and SOURCE_CHANGE events Hans Verkuil
  2019-06-06 12:11 ` [PATCH 9/9] vicodec: improve handling of ENC_CMD_STOP/START Hans Verkuil
  8 siblings, 0 replies; 10+ messages in thread
From: Hans Verkuil @ 2019-06-06 12:11 UTC (permalink / raw)
  To: linux-media; +Cc: Hans Verkuil

After a resolution change is detected, q_data->sizeimage is updated
to the new format, but buf_prepare is still draining buffers that
need to use the old pre-resolution-change value. So store the sizeimage
value in q_data->vb2_sizeimage in queue_setup and use that in
buf_prepare.

Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
---
 drivers/media/platform/vicodec/vicodec-core.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/media/platform/vicodec/vicodec-core.c b/drivers/media/platform/vicodec/vicodec-core.c
index 4b0062ac880c..ce7f7bf1b998 100644
--- a/drivers/media/platform/vicodec/vicodec-core.c
+++ b/drivers/media/platform/vicodec/vicodec-core.c
@@ -84,6 +84,7 @@ struct vicodec_q_data {
 	unsigned int		visible_width;
 	unsigned int		visible_height;
 	unsigned int		sizeimage;
+	unsigned int		vb2_sizeimage;
 	unsigned int		sequence;
 	const struct v4l2_fwht_pixfmt_info *info;
 };
@@ -1361,6 +1362,7 @@ static int vicodec_queue_setup(struct vb2_queue *vq, unsigned int *nbuffers,
 
 	*nplanes = 1;
 	sizes[0] = size;
+	q_data->vb2_sizeimage = size;
 	return 0;
 }
 
@@ -1391,11 +1393,11 @@ static int vicodec_buf_prepare(struct vb2_buffer *vb)
 		}
 	}
 
-	if (vb2_plane_size(vb, 0) < q_data->sizeimage) {
+	if (vb2_plane_size(vb, 0) < q_data->vb2_sizeimage) {
 		dprintk(ctx->dev,
 			"%s data will not fit into plane (%lu < %lu)\n",
 			__func__, vb2_plane_size(vb, 0),
-			(long)q_data->sizeimage);
+			(long)q_data->vb2_sizeimage);
 		return -EINVAL;
 	}
 
-- 
2.20.1


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

* [PATCH 8/9] vicodec: stateless codecs do not have EOS and SOURCE_CHANGE events
  2019-06-06 12:11 [PATCH 0/9] vicodec: fixes and sync with v4 of codec spec Hans Verkuil
                   ` (6 preceding siblings ...)
  2019-06-06 12:11 ` [PATCH 7/9] vicodec: use correct sizeimage value when draining Hans Verkuil
@ 2019-06-06 12:11 ` Hans Verkuil
  2019-06-06 12:11 ` [PATCH 9/9] vicodec: improve handling of ENC_CMD_STOP/START Hans Verkuil
  8 siblings, 0 replies; 10+ messages in thread
From: Hans Verkuil @ 2019-06-06 12:11 UTC (permalink / raw)
  To: linux-media; +Cc: Hans Verkuil

Return an error when attempting to subscribe to those events
for a stateless codec.

Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
---
 drivers/media/platform/vicodec/vicodec-core.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/media/platform/vicodec/vicodec-core.c b/drivers/media/platform/vicodec/vicodec-core.c
index ce7f7bf1b998..91cd0c1dbede 100644
--- a/drivers/media/platform/vicodec/vicodec-core.c
+++ b/drivers/media/platform/vicodec/vicodec-core.c
@@ -1293,6 +1293,8 @@ static int vicodec_subscribe_event(struct v4l2_fh *fh,
 			return -EINVAL;
 		/* fall through */
 	case V4L2_EVENT_EOS:
+		if (ctx->is_stateless)
+			return -EINVAL;
 		return v4l2_event_subscribe(fh, sub, 0, NULL);
 	default:
 		return v4l2_ctrl_subscribe_event(fh, sub);
-- 
2.20.1


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

* [PATCH 9/9] vicodec: improve handling of ENC_CMD_STOP/START
  2019-06-06 12:11 [PATCH 0/9] vicodec: fixes and sync with v4 of codec spec Hans Verkuil
                   ` (7 preceding siblings ...)
  2019-06-06 12:11 ` [PATCH 8/9] vicodec: stateless codecs do not have EOS and SOURCE_CHANGE events Hans Verkuil
@ 2019-06-06 12:11 ` Hans Verkuil
  8 siblings, 0 replies; 10+ messages in thread
From: Hans Verkuil @ 2019-06-06 12:11 UTC (permalink / raw)
  To: linux-media; +Cc: Hans Verkuil

Correctly handle stopping and restarting the encoder, keeping
track of the stop and drain states.

In addition it adds correct handling of corner cases, allowing
v4l2-compliance to pass.

Unfortunately, the code is getting to be quite complicated, so
we need to work on better codec support in v4l2-mem2mem.c to
simplify drivers.

Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
---
 drivers/media/platform/vicodec/vicodec-core.c | 150 ++++++++++++++----
 1 file changed, 122 insertions(+), 28 deletions(-)

diff --git a/drivers/media/platform/vicodec/vicodec-core.c b/drivers/media/platform/vicodec/vicodec-core.c
index 91cd0c1dbede..7e7c1e80f29f 100644
--- a/drivers/media/platform/vicodec/vicodec-core.c
+++ b/drivers/media/platform/vicodec/vicodec-core.c
@@ -117,12 +117,14 @@ struct vicodec_ctx {
 	struct vicodec_dev	*dev;
 	bool			is_enc;
 	bool			is_stateless;
+	bool			is_draining;
+	bool			next_is_last;
+	bool			has_stopped;
 	spinlock_t		*lock;
 
 	struct v4l2_ctrl_handler hdl;
 
 	struct vb2_v4l2_buffer *last_src_buf;
-	struct vb2_v4l2_buffer *last_dst_buf;
 
 	/* Source and destination queue data */
 	struct vicodec_q_data   q_data[2];
@@ -139,6 +141,10 @@ struct vicodec_ctx {
 	bool			source_changed;
 };
 
+static const struct v4l2_event vicodec_eos_event = {
+	.type = V4L2_EVENT_EOS
+};
+
 static inline struct vicodec_ctx *file2ctx(struct file *file)
 {
 	return container_of(file->private_data, struct vicodec_ctx, fh);
@@ -402,9 +408,6 @@ static enum vb2_buffer_state get_next_header(struct vicodec_ctx *ctx,
 /* device_run() - prepares and starts the device */
 static void device_run(void *priv)
 {
-	static const struct v4l2_event eos_event = {
-		.type = V4L2_EVENT_EOS
-	};
 	struct vicodec_ctx *ctx = priv;
 	struct vicodec_dev *dev = ctx->dev;
 	struct vb2_v4l2_buffer *src_buf, *dst_buf;
@@ -427,12 +430,12 @@ static void device_run(void *priv)
 	dst_buf->flags &= ~V4L2_BUF_FLAG_LAST;
 	v4l2_m2m_buf_copy_metadata(src_buf, dst_buf, false);
 
-	ctx->last_dst_buf = dst_buf;
-
 	spin_lock(ctx->lock);
 	if (!ctx->comp_has_next_frame && src_buf == ctx->last_src_buf) {
 		dst_buf->flags |= V4L2_BUF_FLAG_LAST;
-		v4l2_event_queue_fh(&ctx->fh, &eos_event);
+		v4l2_event_queue_fh(&ctx->fh, &vicodec_eos_event);
+		ctx->is_draining = false;
+		ctx->has_stopped = true;
 	}
 	if (ctx->is_enc || ctx->is_stateless) {
 		src_buf->sequence = q_src->sequence++;
@@ -583,6 +586,8 @@ static int job_ready(void *priv)
 	unsigned int max_to_copy;
 	unsigned int comp_frame_size;
 
+	if (ctx->has_stopped)
+		return 0;
 	if (ctx->source_changed)
 		return 0;
 	if (ctx->is_stateless || ctx->is_enc || ctx->comp_has_frame)
@@ -602,6 +607,8 @@ static int job_ready(void *priv)
 	if (ctx->header_size < sizeof(struct fwht_cframe_hdr)) {
 		state = get_next_header(ctx, &p, p_src + sz - p);
 		if (ctx->header_size < sizeof(struct fwht_cframe_hdr)) {
+			if (ctx->is_draining && src_buf == ctx->last_src_buf)
+				return 1;
 			job_remove_src_buf(ctx, state);
 			goto restart;
 		}
@@ -629,6 +636,8 @@ static int job_ready(void *priv)
 		p += copy;
 		ctx->comp_size += copy;
 		if (ctx->comp_size < max_to_copy) {
+			if (ctx->is_draining && src_buf == ctx->last_src_buf)
+				return 1;
 			job_remove_src_buf(ctx, state);
 			goto restart;
 		}
@@ -670,7 +679,6 @@ static int job_ready(void *priv)
 			v4l2_m2m_dst_buf_remove(ctx->fh.m2m_ctx);
 
 		update_capture_data_from_header(ctx);
-		ctx->first_source_change_sent = true;
 		v4l2_event_queue_fh(&ctx->fh, &rs_event);
 		set_last_buffer(dst_buf, src_buf, ctx);
 		ctx->source_changed = true;
@@ -717,7 +725,8 @@ static int enum_fmt(struct v4l2_fmtdesc *f, struct vicodec_ctx *ctx,
 		const struct v4l2_fwht_pixfmt_info *info =
 					get_q_data(ctx, f->type)->info;
 
-		if (!info || ctx->is_enc)
+		if (ctx->is_enc ||
+		    !vb2_is_streaming(&ctx->fh.m2m_ctx->cap_q_ctx.q))
 			info = v4l2_fwht_get_pixfmt(f->index);
 		else
 			info = v4l2_fwht_find_nth_fmt(info->width_div,
@@ -768,9 +777,6 @@ static int vidioc_g_fmt(struct vicodec_ctx *ctx, struct v4l2_format *f)
 	q_data = get_q_data(ctx, f->type);
 	info = q_data->info;
 
-	if (!info)
-		info = v4l2_fwht_get_pixfmt(0);
-
 	switch (f->type) {
 	case V4L2_BUF_TYPE_VIDEO_CAPTURE:
 	case V4L2_BUF_TYPE_VIDEO_OUTPUT:
@@ -1210,19 +1216,39 @@ static int vidioc_s_selection(struct file *file, void *priv,
 	return 0;
 }
 
-static void vicodec_mark_last_buf(struct vicodec_ctx *ctx)
+static int vicodec_mark_last_buf(struct vicodec_ctx *ctx)
 {
-	static const struct v4l2_event eos_event = {
-		.type = V4L2_EVENT_EOS
-	};
+	struct vb2_v4l2_buffer *next_dst_buf;
+	int ret = 0;
 
 	spin_lock(ctx->lock);
+	if (ctx->is_draining) {
+		ret = -EBUSY;
+		goto unlock;
+	}
+	if (ctx->has_stopped)
+		goto unlock;
+
 	ctx->last_src_buf = v4l2_m2m_last_src_buf(ctx->fh.m2m_ctx);
-	if (!ctx->last_src_buf && ctx->last_dst_buf) {
-		ctx->last_dst_buf->flags |= V4L2_BUF_FLAG_LAST;
-		v4l2_event_queue_fh(&ctx->fh, &eos_event);
+	ctx->is_draining = true;
+	if (ctx->last_src_buf)
+		goto unlock;
+
+	next_dst_buf = v4l2_m2m_dst_buf_remove(ctx->fh.m2m_ctx);
+	if (!next_dst_buf) {
+		ctx->next_is_last = true;
+		goto unlock;
 	}
+
+	next_dst_buf->flags |= V4L2_BUF_FLAG_LAST;
+	vb2_buffer_done(&next_dst_buf->vb2_buf, VB2_BUF_STATE_DONE);
+	ctx->is_draining = false;
+	ctx->has_stopped = true;
+	v4l2_event_queue_fh(&ctx->fh, &vicodec_eos_event);
+
+unlock:
 	spin_unlock(ctx->lock);
+	return ret;
 }
 
 static int vicodec_encoder_cmd(struct file *file, void *fh,
@@ -1235,8 +1261,22 @@ static int vicodec_encoder_cmd(struct file *file, void *fh,
 	if (ret < 0)
 		return ret;
 
-	vicodec_mark_last_buf(ctx);
-	return 0;
+	if (!vb2_is_streaming(&ctx->fh.m2m_ctx->cap_q_ctx.q) ||
+	    !vb2_is_streaming(&ctx->fh.m2m_ctx->out_q_ctx.q))
+		return 0;
+
+	if (ec->cmd == V4L2_ENC_CMD_STOP)
+		return vicodec_mark_last_buf(ctx);
+	ret = 0;
+	spin_lock(ctx->lock);
+	if (ctx->is_draining) {
+		ret = -EBUSY;
+	} else if (ctx->has_stopped) {
+		ctx->has_stopped = false;
+		vb2_clear_last_buffer_dequeued(&ctx->fh.m2m_ctx->cap_q_ctx.q);
+	}
+	spin_unlock(ctx->lock);
+	return ret;
 }
 
 static int vicodec_decoder_cmd(struct file *file, void *fh,
@@ -1249,8 +1289,22 @@ static int vicodec_decoder_cmd(struct file *file, void *fh,
 	if (ret < 0)
 		return ret;
 
-	vicodec_mark_last_buf(ctx);
-	return 0;
+	if (!vb2_is_streaming(&ctx->fh.m2m_ctx->cap_q_ctx.q) ||
+	    !vb2_is_streaming(&ctx->fh.m2m_ctx->out_q_ctx.q))
+		return 0;
+
+	if (dc->cmd == V4L2_DEC_CMD_STOP)
+		return vicodec_mark_last_buf(ctx);
+	ret = 0;
+	spin_lock(ctx->lock);
+	if (ctx->is_draining) {
+		ret = -EBUSY;
+	} else if (ctx->has_stopped) {
+		ctx->has_stopped = false;
+		vb2_clear_last_buffer_dequeued(&ctx->fh.m2m_ctx->cap_q_ctx.q);
+	}
+	spin_unlock(ctx->lock);
+	return ret;
 }
 
 static int vicodec_enum_framesizes(struct file *file, void *fh,
@@ -1423,6 +1477,25 @@ static void vicodec_buf_queue(struct vb2_buffer *vb)
 		.u.src_change.changes = V4L2_EVENT_SRC_CH_RESOLUTION,
 	};
 
+	if (vb2_is_streaming(vq_cap)) {
+		if (!V4L2_TYPE_IS_OUTPUT(vb->vb2_queue->type) &&
+		    ctx->next_is_last) {
+			unsigned int i;
+
+			for (i = 0; i < vb->num_planes; i++)
+				vb->planes[i].bytesused = 0;
+			vbuf->flags = V4L2_BUF_FLAG_LAST;
+			vbuf->field = V4L2_FIELD_NONE;
+			vbuf->sequence = get_q_data(ctx, vb->vb2_queue->type)->sequence++;
+			vb2_buffer_done(vb, VB2_BUF_STATE_DONE);
+			ctx->is_draining = false;
+			ctx->has_stopped = true;
+			ctx->next_is_last = false;
+			v4l2_event_queue_fh(&ctx->fh, &vicodec_eos_event);
+			return;
+		}
+	}
+
 	/* buf_queue handles only the first source change event */
 	if (ctx->first_source_change_sent) {
 		v4l2_m2m_buf_queue(ctx->fh.m2m_ctx, vbuf);
@@ -1530,16 +1603,11 @@ static int vicodec_start_streaming(struct vb2_queue *q,
 	unsigned int total_planes_size;
 	u8 *new_comp_frame = NULL;
 
-	if (!info)
-		return -EINVAL;
-
 	chroma_div = info->width_div * info->height_div;
 	q_data->sequence = 0;
 
 	if (V4L2_TYPE_IS_OUTPUT(q->type))
 		ctx->last_src_buf = NULL;
-	else
-		ctx->last_dst_buf = NULL;
 
 	state->gop_cnt = 0;
 
@@ -1615,6 +1683,32 @@ static void vicodec_stop_streaming(struct vb2_queue *q)
 
 	vicodec_return_bufs(q, VB2_BUF_STATE_ERROR);
 
+	if (V4L2_TYPE_IS_OUTPUT(q->type)) {
+		if (ctx->is_draining) {
+			struct vb2_v4l2_buffer *next_dst_buf;
+
+			spin_lock(ctx->lock);
+			ctx->last_src_buf = NULL;
+			next_dst_buf = v4l2_m2m_dst_buf_remove(ctx->fh.m2m_ctx);
+			if (!next_dst_buf) {
+				ctx->next_is_last = true;
+			} else {
+				next_dst_buf->flags |= V4L2_BUF_FLAG_LAST;
+				vb2_buffer_done(&next_dst_buf->vb2_buf, VB2_BUF_STATE_DONE);
+				ctx->is_draining = false;
+				ctx->has_stopped = true;
+				v4l2_event_queue_fh(&ctx->fh, &vicodec_eos_event);
+			}
+			spin_unlock(ctx->lock);
+		}
+	} else {
+		ctx->is_draining = false;
+		ctx->has_stopped = false;
+		ctx->next_is_last = false;
+	}
+	if (!ctx->is_enc && V4L2_TYPE_IS_OUTPUT(q->type))
+		ctx->first_source_change_sent = false;
+
 	if ((!V4L2_TYPE_IS_OUTPUT(q->type) && !ctx->is_enc) ||
 	    (V4L2_TYPE_IS_OUTPUT(q->type) && ctx->is_enc)) {
 		if (!ctx->is_stateless)
-- 
2.20.1


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

end of thread, other threads:[~2019-06-06 12:11 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-06-06 12:11 [PATCH 0/9] vicodec: fixes and sync with v4 of codec spec Hans Verkuil
2019-06-06 12:11 ` [PATCH 1/9] vicodec: move v4l2_ctrl_request_complete after spin_unlock Hans Verkuil
2019-06-06 12:11 ` [PATCH 2/9] vicodec: always return a valid format Hans Verkuil
2019-06-06 12:11 ` [PATCH 3/9] vicodec: fix initial stateless sizeimage value Hans Verkuil
2019-06-06 12:11 ` [PATCH 4/9] vicodec: pass on enc output format to capture side Hans Verkuil
2019-06-06 12:11 ` [PATCH 5/9] vicodec: add V4L2_CID_MIN_BUFFERS_FOR_OUTPUT Hans Verkuil
2019-06-06 12:11 ` [PATCH 6/9] vicodec: set KEY/PFRAME flag when decoding Hans Verkuil
2019-06-06 12:11 ` [PATCH 7/9] vicodec: use correct sizeimage value when draining Hans Verkuil
2019-06-06 12:11 ` [PATCH 8/9] vicodec: stateless codecs do not have EOS and SOURCE_CHANGE events Hans Verkuil
2019-06-06 12:11 ` [PATCH 9/9] vicodec: improve handling of ENC_CMD_STOP/START Hans Verkuil

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).