Re: [PATCH 14/16] s5p-jpeg: Synchronize V4L2_CID_JPEG_CHROMA_SUBSAMPLING control value
On 11/19/2013 03:46 PM, Hans Verkuil wrote:
On 11/19/2013 03:27 PM, Jacek Anaszewski wrote:
When output queue fourcc is set to any flavour of YUV,
the V4L2_CID_JPEG_CHROMA_SUBSAMPLING control value as
well as its in-driver cached counterpart have to be
updated with the subsampling property of the format
so as to be able to provide correct information to the
user space and preclude setting an illegal subsampling
mode for Exynos4x12 encoder.
Signed-off-by: Jacek Anaszewski j.anaszew...@samsung.com
Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com
---
drivers/media/platform/s5p-jpeg/jpeg-core.c |5 +
1 file changed, 5 insertions(+)
diff --git a/drivers/media/platform/s5p-jpeg/jpeg-core.c
b/drivers/media/platform/s5p-jpeg/jpeg-core.c
index 319be0c..d4db612 100644
--- a/drivers/media/platform/s5p-jpeg/jpeg-core.c
+++ b/drivers/media/platform/s5p-jpeg/jpeg-core.c
@@ -1038,6 +1038,7 @@ static int s5p_jpeg_try_fmt_vid_out(struct file *file,
void *priv,
{
struct s5p_jpeg_ctx *ctx = fh_to_ctx(priv);
struct s5p_jpeg_fmt *fmt;
+ struct v4l2_control ctrl_subs;
fmt = s5p_jpeg_find_format(ctx, f-fmt.pix.pixelformat,
FMT_TYPE_OUTPUT);
@@ -1048,6 +1049,10 @@ static int s5p_jpeg_try_fmt_vid_out(struct file *file,
void *priv,
return -EINVAL;
}
+ ctrl_subs.id = V4L2_CID_JPEG_CHROMA_SUBSAMPLING;
+ ctrl_subs.value = fmt-subsampling;
+ v4l2_s_ctrl(priv, ctx-ctrl_handler, ctrl_subs);
TRY_FMT should never have side-effects, so this isn't the correct
way of implementing this.
I am aware of it, but I couldn't have found more suitable place
for implementing this. Below is the rationale standing behind
such an implementation:
- Exynos4x12 device doesn't generate an eoc interrupt if the
subsampling property of an output queue format is lower than the
target jpeg subsampling (e.g. V4L2_PIX_FMT_YUYV [4:2:2 subsampling]
and JPEG 4:4:4)
- It should be possible to inform the user space application that the
subsampling it wants to set is not supported with the current output
queue fourcc.
- It is possible that after calling S_EXT_CTRLS the application
will call S_FMT on output queue with different fourcc which will
change the allowed scope of JPEG subsampling settings. Let's assume
the following flow of ioctls:
- S_FMT V4L2_PIX_FMT_YUYV (4:2:2)
- S_EXT_CTRLS V4L2_JPEG_CHROMA_SUBSAMPLING_422
- S_FMT V4L2_PIX_FMT_YUV420
Now the JPEG subsampling set is illegal as 4:2:2 is lower than 4:2:0
(lower refers here to the lower number of luma samples assigned
to the single chroma sample). It is evident now that the change
of output queue fourcc entails change of the allowed scope of JPEG
subsampling settings. The way I implemented it reflects this
constraint precisely. We could go for adjusting the JPEG subsampling
e.g. in the device_run callback but the user space application
wouldn't know about it unless it called G_EXT_CTRLS ioctl after end
of conversion.
In view of the above it is clear that calling S_FMT in this case HAS
side effect no matter whether we take it into account in the driver
implementation or not. Nevertheless maybe there is some more elegant
way of handling this problem I am not aware of. I am open to any
interesting ideas.
Regards,
Jacek Anaszewski
Also, don't use v4l2_s_ctrl, instead use v4l2_ctrl_s_ctrl. The v4l2_s_ctrl
function is for core framework use only, not for use in drivers.
Regards,
Hans
+
return vidioc_try_fmt(f, fmt, ctx, FMT_TYPE_OUTPUT);
}
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 14/16] s5p-jpeg: Synchronize V4L2_CID_JPEG_CHROMA_SUBSAMPLING control value
On 11/20/13 14:47, Jacek Anaszewski wrote:
On 11/19/2013 03:46 PM, Hans Verkuil wrote:
On 11/19/2013 03:27 PM, Jacek Anaszewski wrote:
When output queue fourcc is set to any flavour of YUV,
the V4L2_CID_JPEG_CHROMA_SUBSAMPLING control value as
well as its in-driver cached counterpart have to be
updated with the subsampling property of the format
so as to be able to provide correct information to the
user space and preclude setting an illegal subsampling
mode for Exynos4x12 encoder.
Signed-off-by: Jacek Anaszewski j.anaszew...@samsung.com
Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com
---
drivers/media/platform/s5p-jpeg/jpeg-core.c |5 +
1 file changed, 5 insertions(+)
diff --git a/drivers/media/platform/s5p-jpeg/jpeg-core.c
b/drivers/media/platform/s5p-jpeg/jpeg-core.c
index 319be0c..d4db612 100644
--- a/drivers/media/platform/s5p-jpeg/jpeg-core.c
+++ b/drivers/media/platform/s5p-jpeg/jpeg-core.c
@@ -1038,6 +1038,7 @@ static int s5p_jpeg_try_fmt_vid_out(struct file
*file, void *priv,
{
struct s5p_jpeg_ctx *ctx = fh_to_ctx(priv);
struct s5p_jpeg_fmt *fmt;
+struct v4l2_control ctrl_subs;
fmt = s5p_jpeg_find_format(ctx, f-fmt.pix.pixelformat,
FMT_TYPE_OUTPUT);
@@ -1048,6 +1049,10 @@ static int s5p_jpeg_try_fmt_vid_out(struct file
*file, void *priv,
return -EINVAL;
}
+ctrl_subs.id = V4L2_CID_JPEG_CHROMA_SUBSAMPLING;
+ctrl_subs.value = fmt-subsampling;
+v4l2_s_ctrl(priv, ctx-ctrl_handler, ctrl_subs);
TRY_FMT should never have side-effects, so this isn't the correct
way of implementing this.
I am aware of it, but I couldn't have found more suitable place
for implementing this. Below is the rationale standing behind
such an implementation:
- Exynos4x12 device doesn't generate an eoc interrupt if the
subsampling property of an output queue format is lower than the
target jpeg subsampling (e.g. V4L2_PIX_FMT_YUYV [4:2:2 subsampling]
and JPEG 4:4:4)
- It should be possible to inform the user space application that the
subsampling it wants to set is not supported with the current output
queue fourcc.
- It is possible that after calling S_EXT_CTRLS the application
will call S_FMT on output queue with different fourcc which will
change the allowed scope of JPEG subsampling settings. Let's assume
the following flow of ioctls:
- S_FMT V4L2_PIX_FMT_YUYV (4:2:2)
- S_EXT_CTRLS V4L2_JPEG_CHROMA_SUBSAMPLING_422
- S_FMT V4L2_PIX_FMT_YUV420
Now the JPEG subsampling set is illegal as 4:2:2 is lower than 4:2:0
(lower refers here to the lower number of luma samples assigned
to the single chroma sample). It is evident now that the change
of output queue fourcc entails change of the allowed scope of JPEG
subsampling settings. The way I implemented it reflects this
constraint precisely. We could go for adjusting the JPEG subsampling
e.g. in the device_run callback but the user space application
wouldn't know about it unless it called G_EXT_CTRLS ioctl after end
of conversion.
In view of the above it is clear that calling S_FMT in this case HAS
side effect no matter whether we take it into account in the driver
implementation or not. Nevertheless maybe there is some more elegant
way of handling this problem I am not aware of. I am open to any
interesting ideas.
I think you misunderstood me. It is OK that S_FMT changes the control,
but TRY_FMT shouldn't. So the code that changes the subsampling control
should be moved to s5p_jpeg_s_fmt_vid_out.
Regards,
Hans
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 14/16] s5p-jpeg: Synchronize V4L2_CID_JPEG_CHROMA_SUBSAMPLING control value
On 11/19/2013 03:27 PM, Jacek Anaszewski wrote:
When output queue fourcc is set to any flavour of YUV,
the V4L2_CID_JPEG_CHROMA_SUBSAMPLING control value as
well as its in-driver cached counterpart have to be
updated with the subsampling property of the format
so as to be able to provide correct information to the
user space and preclude setting an illegal subsampling
mode for Exynos4x12 encoder.
Signed-off-by: Jacek Anaszewski j.anaszew...@samsung.com
Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com
---
drivers/media/platform/s5p-jpeg/jpeg-core.c |5 +
1 file changed, 5 insertions(+)
diff --git a/drivers/media/platform/s5p-jpeg/jpeg-core.c
b/drivers/media/platform/s5p-jpeg/jpeg-core.c
index 319be0c..d4db612 100644
--- a/drivers/media/platform/s5p-jpeg/jpeg-core.c
+++ b/drivers/media/platform/s5p-jpeg/jpeg-core.c
@@ -1038,6 +1038,7 @@ static int s5p_jpeg_try_fmt_vid_out(struct file *file,
void *priv,
{
struct s5p_jpeg_ctx *ctx = fh_to_ctx(priv);
struct s5p_jpeg_fmt *fmt;
+ struct v4l2_control ctrl_subs;
fmt = s5p_jpeg_find_format(ctx, f-fmt.pix.pixelformat,
FMT_TYPE_OUTPUT);
@@ -1048,6 +1049,10 @@ static int s5p_jpeg_try_fmt_vid_out(struct file *file,
void *priv,
return -EINVAL;
}
+ ctrl_subs.id = V4L2_CID_JPEG_CHROMA_SUBSAMPLING;
+ ctrl_subs.value = fmt-subsampling;
+ v4l2_s_ctrl(priv, ctx-ctrl_handler, ctrl_subs);
TRY_FMT should never have side-effects, so this isn't the correct
way of implementing this.
Also, don't use v4l2_s_ctrl, instead use v4l2_ctrl_s_ctrl. The v4l2_s_ctrl
function is for core framework use only, not for use in drivers.
Regards,
Hans
+
return vidioc_try_fmt(f, fmt, ctx, FMT_TYPE_OUTPUT);
}
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
