* [PATCH v2 0/2] Add control for VP9 profile
@ 2018-05-30 7:16 ` Keiichi Watanabe
0 siblings, 0 replies; 25+ messages in thread
From: Keiichi Watanabe @ 2018-05-30 7:16 UTC (permalink / raw)
To: linux-arm-kernel
Cc: Mauro Carvalho Chehab, Tiffany Lin, Andrew-CT Chen,
Matthias Brugger, Hans Verkuil, Sakari Ailus, Sylwester Nawrocki,
Smitha T Murthy, Keiichi Watanabe, Tom Saeger, Andy Shevchenko,
Tomasz Figa, Ricardo Ribalda Delgado, linux-media, linux-kernel,
linux-mediatek
This patch series adds a new control V4L2_CID_MPEG_VIDEO_VP9_PROFILE
for VP9 profile.
This control can be used to select a desired profile for VP9 encoder.
In addition, it is also used to query for supported profiles by an
encoder or a decoder.
Patch 1 adds this control.
By patch 2, this control is added to MediaTek decoder's driver, which
supports VP9 profile 0.
Version 2 changes:
- Support this control in MediaTek decoder's driver
Keiichi Watanabe (2):
media: v4l2-ctrl: Add control for VP9 profile
media: mtk-vcodec: Support VP9 profile in decoder
.../media/uapi/v4l/extended-controls.rst | 26 +++++++++++++++++++
.../platform/mtk-vcodec/mtk_vcodec_dec.c | 6 +++++
drivers/media/v4l2-core/v4l2-ctrls.c | 12 +++++++++
include/uapi/linux/v4l2-controls.h | 8 ++++++
4 files changed, 52 insertions(+)
--
2.17.0.921.gf22659ad46-goog
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH v2 0/2] Add control for VP9 profile
@ 2018-05-30 7:16 ` Keiichi Watanabe
0 siblings, 0 replies; 25+ messages in thread
From: Keiichi Watanabe @ 2018-05-30 7:16 UTC (permalink / raw)
To: linux-arm-kernel
This patch series adds a new control V4L2_CID_MPEG_VIDEO_VP9_PROFILE
for VP9 profile.
This control can be used to select a desired profile for VP9 encoder.
In addition, it is also used to query for supported profiles by an
encoder or a decoder.
Patch 1 adds this control.
By patch 2, this control is added to MediaTek decoder's driver, which
supports VP9 profile 0.
Version 2 changes:
- Support this control in MediaTek decoder's driver
Keiichi Watanabe (2):
media: v4l2-ctrl: Add control for VP9 profile
media: mtk-vcodec: Support VP9 profile in decoder
.../media/uapi/v4l/extended-controls.rst | 26 +++++++++++++++++++
.../platform/mtk-vcodec/mtk_vcodec_dec.c | 6 +++++
drivers/media/v4l2-core/v4l2-ctrls.c | 12 +++++++++
include/uapi/linux/v4l2-controls.h | 8 ++++++
4 files changed, 52 insertions(+)
--
2.17.0.921.gf22659ad46-goog
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH v2 1/2] media: v4l2-ctrl: Add control for VP9 profile
2018-05-30 7:16 ` Keiichi Watanabe
@ 2018-05-30 7:16 ` Keiichi Watanabe
-1 siblings, 0 replies; 25+ messages in thread
From: Keiichi Watanabe @ 2018-05-30 7:16 UTC (permalink / raw)
To: linux-arm-kernel
Cc: Mauro Carvalho Chehab, Tiffany Lin, Andrew-CT Chen,
Matthias Brugger, Hans Verkuil, Sakari Ailus, Sylwester Nawrocki,
Smitha T Murthy, Keiichi Watanabe, Tom Saeger, Andy Shevchenko,
Tomasz Figa, Ricardo Ribalda Delgado, linux-media, linux-kernel,
linux-mediatek
Add a new control V4L2_CID_MPEG_VIDEO_VP9_PROFILE for selecting desired
profile for VP9 encoder and querying for supported profiles by VP9 encoder
or decoder.
An existing control V4L2_CID_MPEG_VIDEO_VPX_PROFILE cannot be
used for querying since it is not a menu control but an integer
control, which cannot return an arbitrary set of supported profiles.
The new control V4L2_CID_MPEG_VIDEO_VP9_PROFILE is a menu control as
with controls for other codec profiles. (e.g. H264)
Signed-off-by: Keiichi Watanabe <keiichiw@chromium.org>
---
.../media/uapi/v4l/extended-controls.rst | 26 +++++++++++++++++++
drivers/media/v4l2-core/v4l2-ctrls.c | 12 +++++++++
include/uapi/linux/v4l2-controls.h | 8 ++++++
3 files changed, 46 insertions(+)
diff --git a/Documentation/media/uapi/v4l/extended-controls.rst b/Documentation/media/uapi/v4l/extended-controls.rst
index 03931f9b1285..4f7f128a4998 100644
--- a/Documentation/media/uapi/v4l/extended-controls.rst
+++ b/Documentation/media/uapi/v4l/extended-controls.rst
@@ -1959,6 +1959,32 @@ enum v4l2_vp8_golden_frame_sel -
Select the desired profile for VPx encoder. Acceptable values are 0,
1, 2 and 3 corresponding to encoder profiles 0, 1, 2 and 3.
+.. _v4l2-mpeg-video-vp9-profile:
+
+``V4L2_CID_MPEG_VIDEO_VP9_PROFILE``
+ (enum)
+
+enum v4l2_mpeg_video_vp9_profile -
+ This control allows to select the profile for VP9 encoder.
+ This is also used to enumerate supported profiles by VP9 encoder or decoder.
+ Possible values are:
+
+
+
+.. flat-table::
+ :header-rows: 0
+ :stub-columns: 0
+
+ * - ``V4L2_MPEG_VIDEO_VP9_PROFILE_0``
+ - Profile 0
+ * - ``V4L2_MPEG_VIDEO_VP9_PROFILE_1``
+ - Profile 1
+ * - ``V4L2_MPEG_VIDEO_VP9_PROFILE_2``
+ - Profile 2
+ * - ``V4L2_MPEG_VIDEO_VP9_PROFILE_3``
+ - Profile 3
+
+
High Efficiency Video Coding (HEVC/H.265) Control Reference
-----------------------------------------------------------
diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c b/drivers/media/v4l2-core/v4l2-ctrls.c
index d29e45516eb7..401ce21c2e63 100644
--- a/drivers/media/v4l2-core/v4l2-ctrls.c
+++ b/drivers/media/v4l2-core/v4l2-ctrls.c
@@ -431,6 +431,13 @@ const char * const *v4l2_ctrl_get_menu(u32 id)
"Use Previous Specific Frame",
NULL,
};
+ static const char * const vp9_profile[] = {
+ "0",
+ "1",
+ "2",
+ "3",
+ NULL,
+ };
static const char * const flash_led_mode[] = {
"Off",
@@ -614,6 +621,8 @@ const char * const *v4l2_ctrl_get_menu(u32 id)
return mpeg4_profile;
case V4L2_CID_MPEG_VIDEO_VPX_GOLDEN_FRAME_SEL:
return vpx_golden_frame_sel;
+ case V4L2_CID_MPEG_VIDEO_VP9_PROFILE:
+ return vp9_profile;
case V4L2_CID_JPEG_CHROMA_SUBSAMPLING:
return jpeg_chroma_subsampling;
case V4L2_CID_DV_TX_MODE:
@@ -841,6 +850,8 @@ const char *v4l2_ctrl_get_name(u32 id)
case V4L2_CID_MPEG_VIDEO_VPX_P_FRAME_QP: return "VPX P-Frame QP Value";
case V4L2_CID_MPEG_VIDEO_VPX_PROFILE: return "VPX Profile";
+ case V4L2_CID_MPEG_VIDEO_VP9_PROFILE: return "VP9 Profile";
+
/* HEVC controls */
case V4L2_CID_MPEG_VIDEO_HEVC_I_FRAME_QP: return "HEVC I-Frame QP Value";
case V4L2_CID_MPEG_VIDEO_HEVC_P_FRAME_QP: return "HEVC P-Frame QP Value";
@@ -1180,6 +1191,7 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type,
case V4L2_CID_DEINTERLACING_MODE:
case V4L2_CID_TUNE_DEEMPHASIS:
case V4L2_CID_MPEG_VIDEO_VPX_GOLDEN_FRAME_SEL:
+ case V4L2_CID_MPEG_VIDEO_VP9_PROFILE:
case V4L2_CID_DETECT_MD_MODE:
case V4L2_CID_MPEG_VIDEO_HEVC_PROFILE:
case V4L2_CID_MPEG_VIDEO_HEVC_LEVEL:
diff --git a/include/uapi/linux/v4l2-controls.h b/include/uapi/linux/v4l2-controls.h
index 8d473c979b61..56203b7b715c 100644
--- a/include/uapi/linux/v4l2-controls.h
+++ b/include/uapi/linux/v4l2-controls.h
@@ -589,6 +589,14 @@ enum v4l2_vp8_golden_frame_sel {
#define V4L2_CID_MPEG_VIDEO_VPX_P_FRAME_QP (V4L2_CID_MPEG_BASE+510)
#define V4L2_CID_MPEG_VIDEO_VPX_PROFILE (V4L2_CID_MPEG_BASE+511)
+#define V4L2_CID_MPEG_VIDEO_VP9_PROFILE (V4L2_CID_MPEG_BASE+512)
+enum v4l2_mpeg_video_vp9_profile {
+ V4L2_MPEG_VIDEO_VP9_PROFILE_0 = 0,
+ V4L2_MPEG_VIDEO_VP9_PROFILE_1 = 1,
+ V4L2_MPEG_VIDEO_VP9_PROFILE_2 = 2,
+ V4L2_MPEG_VIDEO_VP9_PROFILE_3 = 3,
+};
+
/* CIDs for HEVC encoding. */
#define V4L2_CID_MPEG_VIDEO_HEVC_MIN_QP (V4L2_CID_MPEG_BASE + 600)
--
2.17.0.921.gf22659ad46-goog
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH v2 1/2] media: v4l2-ctrl: Add control for VP9 profile
@ 2018-05-30 7:16 ` Keiichi Watanabe
0 siblings, 0 replies; 25+ messages in thread
From: Keiichi Watanabe @ 2018-05-30 7:16 UTC (permalink / raw)
To: linux-arm-kernel
Add a new control V4L2_CID_MPEG_VIDEO_VP9_PROFILE for selecting desired
profile for VP9 encoder and querying for supported profiles by VP9 encoder
or decoder.
An existing control V4L2_CID_MPEG_VIDEO_VPX_PROFILE cannot be
used for querying since it is not a menu control but an integer
control, which cannot return an arbitrary set of supported profiles.
The new control V4L2_CID_MPEG_VIDEO_VP9_PROFILE is a menu control as
with controls for other codec profiles. (e.g. H264)
Signed-off-by: Keiichi Watanabe <keiichiw@chromium.org>
---
.../media/uapi/v4l/extended-controls.rst | 26 +++++++++++++++++++
drivers/media/v4l2-core/v4l2-ctrls.c | 12 +++++++++
include/uapi/linux/v4l2-controls.h | 8 ++++++
3 files changed, 46 insertions(+)
diff --git a/Documentation/media/uapi/v4l/extended-controls.rst b/Documentation/media/uapi/v4l/extended-controls.rst
index 03931f9b1285..4f7f128a4998 100644
--- a/Documentation/media/uapi/v4l/extended-controls.rst
+++ b/Documentation/media/uapi/v4l/extended-controls.rst
@@ -1959,6 +1959,32 @@ enum v4l2_vp8_golden_frame_sel -
Select the desired profile for VPx encoder. Acceptable values are 0,
1, 2 and 3 corresponding to encoder profiles 0, 1, 2 and 3.
+.. _v4l2-mpeg-video-vp9-profile:
+
+``V4L2_CID_MPEG_VIDEO_VP9_PROFILE``
+ (enum)
+
+enum v4l2_mpeg_video_vp9_profile -
+ This control allows to select the profile for VP9 encoder.
+ This is also used to enumerate supported profiles by VP9 encoder or decoder.
+ Possible values are:
+
+
+
+.. flat-table::
+ :header-rows: 0
+ :stub-columns: 0
+
+ * - ``V4L2_MPEG_VIDEO_VP9_PROFILE_0``
+ - Profile 0
+ * - ``V4L2_MPEG_VIDEO_VP9_PROFILE_1``
+ - Profile 1
+ * - ``V4L2_MPEG_VIDEO_VP9_PROFILE_2``
+ - Profile 2
+ * - ``V4L2_MPEG_VIDEO_VP9_PROFILE_3``
+ - Profile 3
+
+
High Efficiency Video Coding (HEVC/H.265) Control Reference
-----------------------------------------------------------
diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c b/drivers/media/v4l2-core/v4l2-ctrls.c
index d29e45516eb7..401ce21c2e63 100644
--- a/drivers/media/v4l2-core/v4l2-ctrls.c
+++ b/drivers/media/v4l2-core/v4l2-ctrls.c
@@ -431,6 +431,13 @@ const char * const *v4l2_ctrl_get_menu(u32 id)
"Use Previous Specific Frame",
NULL,
};
+ static const char * const vp9_profile[] = {
+ "0",
+ "1",
+ "2",
+ "3",
+ NULL,
+ };
static const char * const flash_led_mode[] = {
"Off",
@@ -614,6 +621,8 @@ const char * const *v4l2_ctrl_get_menu(u32 id)
return mpeg4_profile;
case V4L2_CID_MPEG_VIDEO_VPX_GOLDEN_FRAME_SEL:
return vpx_golden_frame_sel;
+ case V4L2_CID_MPEG_VIDEO_VP9_PROFILE:
+ return vp9_profile;
case V4L2_CID_JPEG_CHROMA_SUBSAMPLING:
return jpeg_chroma_subsampling;
case V4L2_CID_DV_TX_MODE:
@@ -841,6 +850,8 @@ const char *v4l2_ctrl_get_name(u32 id)
case V4L2_CID_MPEG_VIDEO_VPX_P_FRAME_QP: return "VPX P-Frame QP Value";
case V4L2_CID_MPEG_VIDEO_VPX_PROFILE: return "VPX Profile";
+ case V4L2_CID_MPEG_VIDEO_VP9_PROFILE: return "VP9 Profile";
+
/* HEVC controls */
case V4L2_CID_MPEG_VIDEO_HEVC_I_FRAME_QP: return "HEVC I-Frame QP Value";
case V4L2_CID_MPEG_VIDEO_HEVC_P_FRAME_QP: return "HEVC P-Frame QP Value";
@@ -1180,6 +1191,7 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type,
case V4L2_CID_DEINTERLACING_MODE:
case V4L2_CID_TUNE_DEEMPHASIS:
case V4L2_CID_MPEG_VIDEO_VPX_GOLDEN_FRAME_SEL:
+ case V4L2_CID_MPEG_VIDEO_VP9_PROFILE:
case V4L2_CID_DETECT_MD_MODE:
case V4L2_CID_MPEG_VIDEO_HEVC_PROFILE:
case V4L2_CID_MPEG_VIDEO_HEVC_LEVEL:
diff --git a/include/uapi/linux/v4l2-controls.h b/include/uapi/linux/v4l2-controls.h
index 8d473c979b61..56203b7b715c 100644
--- a/include/uapi/linux/v4l2-controls.h
+++ b/include/uapi/linux/v4l2-controls.h
@@ -589,6 +589,14 @@ enum v4l2_vp8_golden_frame_sel {
#define V4L2_CID_MPEG_VIDEO_VPX_P_FRAME_QP (V4L2_CID_MPEG_BASE+510)
#define V4L2_CID_MPEG_VIDEO_VPX_PROFILE (V4L2_CID_MPEG_BASE+511)
+#define V4L2_CID_MPEG_VIDEO_VP9_PROFILE (V4L2_CID_MPEG_BASE+512)
+enum v4l2_mpeg_video_vp9_profile {
+ V4L2_MPEG_VIDEO_VP9_PROFILE_0 = 0,
+ V4L2_MPEG_VIDEO_VP9_PROFILE_1 = 1,
+ V4L2_MPEG_VIDEO_VP9_PROFILE_2 = 2,
+ V4L2_MPEG_VIDEO_VP9_PROFILE_3 = 3,
+};
+
/* CIDs for HEVC encoding. */
#define V4L2_CID_MPEG_VIDEO_HEVC_MIN_QP (V4L2_CID_MPEG_BASE + 600)
--
2.17.0.921.gf22659ad46-goog
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH v2 2/2] media: mtk-vcodec: Support VP9 profile in decoder
2018-05-30 7:16 ` Keiichi Watanabe
@ 2018-05-30 7:16 ` Keiichi Watanabe
-1 siblings, 0 replies; 25+ messages in thread
From: Keiichi Watanabe @ 2018-05-30 7:16 UTC (permalink / raw)
To: linux-arm-kernel
Cc: Mauro Carvalho Chehab, Tiffany Lin, Andrew-CT Chen,
Matthias Brugger, Hans Verkuil, Sakari Ailus, Sylwester Nawrocki,
Smitha T Murthy, Keiichi Watanabe, Tom Saeger, Andy Shevchenko,
Tomasz Figa, Ricardo Ribalda Delgado, linux-media, linux-kernel,
linux-mediatek
Add V4L2_CID_MPEG_VIDEO_VP9_PROFILE control in MediaTek decoder's
driver.
MediaTek decoder only supports profile 0 for now.
Signed-off-by: Keiichi Watanabe <keiichiw@chromium.org>
---
drivers/media/platform/mtk-vcodec/mtk_vcodec_dec.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/media/platform/mtk-vcodec/mtk_vcodec_dec.c b/drivers/media/platform/mtk-vcodec/mtk_vcodec_dec.c
index 86f0a7134365..f9393504356d 100644
--- a/drivers/media/platform/mtk-vcodec/mtk_vcodec_dec.c
+++ b/drivers/media/platform/mtk-vcodec/mtk_vcodec_dec.c
@@ -1400,6 +1400,12 @@ int mtk_vcodec_dec_ctrls_setup(struct mtk_vcodec_ctx *ctx)
V4L2_CID_MIN_BUFFERS_FOR_CAPTURE,
0, 32, 1, 1);
ctrl->flags |= V4L2_CTRL_FLAG_VOLATILE;
+ v4l2_ctrl_new_std_menu(&ctx->ctrl_hdl,
+ &mtk_vcodec_dec_ctrl_ops,
+ V4L2_CID_MPEG_VIDEO_VP9_PROFILE,
+ V4L2_MPEG_VIDEO_VP9_PROFILE_3,
+ ~(1U << V4L2_MPEG_VIDEO_VP9_PROFILE_0),
+ V4L2_MPEG_VIDEO_VP9_PROFILE_0);
if (ctx->ctrl_hdl.error) {
mtk_v4l2_err("Adding control failed %d",
--
2.17.0.921.gf22659ad46-goog
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH v2 2/2] media: mtk-vcodec: Support VP9 profile in decoder
@ 2018-05-30 7:16 ` Keiichi Watanabe
0 siblings, 0 replies; 25+ messages in thread
From: Keiichi Watanabe @ 2018-05-30 7:16 UTC (permalink / raw)
To: linux-arm-kernel
Add V4L2_CID_MPEG_VIDEO_VP9_PROFILE control in MediaTek decoder's
driver.
MediaTek decoder only supports profile 0 for now.
Signed-off-by: Keiichi Watanabe <keiichiw@chromium.org>
---
drivers/media/platform/mtk-vcodec/mtk_vcodec_dec.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/media/platform/mtk-vcodec/mtk_vcodec_dec.c b/drivers/media/platform/mtk-vcodec/mtk_vcodec_dec.c
index 86f0a7134365..f9393504356d 100644
--- a/drivers/media/platform/mtk-vcodec/mtk_vcodec_dec.c
+++ b/drivers/media/platform/mtk-vcodec/mtk_vcodec_dec.c
@@ -1400,6 +1400,12 @@ int mtk_vcodec_dec_ctrls_setup(struct mtk_vcodec_ctx *ctx)
V4L2_CID_MIN_BUFFERS_FOR_CAPTURE,
0, 32, 1, 1);
ctrl->flags |= V4L2_CTRL_FLAG_VOLATILE;
+ v4l2_ctrl_new_std_menu(&ctx->ctrl_hdl,
+ &mtk_vcodec_dec_ctrl_ops,
+ V4L2_CID_MPEG_VIDEO_VP9_PROFILE,
+ V4L2_MPEG_VIDEO_VP9_PROFILE_3,
+ ~(1U << V4L2_MPEG_VIDEO_VP9_PROFILE_0),
+ V4L2_MPEG_VIDEO_VP9_PROFILE_0);
if (ctx->ctrl_hdl.error) {
mtk_v4l2_err("Adding control failed %d",
--
2.17.0.921.gf22659ad46-goog
^ permalink raw reply related [flat|nested] 25+ messages in thread
* Re: [PATCH v2 2/2] media: mtk-vcodec: Support VP9 profile in decoder
2018-05-30 7:16 ` Keiichi Watanabe
@ 2018-06-08 9:14 ` Hans Verkuil
-1 siblings, 0 replies; 25+ messages in thread
From: Hans Verkuil @ 2018-06-08 9:14 UTC (permalink / raw)
To: Keiichi Watanabe, linux-arm-kernel
Cc: Mauro Carvalho Chehab, Tiffany Lin, Andrew-CT Chen,
Matthias Brugger, Sakari Ailus, Sylwester Nawrocki,
Smitha T Murthy, Tom Saeger, Andy Shevchenko, Tomasz Figa,
Ricardo Ribalda Delgado, linux-media, linux-kernel,
linux-mediatek
On 05/30/2018 09:16 AM, Keiichi Watanabe wrote:
> Add V4L2_CID_MPEG_VIDEO_VP9_PROFILE control in MediaTek decoder's
> driver.
> MediaTek decoder only supports profile 0 for now.
>
> Signed-off-by: Keiichi Watanabe <keiichiw@chromium.org>
> ---
> drivers/media/platform/mtk-vcodec/mtk_vcodec_dec.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/drivers/media/platform/mtk-vcodec/mtk_vcodec_dec.c b/drivers/media/platform/mtk-vcodec/mtk_vcodec_dec.c
> index 86f0a7134365..f9393504356d 100644
> --- a/drivers/media/platform/mtk-vcodec/mtk_vcodec_dec.c
> +++ b/drivers/media/platform/mtk-vcodec/mtk_vcodec_dec.c
> @@ -1400,6 +1400,12 @@ int mtk_vcodec_dec_ctrls_setup(struct mtk_vcodec_ctx *ctx)
> V4L2_CID_MIN_BUFFERS_FOR_CAPTURE,
> 0, 32, 1, 1);
> ctrl->flags |= V4L2_CTRL_FLAG_VOLATILE;
> + v4l2_ctrl_new_std_menu(&ctx->ctrl_hdl,
> + &mtk_vcodec_dec_ctrl_ops,
> + V4L2_CID_MPEG_VIDEO_VP9_PROFILE,
> + V4L2_MPEG_VIDEO_VP9_PROFILE_3,
It makes no sense to set max to PROFILE_3 if PROFILE_0 is the only choice.
Just set max to PROFILE_0 as well.
> + ~(1U << V4L2_MPEG_VIDEO_VP9_PROFILE_0),
> + V4L2_MPEG_VIDEO_VP9_PROFILE_0);
>
> if (ctx->ctrl_hdl.error) {
> mtk_v4l2_err("Adding control failed %d",
> --
> 2.17.0.921.gf22659ad46-goog
>
Regards,
Hans
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH v2 2/2] media: mtk-vcodec: Support VP9 profile in decoder
@ 2018-06-08 9:14 ` Hans Verkuil
0 siblings, 0 replies; 25+ messages in thread
From: Hans Verkuil @ 2018-06-08 9:14 UTC (permalink / raw)
To: linux-arm-kernel
On 05/30/2018 09:16 AM, Keiichi Watanabe wrote:
> Add V4L2_CID_MPEG_VIDEO_VP9_PROFILE control in MediaTek decoder's
> driver.
> MediaTek decoder only supports profile 0 for now.
>
> Signed-off-by: Keiichi Watanabe <keiichiw@chromium.org>
> ---
> drivers/media/platform/mtk-vcodec/mtk_vcodec_dec.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/drivers/media/platform/mtk-vcodec/mtk_vcodec_dec.c b/drivers/media/platform/mtk-vcodec/mtk_vcodec_dec.c
> index 86f0a7134365..f9393504356d 100644
> --- a/drivers/media/platform/mtk-vcodec/mtk_vcodec_dec.c
> +++ b/drivers/media/platform/mtk-vcodec/mtk_vcodec_dec.c
> @@ -1400,6 +1400,12 @@ int mtk_vcodec_dec_ctrls_setup(struct mtk_vcodec_ctx *ctx)
> V4L2_CID_MIN_BUFFERS_FOR_CAPTURE,
> 0, 32, 1, 1);
> ctrl->flags |= V4L2_CTRL_FLAG_VOLATILE;
> + v4l2_ctrl_new_std_menu(&ctx->ctrl_hdl,
> + &mtk_vcodec_dec_ctrl_ops,
> + V4L2_CID_MPEG_VIDEO_VP9_PROFILE,
> + V4L2_MPEG_VIDEO_VP9_PROFILE_3,
It makes no sense to set max to PROFILE_3 if PROFILE_0 is the only choice.
Just set max to PROFILE_0 as well.
> + ~(1U << V4L2_MPEG_VIDEO_VP9_PROFILE_0),
> + V4L2_MPEG_VIDEO_VP9_PROFILE_0);
>
> if (ctx->ctrl_hdl.error) {
> mtk_v4l2_err("Adding control failed %d",
> --
> 2.17.0.921.gf22659ad46-goog
>
Regards,
Hans
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v2 1/2] media: v4l2-ctrl: Add control for VP9 profile
2018-05-30 7:16 ` Keiichi Watanabe
@ 2018-06-08 9:29 ` Hans Verkuil
-1 siblings, 0 replies; 25+ messages in thread
From: Hans Verkuil @ 2018-06-08 9:29 UTC (permalink / raw)
To: Keiichi Watanabe, linux-arm-kernel
Cc: Mauro Carvalho Chehab, Tiffany Lin, Andrew-CT Chen,
Matthias Brugger, Sakari Ailus, Sylwester Nawrocki,
Smitha T Murthy, Tom Saeger, Andy Shevchenko, Tomasz Figa,
Ricardo Ribalda Delgado, linux-media, linux-kernel,
linux-mediatek, Stanimir Varbanov
On 05/30/2018 09:16 AM, Keiichi Watanabe wrote:
> Add a new control V4L2_CID_MPEG_VIDEO_VP9_PROFILE for selecting desired
> profile for VP9 encoder and querying for supported profiles by VP9 encoder
> or decoder.
>
> An existing control V4L2_CID_MPEG_VIDEO_VPX_PROFILE cannot be
> used for querying since it is not a menu control but an integer
> control, which cannot return an arbitrary set of supported profiles.
>
> The new control V4L2_CID_MPEG_VIDEO_VP9_PROFILE is a menu control as
> with controls for other codec profiles. (e.g. H264)
Please ignore my reply to patch 2/2. I looked at this a bit more and what you
should do is to change the type of V4L2_CID_MPEG_VIDEO_VPX_PROFILE to enum.
All other codec profile controls are all enums, so the fact that VPX_PROFILE
isn't is a bug. Changing the type should not cause any problems since the same
control value is used when you set the control.
Sylwester: I see that s5p-mfc uses this control, but it is explicitly added
as an integer type control, so the s5p-mfc driver should not be affected by
changing the type of this control.
Stanimir: this will slightly change the venus driver, but since it is a very
recent driver I think we can get away with changing the core type of the
VPX_PROFILE control. I think that's better than ending up with two controls
that do the same thing.
Regards,
Hans
>
> Signed-off-by: Keiichi Watanabe <keiichiw@chromium.org>
> ---
> .../media/uapi/v4l/extended-controls.rst | 26 +++++++++++++++++++
> drivers/media/v4l2-core/v4l2-ctrls.c | 12 +++++++++
> include/uapi/linux/v4l2-controls.h | 8 ++++++
> 3 files changed, 46 insertions(+)
>
> diff --git a/Documentation/media/uapi/v4l/extended-controls.rst b/Documentation/media/uapi/v4l/extended-controls.rst
> index 03931f9b1285..4f7f128a4998 100644
> --- a/Documentation/media/uapi/v4l/extended-controls.rst
> +++ b/Documentation/media/uapi/v4l/extended-controls.rst
> @@ -1959,6 +1959,32 @@ enum v4l2_vp8_golden_frame_sel -
> Select the desired profile for VPx encoder. Acceptable values are 0,
> 1, 2 and 3 corresponding to encoder profiles 0, 1, 2 and 3.
>
> +.. _v4l2-mpeg-video-vp9-profile:
> +
> +``V4L2_CID_MPEG_VIDEO_VP9_PROFILE``
> + (enum)
> +
> +enum v4l2_mpeg_video_vp9_profile -
> + This control allows to select the profile for VP9 encoder.
> + This is also used to enumerate supported profiles by VP9 encoder or decoder.
> + Possible values are:
> +
> +
> +
> +.. flat-table::
> + :header-rows: 0
> + :stub-columns: 0
> +
> + * - ``V4L2_MPEG_VIDEO_VP9_PROFILE_0``
> + - Profile 0
> + * - ``V4L2_MPEG_VIDEO_VP9_PROFILE_1``
> + - Profile 1
> + * - ``V4L2_MPEG_VIDEO_VP9_PROFILE_2``
> + - Profile 2
> + * - ``V4L2_MPEG_VIDEO_VP9_PROFILE_3``
> + - Profile 3
> +
> +
>
> High Efficiency Video Coding (HEVC/H.265) Control Reference
> -----------------------------------------------------------
> diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c b/drivers/media/v4l2-core/v4l2-ctrls.c
> index d29e45516eb7..401ce21c2e63 100644
> --- a/drivers/media/v4l2-core/v4l2-ctrls.c
> +++ b/drivers/media/v4l2-core/v4l2-ctrls.c
> @@ -431,6 +431,13 @@ const char * const *v4l2_ctrl_get_menu(u32 id)
> "Use Previous Specific Frame",
> NULL,
> };
> + static const char * const vp9_profile[] = {
> + "0",
> + "1",
> + "2",
> + "3",
> + NULL,
> + };
>
> static const char * const flash_led_mode[] = {
> "Off",
> @@ -614,6 +621,8 @@ const char * const *v4l2_ctrl_get_menu(u32 id)
> return mpeg4_profile;
> case V4L2_CID_MPEG_VIDEO_VPX_GOLDEN_FRAME_SEL:
> return vpx_golden_frame_sel;
> + case V4L2_CID_MPEG_VIDEO_VP9_PROFILE:
> + return vp9_profile;
> case V4L2_CID_JPEG_CHROMA_SUBSAMPLING:
> return jpeg_chroma_subsampling;
> case V4L2_CID_DV_TX_MODE:
> @@ -841,6 +850,8 @@ const char *v4l2_ctrl_get_name(u32 id)
> case V4L2_CID_MPEG_VIDEO_VPX_P_FRAME_QP: return "VPX P-Frame QP Value";
> case V4L2_CID_MPEG_VIDEO_VPX_PROFILE: return "VPX Profile";
>
> + case V4L2_CID_MPEG_VIDEO_VP9_PROFILE: return "VP9 Profile";
> +
> /* HEVC controls */
> case V4L2_CID_MPEG_VIDEO_HEVC_I_FRAME_QP: return "HEVC I-Frame QP Value";
> case V4L2_CID_MPEG_VIDEO_HEVC_P_FRAME_QP: return "HEVC P-Frame QP Value";
> @@ -1180,6 +1191,7 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type,
> case V4L2_CID_DEINTERLACING_MODE:
> case V4L2_CID_TUNE_DEEMPHASIS:
> case V4L2_CID_MPEG_VIDEO_VPX_GOLDEN_FRAME_SEL:
> + case V4L2_CID_MPEG_VIDEO_VP9_PROFILE:
> case V4L2_CID_DETECT_MD_MODE:
> case V4L2_CID_MPEG_VIDEO_HEVC_PROFILE:
> case V4L2_CID_MPEG_VIDEO_HEVC_LEVEL:
> diff --git a/include/uapi/linux/v4l2-controls.h b/include/uapi/linux/v4l2-controls.h
> index 8d473c979b61..56203b7b715c 100644
> --- a/include/uapi/linux/v4l2-controls.h
> +++ b/include/uapi/linux/v4l2-controls.h
> @@ -589,6 +589,14 @@ enum v4l2_vp8_golden_frame_sel {
> #define V4L2_CID_MPEG_VIDEO_VPX_P_FRAME_QP (V4L2_CID_MPEG_BASE+510)
> #define V4L2_CID_MPEG_VIDEO_VPX_PROFILE (V4L2_CID_MPEG_BASE+511)
>
> +#define V4L2_CID_MPEG_VIDEO_VP9_PROFILE (V4L2_CID_MPEG_BASE+512)
> +enum v4l2_mpeg_video_vp9_profile {
> + V4L2_MPEG_VIDEO_VP9_PROFILE_0 = 0,
> + V4L2_MPEG_VIDEO_VP9_PROFILE_1 = 1,
> + V4L2_MPEG_VIDEO_VP9_PROFILE_2 = 2,
> + V4L2_MPEG_VIDEO_VP9_PROFILE_3 = 3,
> +};
> +
> /* CIDs for HEVC encoding. */
>
> #define V4L2_CID_MPEG_VIDEO_HEVC_MIN_QP (V4L2_CID_MPEG_BASE + 600)
> --
> 2.17.0.921.gf22659ad46-goog
>
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH v2 1/2] media: v4l2-ctrl: Add control for VP9 profile
@ 2018-06-08 9:29 ` Hans Verkuil
0 siblings, 0 replies; 25+ messages in thread
From: Hans Verkuil @ 2018-06-08 9:29 UTC (permalink / raw)
To: linux-arm-kernel
On 05/30/2018 09:16 AM, Keiichi Watanabe wrote:
> Add a new control V4L2_CID_MPEG_VIDEO_VP9_PROFILE for selecting desired
> profile for VP9 encoder and querying for supported profiles by VP9 encoder
> or decoder.
>
> An existing control V4L2_CID_MPEG_VIDEO_VPX_PROFILE cannot be
> used for querying since it is not a menu control but an integer
> control, which cannot return an arbitrary set of supported profiles.
>
> The new control V4L2_CID_MPEG_VIDEO_VP9_PROFILE is a menu control as
> with controls for other codec profiles. (e.g. H264)
Please ignore my reply to patch 2/2. I looked at this a bit more and what you
should do is to change the type of V4L2_CID_MPEG_VIDEO_VPX_PROFILE to enum.
All other codec profile controls are all enums, so the fact that VPX_PROFILE
isn't is a bug. Changing the type should not cause any problems since the same
control value is used when you set the control.
Sylwester: I see that s5p-mfc uses this control, but it is explicitly added
as an integer type control, so the s5p-mfc driver should not be affected by
changing the type of this control.
Stanimir: this will slightly change the venus driver, but since it is a very
recent driver I think we can get away with changing the core type of the
VPX_PROFILE control. I think that's better than ending up with two controls
that do the same thing.
Regards,
Hans
>
> Signed-off-by: Keiichi Watanabe <keiichiw@chromium.org>
> ---
> .../media/uapi/v4l/extended-controls.rst | 26 +++++++++++++++++++
> drivers/media/v4l2-core/v4l2-ctrls.c | 12 +++++++++
> include/uapi/linux/v4l2-controls.h | 8 ++++++
> 3 files changed, 46 insertions(+)
>
> diff --git a/Documentation/media/uapi/v4l/extended-controls.rst b/Documentation/media/uapi/v4l/extended-controls.rst
> index 03931f9b1285..4f7f128a4998 100644
> --- a/Documentation/media/uapi/v4l/extended-controls.rst
> +++ b/Documentation/media/uapi/v4l/extended-controls.rst
> @@ -1959,6 +1959,32 @@ enum v4l2_vp8_golden_frame_sel -
> Select the desired profile for VPx encoder. Acceptable values are 0,
> 1, 2 and 3 corresponding to encoder profiles 0, 1, 2 and 3.
>
> +.. _v4l2-mpeg-video-vp9-profile:
> +
> +``V4L2_CID_MPEG_VIDEO_VP9_PROFILE``
> + (enum)
> +
> +enum v4l2_mpeg_video_vp9_profile -
> + This control allows to select the profile for VP9 encoder.
> + This is also used to enumerate supported profiles by VP9 encoder or decoder.
> + Possible values are:
> +
> +
> +
> +.. flat-table::
> + :header-rows: 0
> + :stub-columns: 0
> +
> + * - ``V4L2_MPEG_VIDEO_VP9_PROFILE_0``
> + - Profile 0
> + * - ``V4L2_MPEG_VIDEO_VP9_PROFILE_1``
> + - Profile 1
> + * - ``V4L2_MPEG_VIDEO_VP9_PROFILE_2``
> + - Profile 2
> + * - ``V4L2_MPEG_VIDEO_VP9_PROFILE_3``
> + - Profile 3
> +
> +
>
> High Efficiency Video Coding (HEVC/H.265) Control Reference
> -----------------------------------------------------------
> diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c b/drivers/media/v4l2-core/v4l2-ctrls.c
> index d29e45516eb7..401ce21c2e63 100644
> --- a/drivers/media/v4l2-core/v4l2-ctrls.c
> +++ b/drivers/media/v4l2-core/v4l2-ctrls.c
> @@ -431,6 +431,13 @@ const char * const *v4l2_ctrl_get_menu(u32 id)
> "Use Previous Specific Frame",
> NULL,
> };
> + static const char * const vp9_profile[] = {
> + "0",
> + "1",
> + "2",
> + "3",
> + NULL,
> + };
>
> static const char * const flash_led_mode[] = {
> "Off",
> @@ -614,6 +621,8 @@ const char * const *v4l2_ctrl_get_menu(u32 id)
> return mpeg4_profile;
> case V4L2_CID_MPEG_VIDEO_VPX_GOLDEN_FRAME_SEL:
> return vpx_golden_frame_sel;
> + case V4L2_CID_MPEG_VIDEO_VP9_PROFILE:
> + return vp9_profile;
> case V4L2_CID_JPEG_CHROMA_SUBSAMPLING:
> return jpeg_chroma_subsampling;
> case V4L2_CID_DV_TX_MODE:
> @@ -841,6 +850,8 @@ const char *v4l2_ctrl_get_name(u32 id)
> case V4L2_CID_MPEG_VIDEO_VPX_P_FRAME_QP: return "VPX P-Frame QP Value";
> case V4L2_CID_MPEG_VIDEO_VPX_PROFILE: return "VPX Profile";
>
> + case V4L2_CID_MPEG_VIDEO_VP9_PROFILE: return "VP9 Profile";
> +
> /* HEVC controls */
> case V4L2_CID_MPEG_VIDEO_HEVC_I_FRAME_QP: return "HEVC I-Frame QP Value";
> case V4L2_CID_MPEG_VIDEO_HEVC_P_FRAME_QP: return "HEVC P-Frame QP Value";
> @@ -1180,6 +1191,7 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type,
> case V4L2_CID_DEINTERLACING_MODE:
> case V4L2_CID_TUNE_DEEMPHASIS:
> case V4L2_CID_MPEG_VIDEO_VPX_GOLDEN_FRAME_SEL:
> + case V4L2_CID_MPEG_VIDEO_VP9_PROFILE:
> case V4L2_CID_DETECT_MD_MODE:
> case V4L2_CID_MPEG_VIDEO_HEVC_PROFILE:
> case V4L2_CID_MPEG_VIDEO_HEVC_LEVEL:
> diff --git a/include/uapi/linux/v4l2-controls.h b/include/uapi/linux/v4l2-controls.h
> index 8d473c979b61..56203b7b715c 100644
> --- a/include/uapi/linux/v4l2-controls.h
> +++ b/include/uapi/linux/v4l2-controls.h
> @@ -589,6 +589,14 @@ enum v4l2_vp8_golden_frame_sel {
> #define V4L2_CID_MPEG_VIDEO_VPX_P_FRAME_QP (V4L2_CID_MPEG_BASE+510)
> #define V4L2_CID_MPEG_VIDEO_VPX_PROFILE (V4L2_CID_MPEG_BASE+511)
>
> +#define V4L2_CID_MPEG_VIDEO_VP9_PROFILE (V4L2_CID_MPEG_BASE+512)
> +enum v4l2_mpeg_video_vp9_profile {
> + V4L2_MPEG_VIDEO_VP9_PROFILE_0 = 0,
> + V4L2_MPEG_VIDEO_VP9_PROFILE_1 = 1,
> + V4L2_MPEG_VIDEO_VP9_PROFILE_2 = 2,
> + V4L2_MPEG_VIDEO_VP9_PROFILE_3 = 3,
> +};
> +
> /* CIDs for HEVC encoding. */
>
> #define V4L2_CID_MPEG_VIDEO_HEVC_MIN_QP (V4L2_CID_MPEG_BASE + 600)
> --
> 2.17.0.921.gf22659ad46-goog
>
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v2 1/2] media: v4l2-ctrl: Add control for VP9 profile
2018-06-08 9:29 ` Hans Verkuil
(?)
@ 2018-06-08 9:31 ` Tomasz Figa
-1 siblings, 0 replies; 25+ messages in thread
From: Tomasz Figa @ 2018-06-08 9:31 UTC (permalink / raw)
To: Hans Verkuil
Cc: keiichiw,
list@263.net:IOMMU DRIVERS
<iommu@lists.linux-foundation.org>,
Joerg Roedel <joro@8bytes.org>,,
Mauro Carvalho Chehab, Tiffany Lin (林慧珊),
Andrew-CT Chen (陳智迪),
Matthias Brugger, Sakari Ailus, Sylwester Nawrocki, smitha.t,
tom.saeger, andriy.shevchenko, Ricardo Ribalda Delgado,
Linux Media Mailing List, Linux Kernel Mailing List,
linux-mediatek, svarbanov
Hi Hans,
On Fri, Jun 8, 2018 at 6:29 PM Hans Verkuil <hverkuil@xs4all.nl> wrote:
>
> On 05/30/2018 09:16 AM, Keiichi Watanabe wrote:
> > Add a new control V4L2_CID_MPEG_VIDEO_VP9_PROFILE for selecting desired
> > profile for VP9 encoder and querying for supported profiles by VP9 encoder
> > or decoder.
> >
> > An existing control V4L2_CID_MPEG_VIDEO_VPX_PROFILE cannot be
> > used for querying since it is not a menu control but an integer
> > control, which cannot return an arbitrary set of supported profiles.
> >
> > The new control V4L2_CID_MPEG_VIDEO_VP9_PROFILE is a menu control as
> > with controls for other codec profiles. (e.g. H264)
>
> Please ignore my reply to patch 2/2. I looked at this a bit more and what you
> should do is to change the type of V4L2_CID_MPEG_VIDEO_VPX_PROFILE to enum.
Note that we still need a way to query VP8 and VP9 separately, since
the supported profiles will differ. (Most of hardware we have today
support all 4 profiles of VP8 and only profile 0 of VP9.)
Best regards,
Tomasz
>
> All other codec profile controls are all enums, so the fact that VPX_PROFILE
> isn't is a bug. Changing the type should not cause any problems since the same
> control value is used when you set the control.
>
> Sylwester: I see that s5p-mfc uses this control, but it is explicitly added
> as an integer type control, so the s5p-mfc driver should not be affected by
> changing the type of this control.
>
> Stanimir: this will slightly change the venus driver, but since it is a very
> recent driver I think we can get away with changing the core type of the
> VPX_PROFILE control. I think that's better than ending up with two controls
> that do the same thing.
>
> Regards,
>
> Hans
>
> >
> > Signed-off-by: Keiichi Watanabe <keiichiw@chromium.org>
> > ---
> > .../media/uapi/v4l/extended-controls.rst | 26 +++++++++++++++++++
> > drivers/media/v4l2-core/v4l2-ctrls.c | 12 +++++++++
> > include/uapi/linux/v4l2-controls.h | 8 ++++++
> > 3 files changed, 46 insertions(+)
> >
> > diff --git a/Documentation/media/uapi/v4l/extended-controls.rst b/Documentation/media/uapi/v4l/extended-controls.rst
> > index 03931f9b1285..4f7f128a4998 100644
> > --- a/Documentation/media/uapi/v4l/extended-controls.rst
> > +++ b/Documentation/media/uapi/v4l/extended-controls.rst
> > @@ -1959,6 +1959,32 @@ enum v4l2_vp8_golden_frame_sel -
> > Select the desired profile for VPx encoder. Acceptable values are 0,
> > 1, 2 and 3 corresponding to encoder profiles 0, 1, 2 and 3.
> >
> > +.. _v4l2-mpeg-video-vp9-profile:
> > +
> > +``V4L2_CID_MPEG_VIDEO_VP9_PROFILE``
> > + (enum)
> > +
> > +enum v4l2_mpeg_video_vp9_profile -
> > + This control allows to select the profile for VP9 encoder.
> > + This is also used to enumerate supported profiles by VP9 encoder or decoder.
> > + Possible values are:
> > +
> > +
> > +
> > +.. flat-table::
> > + :header-rows: 0
> > + :stub-columns: 0
> > +
> > + * - ``V4L2_MPEG_VIDEO_VP9_PROFILE_0``
> > + - Profile 0
> > + * - ``V4L2_MPEG_VIDEO_VP9_PROFILE_1``
> > + - Profile 1
> > + * - ``V4L2_MPEG_VIDEO_VP9_PROFILE_2``
> > + - Profile 2
> > + * - ``V4L2_MPEG_VIDEO_VP9_PROFILE_3``
> > + - Profile 3
> > +
> > +
> >
> > High Efficiency Video Coding (HEVC/H.265) Control Reference
> > -----------------------------------------------------------
> > diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c b/drivers/media/v4l2-core/v4l2-ctrls.c
> > index d29e45516eb7..401ce21c2e63 100644
> > --- a/drivers/media/v4l2-core/v4l2-ctrls.c
> > +++ b/drivers/media/v4l2-core/v4l2-ctrls.c
> > @@ -431,6 +431,13 @@ const char * const *v4l2_ctrl_get_menu(u32 id)
> > "Use Previous Specific Frame",
> > NULL,
> > };
> > + static const char * const vp9_profile[] = {
> > + "0",
> > + "1",
> > + "2",
> > + "3",
> > + NULL,
> > + };
> >
> > static const char * const flash_led_mode[] = {
> > "Off",
> > @@ -614,6 +621,8 @@ const char * const *v4l2_ctrl_get_menu(u32 id)
> > return mpeg4_profile;
> > case V4L2_CID_MPEG_VIDEO_VPX_GOLDEN_FRAME_SEL:
> > return vpx_golden_frame_sel;
> > + case V4L2_CID_MPEG_VIDEO_VP9_PROFILE:
> > + return vp9_profile;
> > case V4L2_CID_JPEG_CHROMA_SUBSAMPLING:
> > return jpeg_chroma_subsampling;
> > case V4L2_CID_DV_TX_MODE:
> > @@ -841,6 +850,8 @@ const char *v4l2_ctrl_get_name(u32 id)
> > case V4L2_CID_MPEG_VIDEO_VPX_P_FRAME_QP: return "VPX P-Frame QP Value";
> > case V4L2_CID_MPEG_VIDEO_VPX_PROFILE: return "VPX Profile";
> >
> > + case V4L2_CID_MPEG_VIDEO_VP9_PROFILE: return "VP9 Profile";
> > +
> > /* HEVC controls */
> > case V4L2_CID_MPEG_VIDEO_HEVC_I_FRAME_QP: return "HEVC I-Frame QP Value";
> > case V4L2_CID_MPEG_VIDEO_HEVC_P_FRAME_QP: return "HEVC P-Frame QP Value";
> > @@ -1180,6 +1191,7 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type,
> > case V4L2_CID_DEINTERLACING_MODE:
> > case V4L2_CID_TUNE_DEEMPHASIS:
> > case V4L2_CID_MPEG_VIDEO_VPX_GOLDEN_FRAME_SEL:
> > + case V4L2_CID_MPEG_VIDEO_VP9_PROFILE:
> > case V4L2_CID_DETECT_MD_MODE:
> > case V4L2_CID_MPEG_VIDEO_HEVC_PROFILE:
> > case V4L2_CID_MPEG_VIDEO_HEVC_LEVEL:
> > diff --git a/include/uapi/linux/v4l2-controls.h b/include/uapi/linux/v4l2-controls.h
> > index 8d473c979b61..56203b7b715c 100644
> > --- a/include/uapi/linux/v4l2-controls.h
> > +++ b/include/uapi/linux/v4l2-controls.h
> > @@ -589,6 +589,14 @@ enum v4l2_vp8_golden_frame_sel {
> > #define V4L2_CID_MPEG_VIDEO_VPX_P_FRAME_QP (V4L2_CID_MPEG_BASE+510)
> > #define V4L2_CID_MPEG_VIDEO_VPX_PROFILE (V4L2_CID_MPEG_BASE+511)
> >
> > +#define V4L2_CID_MPEG_VIDEO_VP9_PROFILE (V4L2_CID_MPEG_BASE+512)
> > +enum v4l2_mpeg_video_vp9_profile {
> > + V4L2_MPEG_VIDEO_VP9_PROFILE_0 = 0,
> > + V4L2_MPEG_VIDEO_VP9_PROFILE_1 = 1,
> > + V4L2_MPEG_VIDEO_VP9_PROFILE_2 = 2,
> > + V4L2_MPEG_VIDEO_VP9_PROFILE_3 = 3,
> > +};
> > +
> > /* CIDs for HEVC encoding. */
> >
> > #define V4L2_CID_MPEG_VIDEO_HEVC_MIN_QP (V4L2_CID_MPEG_BASE + 600)
> > --
> > 2.17.0.921.gf22659ad46-goog
> >
>
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v2 1/2] media: v4l2-ctrl: Add control for VP9 profile
@ 2018-06-08 9:31 ` Tomasz Figa
0 siblings, 0 replies; 25+ messages in thread
From: Tomasz Figa @ 2018-06-08 9:31 UTC (permalink / raw)
To: Hans Verkuil
Cc: keiichiw,
list@263.net:IOMMU DRIVERS
<iommu@lists.linux-foundation.org>,
Joerg Roedel <joro@8bytes.org>,,
Mauro Carvalho Chehab, Tiffany Lin (林慧珊),
Andrew-CT Chen (陳智迪),
Matthias Brugger, Sakari Ailus, Sylwester Nawrocki, smitha.t,
tom.saeger, andriy.shevchenko, Ricardo Ribalda Delgado,
Linux Media Mailing List, Linux Kernel Mailing List,
linux-mediatek
Hi Hans,
On Fri, Jun 8, 2018 at 6:29 PM Hans Verkuil <hverkuil@xs4all.nl> wrote:
>
> On 05/30/2018 09:16 AM, Keiichi Watanabe wrote:
> > Add a new control V4L2_CID_MPEG_VIDEO_VP9_PROFILE for selecting desired
> > profile for VP9 encoder and querying for supported profiles by VP9 encoder
> > or decoder.
> >
> > An existing control V4L2_CID_MPEG_VIDEO_VPX_PROFILE cannot be
> > used for querying since it is not a menu control but an integer
> > control, which cannot return an arbitrary set of supported profiles.
> >
> > The new control V4L2_CID_MPEG_VIDEO_VP9_PROFILE is a menu control as
> > with controls for other codec profiles. (e.g. H264)
>
> Please ignore my reply to patch 2/2. I looked at this a bit more and what you
> should do is to change the type of V4L2_CID_MPEG_VIDEO_VPX_PROFILE to enum.
Note that we still need a way to query VP8 and VP9 separately, since
the supported profiles will differ. (Most of hardware we have today
support all 4 profiles of VP8 and only profile 0 of VP9.)
Best regards,
Tomasz
>
> All other codec profile controls are all enums, so the fact that VPX_PROFILE
> isn't is a bug. Changing the type should not cause any problems since the same
> control value is used when you set the control.
>
> Sylwester: I see that s5p-mfc uses this control, but it is explicitly added
> as an integer type control, so the s5p-mfc driver should not be affected by
> changing the type of this control.
>
> Stanimir: this will slightly change the venus driver, but since it is a very
> recent driver I think we can get away with changing the core type of the
> VPX_PROFILE control. I think that's better than ending up with two controls
> that do the same thing.
>
> Regards,
>
> Hans
>
> >
> > Signed-off-by: Keiichi Watanabe <keiichiw@chromium.org>
> > ---
> > .../media/uapi/v4l/extended-controls.rst | 26 +++++++++++++++++++
> > drivers/media/v4l2-core/v4l2-ctrls.c | 12 +++++++++
> > include/uapi/linux/v4l2-controls.h | 8 ++++++
> > 3 files changed, 46 insertions(+)
> >
> > diff --git a/Documentation/media/uapi/v4l/extended-controls.rst b/Documentation/media/uapi/v4l/extended-controls.rst
> > index 03931f9b1285..4f7f128a4998 100644
> > --- a/Documentation/media/uapi/v4l/extended-controls.rst
> > +++ b/Documentation/media/uapi/v4l/extended-controls.rst
> > @@ -1959,6 +1959,32 @@ enum v4l2_vp8_golden_frame_sel -
> > Select the desired profile for VPx encoder. Acceptable values are 0,
> > 1, 2 and 3 corresponding to encoder profiles 0, 1, 2 and 3.
> >
> > +.. _v4l2-mpeg-video-vp9-profile:
> > +
> > +``V4L2_CID_MPEG_VIDEO_VP9_PROFILE``
> > + (enum)
> > +
> > +enum v4l2_mpeg_video_vp9_profile -
> > + This control allows to select the profile for VP9 encoder.
> > + This is also used to enumerate supported profiles by VP9 encoder or decoder.
> > + Possible values are:
> > +
> > +
> > +
> > +.. flat-table::
> > + :header-rows: 0
> > + :stub-columns: 0
> > +
> > + * - ``V4L2_MPEG_VIDEO_VP9_PROFILE_0``
> > + - Profile 0
> > + * - ``V4L2_MPEG_VIDEO_VP9_PROFILE_1``
> > + - Profile 1
> > + * - ``V4L2_MPEG_VIDEO_VP9_PROFILE_2``
> > + - Profile 2
> > + * - ``V4L2_MPEG_VIDEO_VP9_PROFILE_3``
> > + - Profile 3
> > +
> > +
> >
> > High Efficiency Video Coding (HEVC/H.265) Control Reference
> > -----------------------------------------------------------
> > diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c b/drivers/media/v4l2-core/v4l2-ctrls.c
> > index d29e45516eb7..401ce21c2e63 100644
> > --- a/drivers/media/v4l2-core/v4l2-ctrls.c
> > +++ b/drivers/media/v4l2-core/v4l2-ctrls.c
> > @@ -431,6 +431,13 @@ const char * const *v4l2_ctrl_get_menu(u32 id)
> > "Use Previous Specific Frame",
> > NULL,
> > };
> > + static const char * const vp9_profile[] = {
> > + "0",
> > + "1",
> > + "2",
> > + "3",
> > + NULL,
> > + };
> >
> > static const char * const flash_led_mode[] = {
> > "Off",
> > @@ -614,6 +621,8 @@ const char * const *v4l2_ctrl_get_menu(u32 id)
> > return mpeg4_profile;
> > case V4L2_CID_MPEG_VIDEO_VPX_GOLDEN_FRAME_SEL:
> > return vpx_golden_frame_sel;
> > + case V4L2_CID_MPEG_VIDEO_VP9_PROFILE:
> > + return vp9_profile;
> > case V4L2_CID_JPEG_CHROMA_SUBSAMPLING:
> > return jpeg_chroma_subsampling;
> > case V4L2_CID_DV_TX_MODE:
> > @@ -841,6 +850,8 @@ const char *v4l2_ctrl_get_name(u32 id)
> > case V4L2_CID_MPEG_VIDEO_VPX_P_FRAME_QP: return "VPX P-Frame QP Value";
> > case V4L2_CID_MPEG_VIDEO_VPX_PROFILE: return "VPX Profile";
> >
> > + case V4L2_CID_MPEG_VIDEO_VP9_PROFILE: return "VP9 Profile";
> > +
> > /* HEVC controls */
> > case V4L2_CID_MPEG_VIDEO_HEVC_I_FRAME_QP: return "HEVC I-Frame QP Value";
> > case V4L2_CID_MPEG_VIDEO_HEVC_P_FRAME_QP: return "HEVC P-Frame QP Value";
> > @@ -1180,6 +1191,7 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type,
> > case V4L2_CID_DEINTERLACING_MODE:
> > case V4L2_CID_TUNE_DEEMPHASIS:
> > case V4L2_CID_MPEG_VIDEO_VPX_GOLDEN_FRAME_SEL:
> > + case V4L2_CID_MPEG_VIDEO_VP9_PROFILE:
> > case V4L2_CID_DETECT_MD_MODE:
> > case V4L2_CID_MPEG_VIDEO_HEVC_PROFILE:
> > case V4L2_CID_MPEG_VIDEO_HEVC_LEVEL:
> > diff --git a/include/uapi/linux/v4l2-controls.h b/include/uapi/linux/v4l2-controls.h
> > index 8d473c979b61..56203b7b715c 100644
> > --- a/include/uapi/linux/v4l2-controls.h
> > +++ b/include/uapi/linux/v4l2-controls.h
> > @@ -589,6 +589,14 @@ enum v4l2_vp8_golden_frame_sel {
> > #define V4L2_CID_MPEG_VIDEO_VPX_P_FRAME_QP (V4L2_CID_MPEG_BASE+510)
> > #define V4L2_CID_MPEG_VIDEO_VPX_PROFILE (V4L2_CID_MPEG_BASE+511)
> >
> > +#define V4L2_CID_MPEG_VIDEO_VP9_PROFILE (V4L2_CID_MPEG_BASE+512)
> > +enum v4l2_mpeg_video_vp9_profile {
> > + V4L2_MPEG_VIDEO_VP9_PROFILE_0 = 0,
> > + V4L2_MPEG_VIDEO_VP9_PROFILE_1 = 1,
> > + V4L2_MPEG_VIDEO_VP9_PROFILE_2 = 2,
> > + V4L2_MPEG_VIDEO_VP9_PROFILE_3 = 3,
> > +};
> > +
> > /* CIDs for HEVC encoding. */
> >
> > #define V4L2_CID_MPEG_VIDEO_HEVC_MIN_QP (V4L2_CID_MPEG_BASE + 600)
> > --
> > 2.17.0.921.gf22659ad46-goog
> >
>
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH v2 1/2] media: v4l2-ctrl: Add control for VP9 profile
@ 2018-06-08 9:31 ` Tomasz Figa
0 siblings, 0 replies; 25+ messages in thread
From: Tomasz Figa @ 2018-06-08 9:31 UTC (permalink / raw)
To: linux-arm-kernel
Hi Hans,
On Fri, Jun 8, 2018 at 6:29 PM Hans Verkuil <hverkuil@xs4all.nl> wrote:
>
> On 05/30/2018 09:16 AM, Keiichi Watanabe wrote:
> > Add a new control V4L2_CID_MPEG_VIDEO_VP9_PROFILE for selecting desired
> > profile for VP9 encoder and querying for supported profiles by VP9 encoder
> > or decoder.
> >
> > An existing control V4L2_CID_MPEG_VIDEO_VPX_PROFILE cannot be
> > used for querying since it is not a menu control but an integer
> > control, which cannot return an arbitrary set of supported profiles.
> >
> > The new control V4L2_CID_MPEG_VIDEO_VP9_PROFILE is a menu control as
> > with controls for other codec profiles. (e.g. H264)
>
> Please ignore my reply to patch 2/2. I looked at this a bit more and what you
> should do is to change the type of V4L2_CID_MPEG_VIDEO_VPX_PROFILE to enum.
Note that we still need a way to query VP8 and VP9 separately, since
the supported profiles will differ. (Most of hardware we have today
support all 4 profiles of VP8 and only profile 0 of VP9.)
Best regards,
Tomasz
>
> All other codec profile controls are all enums, so the fact that VPX_PROFILE
> isn't is a bug. Changing the type should not cause any problems since the same
> control value is used when you set the control.
>
> Sylwester: I see that s5p-mfc uses this control, but it is explicitly added
> as an integer type control, so the s5p-mfc driver should not be affected by
> changing the type of this control.
>
> Stanimir: this will slightly change the venus driver, but since it is a very
> recent driver I think we can get away with changing the core type of the
> VPX_PROFILE control. I think that's better than ending up with two controls
> that do the same thing.
>
> Regards,
>
> Hans
>
> >
> > Signed-off-by: Keiichi Watanabe <keiichiw@chromium.org>
> > ---
> > .../media/uapi/v4l/extended-controls.rst | 26 +++++++++++++++++++
> > drivers/media/v4l2-core/v4l2-ctrls.c | 12 +++++++++
> > include/uapi/linux/v4l2-controls.h | 8 ++++++
> > 3 files changed, 46 insertions(+)
> >
> > diff --git a/Documentation/media/uapi/v4l/extended-controls.rst b/Documentation/media/uapi/v4l/extended-controls.rst
> > index 03931f9b1285..4f7f128a4998 100644
> > --- a/Documentation/media/uapi/v4l/extended-controls.rst
> > +++ b/Documentation/media/uapi/v4l/extended-controls.rst
> > @@ -1959,6 +1959,32 @@ enum v4l2_vp8_golden_frame_sel -
> > Select the desired profile for VPx encoder. Acceptable values are 0,
> > 1, 2 and 3 corresponding to encoder profiles 0, 1, 2 and 3.
> >
> > +.. _v4l2-mpeg-video-vp9-profile:
> > +
> > +``V4L2_CID_MPEG_VIDEO_VP9_PROFILE``
> > + (enum)
> > +
> > +enum v4l2_mpeg_video_vp9_profile -
> > + This control allows to select the profile for VP9 encoder.
> > + This is also used to enumerate supported profiles by VP9 encoder or decoder.
> > + Possible values are:
> > +
> > +
> > +
> > +.. flat-table::
> > + :header-rows: 0
> > + :stub-columns: 0
> > +
> > + * - ``V4L2_MPEG_VIDEO_VP9_PROFILE_0``
> > + - Profile 0
> > + * - ``V4L2_MPEG_VIDEO_VP9_PROFILE_1``
> > + - Profile 1
> > + * - ``V4L2_MPEG_VIDEO_VP9_PROFILE_2``
> > + - Profile 2
> > + * - ``V4L2_MPEG_VIDEO_VP9_PROFILE_3``
> > + - Profile 3
> > +
> > +
> >
> > High Efficiency Video Coding (HEVC/H.265) Control Reference
> > -----------------------------------------------------------
> > diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c b/drivers/media/v4l2-core/v4l2-ctrls.c
> > index d29e45516eb7..401ce21c2e63 100644
> > --- a/drivers/media/v4l2-core/v4l2-ctrls.c
> > +++ b/drivers/media/v4l2-core/v4l2-ctrls.c
> > @@ -431,6 +431,13 @@ const char * const *v4l2_ctrl_get_menu(u32 id)
> > "Use Previous Specific Frame",
> > NULL,
> > };
> > + static const char * const vp9_profile[] = {
> > + "0",
> > + "1",
> > + "2",
> > + "3",
> > + NULL,
> > + };
> >
> > static const char * const flash_led_mode[] = {
> > "Off",
> > @@ -614,6 +621,8 @@ const char * const *v4l2_ctrl_get_menu(u32 id)
> > return mpeg4_profile;
> > case V4L2_CID_MPEG_VIDEO_VPX_GOLDEN_FRAME_SEL:
> > return vpx_golden_frame_sel;
> > + case V4L2_CID_MPEG_VIDEO_VP9_PROFILE:
> > + return vp9_profile;
> > case V4L2_CID_JPEG_CHROMA_SUBSAMPLING:
> > return jpeg_chroma_subsampling;
> > case V4L2_CID_DV_TX_MODE:
> > @@ -841,6 +850,8 @@ const char *v4l2_ctrl_get_name(u32 id)
> > case V4L2_CID_MPEG_VIDEO_VPX_P_FRAME_QP: return "VPX P-Frame QP Value";
> > case V4L2_CID_MPEG_VIDEO_VPX_PROFILE: return "VPX Profile";
> >
> > + case V4L2_CID_MPEG_VIDEO_VP9_PROFILE: return "VP9 Profile";
> > +
> > /* HEVC controls */
> > case V4L2_CID_MPEG_VIDEO_HEVC_I_FRAME_QP: return "HEVC I-Frame QP Value";
> > case V4L2_CID_MPEG_VIDEO_HEVC_P_FRAME_QP: return "HEVC P-Frame QP Value";
> > @@ -1180,6 +1191,7 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type,
> > case V4L2_CID_DEINTERLACING_MODE:
> > case V4L2_CID_TUNE_DEEMPHASIS:
> > case V4L2_CID_MPEG_VIDEO_VPX_GOLDEN_FRAME_SEL:
> > + case V4L2_CID_MPEG_VIDEO_VP9_PROFILE:
> > case V4L2_CID_DETECT_MD_MODE:
> > case V4L2_CID_MPEG_VIDEO_HEVC_PROFILE:
> > case V4L2_CID_MPEG_VIDEO_HEVC_LEVEL:
> > diff --git a/include/uapi/linux/v4l2-controls.h b/include/uapi/linux/v4l2-controls.h
> > index 8d473c979b61..56203b7b715c 100644
> > --- a/include/uapi/linux/v4l2-controls.h
> > +++ b/include/uapi/linux/v4l2-controls.h
> > @@ -589,6 +589,14 @@ enum v4l2_vp8_golden_frame_sel {
> > #define V4L2_CID_MPEG_VIDEO_VPX_P_FRAME_QP (V4L2_CID_MPEG_BASE+510)
> > #define V4L2_CID_MPEG_VIDEO_VPX_PROFILE (V4L2_CID_MPEG_BASE+511)
> >
> > +#define V4L2_CID_MPEG_VIDEO_VP9_PROFILE (V4L2_CID_MPEG_BASE+512)
> > +enum v4l2_mpeg_video_vp9_profile {
> > + V4L2_MPEG_VIDEO_VP9_PROFILE_0 = 0,
> > + V4L2_MPEG_VIDEO_VP9_PROFILE_1 = 1,
> > + V4L2_MPEG_VIDEO_VP9_PROFILE_2 = 2,
> > + V4L2_MPEG_VIDEO_VP9_PROFILE_3 = 3,
> > +};
> > +
> > /* CIDs for HEVC encoding. */
> >
> > #define V4L2_CID_MPEG_VIDEO_HEVC_MIN_QP (V4L2_CID_MPEG_BASE + 600)
> > --
> > 2.17.0.921.gf22659ad46-goog
> >
>
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v2 1/2] media: v4l2-ctrl: Add control for VP9 profile
2018-06-08 9:31 ` Tomasz Figa
(?)
@ 2018-06-08 9:40 ` Hans Verkuil
-1 siblings, 0 replies; 25+ messages in thread
From: Hans Verkuil @ 2018-06-08 9:40 UTC (permalink / raw)
To: Tomasz Figa
Cc: keiichiw, list@263.net:IOMMU DRIVERS, Joerg Roedel,
linux-arm-kernel, Mauro Carvalho Chehab,
Tiffany Lin (林慧珊),
Andrew-CT Chen (陳智迪),
Matthias Brugger, Sakari Ailus, Sylwester Nawrocki, smitha.t,
tom.saeger, andriy.shevchenko, Ricardo Ribalda Delgado,
Linux Media Mailing List, Linux Kernel Mailing List,
linux-mediatek, svarbanov
On 06/08/2018 11:31 AM, Tomasz Figa wrote:
> Hi Hans,
>
> On Fri, Jun 8, 2018 at 6:29 PM Hans Verkuil <hverkuil@xs4all.nl> wrote:
>>
>> On 05/30/2018 09:16 AM, Keiichi Watanabe wrote:
>>> Add a new control V4L2_CID_MPEG_VIDEO_VP9_PROFILE for selecting desired
>>> profile for VP9 encoder and querying for supported profiles by VP9 encoder
>>> or decoder.
>>>
>>> An existing control V4L2_CID_MPEG_VIDEO_VPX_PROFILE cannot be
>>> used for querying since it is not a menu control but an integer
>>> control, which cannot return an arbitrary set of supported profiles.
>>>
>>> The new control V4L2_CID_MPEG_VIDEO_VP9_PROFILE is a menu control as
>>> with controls for other codec profiles. (e.g. H264)
>>
>> Please ignore my reply to patch 2/2. I looked at this a bit more and what you
>> should do is to change the type of V4L2_CID_MPEG_VIDEO_VPX_PROFILE to enum.
>
> Note that we still need a way to query VP8 and VP9 separately, since
> the supported profiles will differ. (Most of hardware we have today
> support all 4 profiles of VP8 and only profile 0 of VP9.)
Urgh. So V4L2_CID_MPEG_VIDEO_VPX_PROFILE is really just for VP8?
In that case I would suggest that we rename V4L2_CID_MPEG_VIDEO_VPX_PROFILE to
V4L2_CID_MPEG_VIDEO_VP8_PROFILE and change it to an enum. Also add this line
to v4l2-controls.h:
#define V4L2_CID_MPEG_VIDEO_VPX_PROFILE V4L2_CID_MPEG_VIDEO_VP8_PROFILE
And add a new V4L2_CID_MPEG_VIDEO_VP9_PROFILE (basically this patch).
Would that work?
This standardizes on enums for all profile controls (except s5p-mfc, which
makes its own int control, up to samsung to decide whether or not to change
that), and you have VP8 and VP9 specific profiles.
Regards,
Hans
>
> Best regards,
> Tomasz
>
>>
>> All other codec profile controls are all enums, so the fact that VPX_PROFILE
>> isn't is a bug. Changing the type should not cause any problems since the same
>> control value is used when you set the control.
>>
>> Sylwester: I see that s5p-mfc uses this control, but it is explicitly added
>> as an integer type control, so the s5p-mfc driver should not be affected by
>> changing the type of this control.
>>
>> Stanimir: this will slightly change the venus driver, but since it is a very
>> recent driver I think we can get away with changing the core type of the
>> VPX_PROFILE control. I think that's better than ending up with two controls
>> that do the same thing.
>>
>> Regards,
>>
>> Hans
>>
>>>
>>> Signed-off-by: Keiichi Watanabe <keiichiw@chromium.org>
>>> ---
>>> .../media/uapi/v4l/extended-controls.rst | 26 +++++++++++++++++++
>>> drivers/media/v4l2-core/v4l2-ctrls.c | 12 +++++++++
>>> include/uapi/linux/v4l2-controls.h | 8 ++++++
>>> 3 files changed, 46 insertions(+)
>>>
>>> diff --git a/Documentation/media/uapi/v4l/extended-controls.rst b/Documentation/media/uapi/v4l/extended-controls.rst
>>> index 03931f9b1285..4f7f128a4998 100644
>>> --- a/Documentation/media/uapi/v4l/extended-controls.rst
>>> +++ b/Documentation/media/uapi/v4l/extended-controls.rst
>>> @@ -1959,6 +1959,32 @@ enum v4l2_vp8_golden_frame_sel -
>>> Select the desired profile for VPx encoder. Acceptable values are 0,
>>> 1, 2 and 3 corresponding to encoder profiles 0, 1, 2 and 3.
>>>
>>> +.. _v4l2-mpeg-video-vp9-profile:
>>> +
>>> +``V4L2_CID_MPEG_VIDEO_VP9_PROFILE``
>>> + (enum)
>>> +
>>> +enum v4l2_mpeg_video_vp9_profile -
>>> + This control allows to select the profile for VP9 encoder.
>>> + This is also used to enumerate supported profiles by VP9 encoder or decoder.
>>> + Possible values are:
>>> +
>>> +
>>> +
>>> +.. flat-table::
>>> + :header-rows: 0
>>> + :stub-columns: 0
>>> +
>>> + * - ``V4L2_MPEG_VIDEO_VP9_PROFILE_0``
>>> + - Profile 0
>>> + * - ``V4L2_MPEG_VIDEO_VP9_PROFILE_1``
>>> + - Profile 1
>>> + * - ``V4L2_MPEG_VIDEO_VP9_PROFILE_2``
>>> + - Profile 2
>>> + * - ``V4L2_MPEG_VIDEO_VP9_PROFILE_3``
>>> + - Profile 3
>>> +
>>> +
>>>
>>> High Efficiency Video Coding (HEVC/H.265) Control Reference
>>> -----------------------------------------------------------
>>> diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c b/drivers/media/v4l2-core/v4l2-ctrls.c
>>> index d29e45516eb7..401ce21c2e63 100644
>>> --- a/drivers/media/v4l2-core/v4l2-ctrls.c
>>> +++ b/drivers/media/v4l2-core/v4l2-ctrls.c
>>> @@ -431,6 +431,13 @@ const char * const *v4l2_ctrl_get_menu(u32 id)
>>> "Use Previous Specific Frame",
>>> NULL,
>>> };
>>> + static const char * const vp9_profile[] = {
>>> + "0",
>>> + "1",
>>> + "2",
>>> + "3",
>>> + NULL,
>>> + };
>>>
>>> static const char * const flash_led_mode[] = {
>>> "Off",
>>> @@ -614,6 +621,8 @@ const char * const *v4l2_ctrl_get_menu(u32 id)
>>> return mpeg4_profile;
>>> case V4L2_CID_MPEG_VIDEO_VPX_GOLDEN_FRAME_SEL:
>>> return vpx_golden_frame_sel;
>>> + case V4L2_CID_MPEG_VIDEO_VP9_PROFILE:
>>> + return vp9_profile;
>>> case V4L2_CID_JPEG_CHROMA_SUBSAMPLING:
>>> return jpeg_chroma_subsampling;
>>> case V4L2_CID_DV_TX_MODE:
>>> @@ -841,6 +850,8 @@ const char *v4l2_ctrl_get_name(u32 id)
>>> case V4L2_CID_MPEG_VIDEO_VPX_P_FRAME_QP: return "VPX P-Frame QP Value";
>>> case V4L2_CID_MPEG_VIDEO_VPX_PROFILE: return "VPX Profile";
>>>
>>> + case V4L2_CID_MPEG_VIDEO_VP9_PROFILE: return "VP9 Profile";
>>> +
>>> /* HEVC controls */
>>> case V4L2_CID_MPEG_VIDEO_HEVC_I_FRAME_QP: return "HEVC I-Frame QP Value";
>>> case V4L2_CID_MPEG_VIDEO_HEVC_P_FRAME_QP: return "HEVC P-Frame QP Value";
>>> @@ -1180,6 +1191,7 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type,
>>> case V4L2_CID_DEINTERLACING_MODE:
>>> case V4L2_CID_TUNE_DEEMPHASIS:
>>> case V4L2_CID_MPEG_VIDEO_VPX_GOLDEN_FRAME_SEL:
>>> + case V4L2_CID_MPEG_VIDEO_VP9_PROFILE:
>>> case V4L2_CID_DETECT_MD_MODE:
>>> case V4L2_CID_MPEG_VIDEO_HEVC_PROFILE:
>>> case V4L2_CID_MPEG_VIDEO_HEVC_LEVEL:
>>> diff --git a/include/uapi/linux/v4l2-controls.h b/include/uapi/linux/v4l2-controls.h
>>> index 8d473c979b61..56203b7b715c 100644
>>> --- a/include/uapi/linux/v4l2-controls.h
>>> +++ b/include/uapi/linux/v4l2-controls.h
>>> @@ -589,6 +589,14 @@ enum v4l2_vp8_golden_frame_sel {
>>> #define V4L2_CID_MPEG_VIDEO_VPX_P_FRAME_QP (V4L2_CID_MPEG_BASE+510)
>>> #define V4L2_CID_MPEG_VIDEO_VPX_PROFILE (V4L2_CID_MPEG_BASE+511)
>>>
>>> +#define V4L2_CID_MPEG_VIDEO_VP9_PROFILE (V4L2_CID_MPEG_BASE+512)
>>> +enum v4l2_mpeg_video_vp9_profile {
>>> + V4L2_MPEG_VIDEO_VP9_PROFILE_0 = 0,
>>> + V4L2_MPEG_VIDEO_VP9_PROFILE_1 = 1,
>>> + V4L2_MPEG_VIDEO_VP9_PROFILE_2 = 2,
>>> + V4L2_MPEG_VIDEO_VP9_PROFILE_3 = 3,
>>> +};
>>> +
>>> /* CIDs for HEVC encoding. */
>>>
>>> #define V4L2_CID_MPEG_VIDEO_HEVC_MIN_QP (V4L2_CID_MPEG_BASE + 600)
>>> --
>>> 2.17.0.921.gf22659ad46-goog
>>>
>>
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v2 1/2] media: v4l2-ctrl: Add control for VP9 profile
@ 2018-06-08 9:40 ` Hans Verkuil
0 siblings, 0 replies; 25+ messages in thread
From: Hans Verkuil @ 2018-06-08 9:40 UTC (permalink / raw)
To: Tomasz Figa
Cc: keiichiw, list@263.net:IOMMU DRIVERS, Joerg Roedel,
linux-arm-kernel, Mauro Carvalho Chehab,
Tiffany Lin (林慧珊),
Andrew-CT Chen (陳智迪),
Matthias Brugger, Sakari Ailus, Sylwester Nawrocki, smitha.t,
tom.saeger, andriy.shevchenko, Ricardo Ribalda Delgado,
Linux Media Mailing List, Linux Kernel Mailing List,
linux-mediatek
On 06/08/2018 11:31 AM, Tomasz Figa wrote:
> Hi Hans,
>
> On Fri, Jun 8, 2018 at 6:29 PM Hans Verkuil <hverkuil@xs4all.nl> wrote:
>>
>> On 05/30/2018 09:16 AM, Keiichi Watanabe wrote:
>>> Add a new control V4L2_CID_MPEG_VIDEO_VP9_PROFILE for selecting desired
>>> profile for VP9 encoder and querying for supported profiles by VP9 encoder
>>> or decoder.
>>>
>>> An existing control V4L2_CID_MPEG_VIDEO_VPX_PROFILE cannot be
>>> used for querying since it is not a menu control but an integer
>>> control, which cannot return an arbitrary set of supported profiles.
>>>
>>> The new control V4L2_CID_MPEG_VIDEO_VP9_PROFILE is a menu control as
>>> with controls for other codec profiles. (e.g. H264)
>>
>> Please ignore my reply to patch 2/2. I looked at this a bit more and what you
>> should do is to change the type of V4L2_CID_MPEG_VIDEO_VPX_PROFILE to enum.
>
> Note that we still need a way to query VP8 and VP9 separately, since
> the supported profiles will differ. (Most of hardware we have today
> support all 4 profiles of VP8 and only profile 0 of VP9.)
Urgh. So V4L2_CID_MPEG_VIDEO_VPX_PROFILE is really just for VP8?
In that case I would suggest that we rename V4L2_CID_MPEG_VIDEO_VPX_PROFILE to
V4L2_CID_MPEG_VIDEO_VP8_PROFILE and change it to an enum. Also add this line
to v4l2-controls.h:
#define V4L2_CID_MPEG_VIDEO_VPX_PROFILE V4L2_CID_MPEG_VIDEO_VP8_PROFILE
And add a new V4L2_CID_MPEG_VIDEO_VP9_PROFILE (basically this patch).
Would that work?
This standardizes on enums for all profile controls (except s5p-mfc, which
makes its own int control, up to samsung to decide whether or not to change
that), and you have VP8 and VP9 specific profiles.
Regards,
Hans
>
> Best regards,
> Tomasz
>
>>
>> All other codec profile controls are all enums, so the fact that VPX_PROFILE
>> isn't is a bug. Changing the type should not cause any problems since the same
>> control value is used when you set the control.
>>
>> Sylwester: I see that s5p-mfc uses this control, but it is explicitly added
>> as an integer type control, so the s5p-mfc driver should not be affected by
>> changing the type of this control.
>>
>> Stanimir: this will slightly change the venus driver, but since it is a very
>> recent driver I think we can get away with changing the core type of the
>> VPX_PROFILE control. I think that's better than ending up with two controls
>> that do the same thing.
>>
>> Regards,
>>
>> Hans
>>
>>>
>>> Signed-off-by: Keiichi Watanabe <keiichiw@chromium.org>
>>> ---
>>> .../media/uapi/v4l/extended-controls.rst | 26 +++++++++++++++++++
>>> drivers/media/v4l2-core/v4l2-ctrls.c | 12 +++++++++
>>> include/uapi/linux/v4l2-controls.h | 8 ++++++
>>> 3 files changed, 46 insertions(+)
>>>
>>> diff --git a/Documentation/media/uapi/v4l/extended-controls.rst b/Documentation/media/uapi/v4l/extended-controls.rst
>>> index 03931f9b1285..4f7f128a4998 100644
>>> --- a/Documentation/media/uapi/v4l/extended-controls.rst
>>> +++ b/Documentation/media/uapi/v4l/extended-controls.rst
>>> @@ -1959,6 +1959,32 @@ enum v4l2_vp8_golden_frame_sel -
>>> Select the desired profile for VPx encoder. Acceptable values are 0,
>>> 1, 2 and 3 corresponding to encoder profiles 0, 1, 2 and 3.
>>>
>>> +.. _v4l2-mpeg-video-vp9-profile:
>>> +
>>> +``V4L2_CID_MPEG_VIDEO_VP9_PROFILE``
>>> + (enum)
>>> +
>>> +enum v4l2_mpeg_video_vp9_profile -
>>> + This control allows to select the profile for VP9 encoder.
>>> + This is also used to enumerate supported profiles by VP9 encoder or decoder.
>>> + Possible values are:
>>> +
>>> +
>>> +
>>> +.. flat-table::
>>> + :header-rows: 0
>>> + :stub-columns: 0
>>> +
>>> + * - ``V4L2_MPEG_VIDEO_VP9_PROFILE_0``
>>> + - Profile 0
>>> + * - ``V4L2_MPEG_VIDEO_VP9_PROFILE_1``
>>> + - Profile 1
>>> + * - ``V4L2_MPEG_VIDEO_VP9_PROFILE_2``
>>> + - Profile 2
>>> + * - ``V4L2_MPEG_VIDEO_VP9_PROFILE_3``
>>> + - Profile 3
>>> +
>>> +
>>>
>>> High Efficiency Video Coding (HEVC/H.265) Control Reference
>>> -----------------------------------------------------------
>>> diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c b/drivers/media/v4l2-core/v4l2-ctrls.c
>>> index d29e45516eb7..401ce21c2e63 100644
>>> --- a/drivers/media/v4l2-core/v4l2-ctrls.c
>>> +++ b/drivers/media/v4l2-core/v4l2-ctrls.c
>>> @@ -431,6 +431,13 @@ const char * const *v4l2_ctrl_get_menu(u32 id)
>>> "Use Previous Specific Frame",
>>> NULL,
>>> };
>>> + static const char * const vp9_profile[] = {
>>> + "0",
>>> + "1",
>>> + "2",
>>> + "3",
>>> + NULL,
>>> + };
>>>
>>> static const char * const flash_led_mode[] = {
>>> "Off",
>>> @@ -614,6 +621,8 @@ const char * const *v4l2_ctrl_get_menu(u32 id)
>>> return mpeg4_profile;
>>> case V4L2_CID_MPEG_VIDEO_VPX_GOLDEN_FRAME_SEL:
>>> return vpx_golden_frame_sel;
>>> + case V4L2_CID_MPEG_VIDEO_VP9_PROFILE:
>>> + return vp9_profile;
>>> case V4L2_CID_JPEG_CHROMA_SUBSAMPLING:
>>> return jpeg_chroma_subsampling;
>>> case V4L2_CID_DV_TX_MODE:
>>> @@ -841,6 +850,8 @@ const char *v4l2_ctrl_get_name(u32 id)
>>> case V4L2_CID_MPEG_VIDEO_VPX_P_FRAME_QP: return "VPX P-Frame QP Value";
>>> case V4L2_CID_MPEG_VIDEO_VPX_PROFILE: return "VPX Profile";
>>>
>>> + case V4L2_CID_MPEG_VIDEO_VP9_PROFILE: return "VP9 Profile";
>>> +
>>> /* HEVC controls */
>>> case V4L2_CID_MPEG_VIDEO_HEVC_I_FRAME_QP: return "HEVC I-Frame QP Value";
>>> case V4L2_CID_MPEG_VIDEO_HEVC_P_FRAME_QP: return "HEVC P-Frame QP Value";
>>> @@ -1180,6 +1191,7 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type,
>>> case V4L2_CID_DEINTERLACING_MODE:
>>> case V4L2_CID_TUNE_DEEMPHASIS:
>>> case V4L2_CID_MPEG_VIDEO_VPX_GOLDEN_FRAME_SEL:
>>> + case V4L2_CID_MPEG_VIDEO_VP9_PROFILE:
>>> case V4L2_CID_DETECT_MD_MODE:
>>> case V4L2_CID_MPEG_VIDEO_HEVC_PROFILE:
>>> case V4L2_CID_MPEG_VIDEO_HEVC_LEVEL:
>>> diff --git a/include/uapi/linux/v4l2-controls.h b/include/uapi/linux/v4l2-controls.h
>>> index 8d473c979b61..56203b7b715c 100644
>>> --- a/include/uapi/linux/v4l2-controls.h
>>> +++ b/include/uapi/linux/v4l2-controls.h
>>> @@ -589,6 +589,14 @@ enum v4l2_vp8_golden_frame_sel {
>>> #define V4L2_CID_MPEG_VIDEO_VPX_P_FRAME_QP (V4L2_CID_MPEG_BASE+510)
>>> #define V4L2_CID_MPEG_VIDEO_VPX_PROFILE (V4L2_CID_MPEG_BASE+511)
>>>
>>> +#define V4L2_CID_MPEG_VIDEO_VP9_PROFILE (V4L2_CID_MPEG_BASE+512)
>>> +enum v4l2_mpeg_video_vp9_profile {
>>> + V4L2_MPEG_VIDEO_VP9_PROFILE_0 = 0,
>>> + V4L2_MPEG_VIDEO_VP9_PROFILE_1 = 1,
>>> + V4L2_MPEG_VIDEO_VP9_PROFILE_2 = 2,
>>> + V4L2_MPEG_VIDEO_VP9_PROFILE_3 = 3,
>>> +};
>>> +
>>> /* CIDs for HEVC encoding. */
>>>
>>> #define V4L2_CID_MPEG_VIDEO_HEVC_MIN_QP (V4L2_CID_MPEG_BASE + 600)
>>> --
>>> 2.17.0.921.gf22659ad46-goog
>>>
>>
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH v2 1/2] media: v4l2-ctrl: Add control for VP9 profile
@ 2018-06-08 9:40 ` Hans Verkuil
0 siblings, 0 replies; 25+ messages in thread
From: Hans Verkuil @ 2018-06-08 9:40 UTC (permalink / raw)
To: linux-arm-kernel
On 06/08/2018 11:31 AM, Tomasz Figa wrote:
> Hi Hans,
>
> On Fri, Jun 8, 2018 at 6:29 PM Hans Verkuil <hverkuil@xs4all.nl> wrote:
>>
>> On 05/30/2018 09:16 AM, Keiichi Watanabe wrote:
>>> Add a new control V4L2_CID_MPEG_VIDEO_VP9_PROFILE for selecting desired
>>> profile for VP9 encoder and querying for supported profiles by VP9 encoder
>>> or decoder.
>>>
>>> An existing control V4L2_CID_MPEG_VIDEO_VPX_PROFILE cannot be
>>> used for querying since it is not a menu control but an integer
>>> control, which cannot return an arbitrary set of supported profiles.
>>>
>>> The new control V4L2_CID_MPEG_VIDEO_VP9_PROFILE is a menu control as
>>> with controls for other codec profiles. (e.g. H264)
>>
>> Please ignore my reply to patch 2/2. I looked at this a bit more and what you
>> should do is to change the type of V4L2_CID_MPEG_VIDEO_VPX_PROFILE to enum.
>
> Note that we still need a way to query VP8 and VP9 separately, since
> the supported profiles will differ. (Most of hardware we have today
> support all 4 profiles of VP8 and only profile 0 of VP9.)
Urgh. So V4L2_CID_MPEG_VIDEO_VPX_PROFILE is really just for VP8?
In that case I would suggest that we rename V4L2_CID_MPEG_VIDEO_VPX_PROFILE to
V4L2_CID_MPEG_VIDEO_VP8_PROFILE and change it to an enum. Also add this line
to v4l2-controls.h:
#define V4L2_CID_MPEG_VIDEO_VPX_PROFILE V4L2_CID_MPEG_VIDEO_VP8_PROFILE
And add a new V4L2_CID_MPEG_VIDEO_VP9_PROFILE (basically this patch).
Would that work?
This standardizes on enums for all profile controls (except s5p-mfc, which
makes its own int control, up to samsung to decide whether or not to change
that), and you have VP8 and VP9 specific profiles.
Regards,
Hans
>
> Best regards,
> Tomasz
>
>>
>> All other codec profile controls are all enums, so the fact that VPX_PROFILE
>> isn't is a bug. Changing the type should not cause any problems since the same
>> control value is used when you set the control.
>>
>> Sylwester: I see that s5p-mfc uses this control, but it is explicitly added
>> as an integer type control, so the s5p-mfc driver should not be affected by
>> changing the type of this control.
>>
>> Stanimir: this will slightly change the venus driver, but since it is a very
>> recent driver I think we can get away with changing the core type of the
>> VPX_PROFILE control. I think that's better than ending up with two controls
>> that do the same thing.
>>
>> Regards,
>>
>> Hans
>>
>>>
>>> Signed-off-by: Keiichi Watanabe <keiichiw@chromium.org>
>>> ---
>>> .../media/uapi/v4l/extended-controls.rst | 26 +++++++++++++++++++
>>> drivers/media/v4l2-core/v4l2-ctrls.c | 12 +++++++++
>>> include/uapi/linux/v4l2-controls.h | 8 ++++++
>>> 3 files changed, 46 insertions(+)
>>>
>>> diff --git a/Documentation/media/uapi/v4l/extended-controls.rst b/Documentation/media/uapi/v4l/extended-controls.rst
>>> index 03931f9b1285..4f7f128a4998 100644
>>> --- a/Documentation/media/uapi/v4l/extended-controls.rst
>>> +++ b/Documentation/media/uapi/v4l/extended-controls.rst
>>> @@ -1959,6 +1959,32 @@ enum v4l2_vp8_golden_frame_sel -
>>> Select the desired profile for VPx encoder. Acceptable values are 0,
>>> 1, 2 and 3 corresponding to encoder profiles 0, 1, 2 and 3.
>>>
>>> +.. _v4l2-mpeg-video-vp9-profile:
>>> +
>>> +``V4L2_CID_MPEG_VIDEO_VP9_PROFILE``
>>> + (enum)
>>> +
>>> +enum v4l2_mpeg_video_vp9_profile -
>>> + This control allows to select the profile for VP9 encoder.
>>> + This is also used to enumerate supported profiles by VP9 encoder or decoder.
>>> + Possible values are:
>>> +
>>> +
>>> +
>>> +.. flat-table::
>>> + :header-rows: 0
>>> + :stub-columns: 0
>>> +
>>> + * - ``V4L2_MPEG_VIDEO_VP9_PROFILE_0``
>>> + - Profile 0
>>> + * - ``V4L2_MPEG_VIDEO_VP9_PROFILE_1``
>>> + - Profile 1
>>> + * - ``V4L2_MPEG_VIDEO_VP9_PROFILE_2``
>>> + - Profile 2
>>> + * - ``V4L2_MPEG_VIDEO_VP9_PROFILE_3``
>>> + - Profile 3
>>> +
>>> +
>>>
>>> High Efficiency Video Coding (HEVC/H.265) Control Reference
>>> -----------------------------------------------------------
>>> diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c b/drivers/media/v4l2-core/v4l2-ctrls.c
>>> index d29e45516eb7..401ce21c2e63 100644
>>> --- a/drivers/media/v4l2-core/v4l2-ctrls.c
>>> +++ b/drivers/media/v4l2-core/v4l2-ctrls.c
>>> @@ -431,6 +431,13 @@ const char * const *v4l2_ctrl_get_menu(u32 id)
>>> "Use Previous Specific Frame",
>>> NULL,
>>> };
>>> + static const char * const vp9_profile[] = {
>>> + "0",
>>> + "1",
>>> + "2",
>>> + "3",
>>> + NULL,
>>> + };
>>>
>>> static const char * const flash_led_mode[] = {
>>> "Off",
>>> @@ -614,6 +621,8 @@ const char * const *v4l2_ctrl_get_menu(u32 id)
>>> return mpeg4_profile;
>>> case V4L2_CID_MPEG_VIDEO_VPX_GOLDEN_FRAME_SEL:
>>> return vpx_golden_frame_sel;
>>> + case V4L2_CID_MPEG_VIDEO_VP9_PROFILE:
>>> + return vp9_profile;
>>> case V4L2_CID_JPEG_CHROMA_SUBSAMPLING:
>>> return jpeg_chroma_subsampling;
>>> case V4L2_CID_DV_TX_MODE:
>>> @@ -841,6 +850,8 @@ const char *v4l2_ctrl_get_name(u32 id)
>>> case V4L2_CID_MPEG_VIDEO_VPX_P_FRAME_QP: return "VPX P-Frame QP Value";
>>> case V4L2_CID_MPEG_VIDEO_VPX_PROFILE: return "VPX Profile";
>>>
>>> + case V4L2_CID_MPEG_VIDEO_VP9_PROFILE: return "VP9 Profile";
>>> +
>>> /* HEVC controls */
>>> case V4L2_CID_MPEG_VIDEO_HEVC_I_FRAME_QP: return "HEVC I-Frame QP Value";
>>> case V4L2_CID_MPEG_VIDEO_HEVC_P_FRAME_QP: return "HEVC P-Frame QP Value";
>>> @@ -1180,6 +1191,7 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type,
>>> case V4L2_CID_DEINTERLACING_MODE:
>>> case V4L2_CID_TUNE_DEEMPHASIS:
>>> case V4L2_CID_MPEG_VIDEO_VPX_GOLDEN_FRAME_SEL:
>>> + case V4L2_CID_MPEG_VIDEO_VP9_PROFILE:
>>> case V4L2_CID_DETECT_MD_MODE:
>>> case V4L2_CID_MPEG_VIDEO_HEVC_PROFILE:
>>> case V4L2_CID_MPEG_VIDEO_HEVC_LEVEL:
>>> diff --git a/include/uapi/linux/v4l2-controls.h b/include/uapi/linux/v4l2-controls.h
>>> index 8d473c979b61..56203b7b715c 100644
>>> --- a/include/uapi/linux/v4l2-controls.h
>>> +++ b/include/uapi/linux/v4l2-controls.h
>>> @@ -589,6 +589,14 @@ enum v4l2_vp8_golden_frame_sel {
>>> #define V4L2_CID_MPEG_VIDEO_VPX_P_FRAME_QP (V4L2_CID_MPEG_BASE+510)
>>> #define V4L2_CID_MPEG_VIDEO_VPX_PROFILE (V4L2_CID_MPEG_BASE+511)
>>>
>>> +#define V4L2_CID_MPEG_VIDEO_VP9_PROFILE (V4L2_CID_MPEG_BASE+512)
>>> +enum v4l2_mpeg_video_vp9_profile {
>>> + V4L2_MPEG_VIDEO_VP9_PROFILE_0 = 0,
>>> + V4L2_MPEG_VIDEO_VP9_PROFILE_1 = 1,
>>> + V4L2_MPEG_VIDEO_VP9_PROFILE_2 = 2,
>>> + V4L2_MPEG_VIDEO_VP9_PROFILE_3 = 3,
>>> +};
>>> +
>>> /* CIDs for HEVC encoding. */
>>>
>>> #define V4L2_CID_MPEG_VIDEO_HEVC_MIN_QP (V4L2_CID_MPEG_BASE + 600)
>>> --
>>> 2.17.0.921.gf22659ad46-goog
>>>
>>
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v2 1/2] media: v4l2-ctrl: Add control for VP9 profile
@ 2018-06-08 9:44 ` Tomasz Figa
0 siblings, 0 replies; 25+ messages in thread
From: Tomasz Figa @ 2018-06-08 9:44 UTC (permalink / raw)
To: Hans Verkuil
Cc: keiichiw, open list:IOMMU DRIVERS,
list@263.net:IOMMU DRIVERS
<iommu@lists.linux-foundation.org>,
Joerg Roedel <joro@8bytes.org>,,
list@263.net:IOMMU DRIVERS
<iommu@lists.linux-foundation.org>,
Joerg Roedel <joro@8bytes.org>,,
Mauro Carvalho Chehab, Tiffany Lin (林慧珊),
Andrew-CT Chen (陳智迪),
Matthias Brugger, Sakari Ailus, Sylwester Nawrocki, smitha.t,
tom.saeger, andriy.shevchenko, Ricardo Ribalda Delgado,
Linux Media Mailing List, Linux Kernel Mailing List,
linux-mediatek, svarbanov
On Fri, Jun 8, 2018 at 6:40 PM Hans Verkuil <hverkuil@xs4all.nl> wrote:
>
> On 06/08/2018 11:31 AM, Tomasz Figa wrote:
> > Hi Hans,
> >
> > On Fri, Jun 8, 2018 at 6:29 PM Hans Verkuil <hverkuil@xs4all.nl> wrote:
> >>
> >> On 05/30/2018 09:16 AM, Keiichi Watanabe wrote:
> >>> Add a new control V4L2_CID_MPEG_VIDEO_VP9_PROFILE for selecting desired
> >>> profile for VP9 encoder and querying for supported profiles by VP9 encoder
> >>> or decoder.
> >>>
> >>> An existing control V4L2_CID_MPEG_VIDEO_VPX_PROFILE cannot be
> >>> used for querying since it is not a menu control but an integer
> >>> control, which cannot return an arbitrary set of supported profiles.
> >>>
> >>> The new control V4L2_CID_MPEG_VIDEO_VP9_PROFILE is a menu control as
> >>> with controls for other codec profiles. (e.g. H264)
> >>
> >> Please ignore my reply to patch 2/2. I looked at this a bit more and what you
> >> should do is to change the type of V4L2_CID_MPEG_VIDEO_VPX_PROFILE to enum.
> >
> > Note that we still need a way to query VP8 and VP9 separately, since
> > the supported profiles will differ. (Most of hardware we have today
> > support all 4 profiles of VP8 and only profile 0 of VP9.)
>
> Urgh. So V4L2_CID_MPEG_VIDEO_VPX_PROFILE is really just for VP8?
I don't know the background, but it's completely inconsistent to
similar controls we have for other codecs. (Also we don't have
V4L2_CID_MPEG_VIDEO_PROFILE, but rather 1 control for each codec).
>
> In that case I would suggest that we rename V4L2_CID_MPEG_VIDEO_VPX_PROFILE to
> V4L2_CID_MPEG_VIDEO_VP8_PROFILE and change it to an enum. Also add this line
> to v4l2-controls.h:
>
> #define V4L2_CID_MPEG_VIDEO_VPX_PROFILE V4L2_CID_MPEG_VIDEO_VP8_PROFILE
>
> And add a new V4L2_CID_MPEG_VIDEO_VP9_PROFILE (basically this patch).
>
> Would that work?
>
> This standardizes on enums for all profile controls (except s5p-mfc, which
> makes its own int control, up to samsung to decide whether or not to change
> that), and you have VP8 and VP9 specific profiles.
Sounds good to me, thanks.
Best regards,
Tomasz
>
> Regards,
>
> Hans
>
> >
> > Best regards,
> > Tomasz
> >
> >>
> >> All other codec profile controls are all enums, so the fact that VPX_PROFILE
> >> isn't is a bug. Changing the type should not cause any problems since the same
> >> control value is used when you set the control.
> >>
> >> Sylwester: I see that s5p-mfc uses this control, but it is explicitly added
> >> as an integer type control, so the s5p-mfc driver should not be affected by
> >> changing the type of this control.
> >>
> >> Stanimir: this will slightly change the venus driver, but since it is a very
> >> recent driver I think we can get away with changing the core type of the
> >> VPX_PROFILE control. I think that's better than ending up with two controls
> >> that do the same thing.
> >>
> >> Regards,
> >>
> >> Hans
> >>
> >>>
> >>> Signed-off-by: Keiichi Watanabe <keiichiw@chromium.org>
> >>> ---
> >>> .../media/uapi/v4l/extended-controls.rst | 26 +++++++++++++++++++
> >>> drivers/media/v4l2-core/v4l2-ctrls.c | 12 +++++++++
> >>> include/uapi/linux/v4l2-controls.h | 8 ++++++
> >>> 3 files changed, 46 insertions(+)
> >>>
> >>> diff --git a/Documentation/media/uapi/v4l/extended-controls.rst b/Documentation/media/uapi/v4l/extended-controls.rst
> >>> index 03931f9b1285..4f7f128a4998 100644
> >>> --- a/Documentation/media/uapi/v4l/extended-controls.rst
> >>> +++ b/Documentation/media/uapi/v4l/extended-controls.rst
> >>> @@ -1959,6 +1959,32 @@ enum v4l2_vp8_golden_frame_sel -
> >>> Select the desired profile for VPx encoder. Acceptable values are 0,
> >>> 1, 2 and 3 corresponding to encoder profiles 0, 1, 2 and 3.
> >>>
> >>> +.. _v4l2-mpeg-video-vp9-profile:
> >>> +
> >>> +``V4L2_CID_MPEG_VIDEO_VP9_PROFILE``
> >>> + (enum)
> >>> +
> >>> +enum v4l2_mpeg_video_vp9_profile -
> >>> + This control allows to select the profile for VP9 encoder.
> >>> + This is also used to enumerate supported profiles by VP9 encoder or decoder.
> >>> + Possible values are:
> >>> +
> >>> +
> >>> +
> >>> +.. flat-table::
> >>> + :header-rows: 0
> >>> + :stub-columns: 0
> >>> +
> >>> + * - ``V4L2_MPEG_VIDEO_VP9_PROFILE_0``
> >>> + - Profile 0
> >>> + * - ``V4L2_MPEG_VIDEO_VP9_PROFILE_1``
> >>> + - Profile 1
> >>> + * - ``V4L2_MPEG_VIDEO_VP9_PROFILE_2``
> >>> + - Profile 2
> >>> + * - ``V4L2_MPEG_VIDEO_VP9_PROFILE_3``
> >>> + - Profile 3
> >>> +
> >>> +
> >>>
> >>> High Efficiency Video Coding (HEVC/H.265) Control Reference
> >>> -----------------------------------------------------------
> >>> diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c b/drivers/media/v4l2-core/v4l2-ctrls.c
> >>> index d29e45516eb7..401ce21c2e63 100644
> >>> --- a/drivers/media/v4l2-core/v4l2-ctrls.c
> >>> +++ b/drivers/media/v4l2-core/v4l2-ctrls.c
> >>> @@ -431,6 +431,13 @@ const char * const *v4l2_ctrl_get_menu(u32 id)
> >>> "Use Previous Specific Frame",
> >>> NULL,
> >>> };
> >>> + static const char * const vp9_profile[] = {
> >>> + "0",
> >>> + "1",
> >>> + "2",
> >>> + "3",
> >>> + NULL,
> >>> + };
> >>>
> >>> static const char * const flash_led_mode[] = {
> >>> "Off",
> >>> @@ -614,6 +621,8 @@ const char * const *v4l2_ctrl_get_menu(u32 id)
> >>> return mpeg4_profile;
> >>> case V4L2_CID_MPEG_VIDEO_VPX_GOLDEN_FRAME_SEL:
> >>> return vpx_golden_frame_sel;
> >>> + case V4L2_CID_MPEG_VIDEO_VP9_PROFILE:
> >>> + return vp9_profile;
> >>> case V4L2_CID_JPEG_CHROMA_SUBSAMPLING:
> >>> return jpeg_chroma_subsampling;
> >>> case V4L2_CID_DV_TX_MODE:
> >>> @@ -841,6 +850,8 @@ const char *v4l2_ctrl_get_name(u32 id)
> >>> case V4L2_CID_MPEG_VIDEO_VPX_P_FRAME_QP: return "VPX P-Frame QP Value";
> >>> case V4L2_CID_MPEG_VIDEO_VPX_PROFILE: return "VPX Profile";
> >>>
> >>> + case V4L2_CID_MPEG_VIDEO_VP9_PROFILE: return "VP9 Profile";
> >>> +
> >>> /* HEVC controls */
> >>> case V4L2_CID_MPEG_VIDEO_HEVC_I_FRAME_QP: return "HEVC I-Frame QP Value";
> >>> case V4L2_CID_MPEG_VIDEO_HEVC_P_FRAME_QP: return "HEVC P-Frame QP Value";
> >>> @@ -1180,6 +1191,7 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type,
> >>> case V4L2_CID_DEINTERLACING_MODE:
> >>> case V4L2_CID_TUNE_DEEMPHASIS:
> >>> case V4L2_CID_MPEG_VIDEO_VPX_GOLDEN_FRAME_SEL:
> >>> + case V4L2_CID_MPEG_VIDEO_VP9_PROFILE:
> >>> case V4L2_CID_DETECT_MD_MODE:
> >>> case V4L2_CID_MPEG_VIDEO_HEVC_PROFILE:
> >>> case V4L2_CID_MPEG_VIDEO_HEVC_LEVEL:
> >>> diff --git a/include/uapi/linux/v4l2-controls.h b/include/uapi/linux/v4l2-controls.h
> >>> index 8d473c979b61..56203b7b715c 100644
> >>> --- a/include/uapi/linux/v4l2-controls.h
> >>> +++ b/include/uapi/linux/v4l2-controls.h
> >>> @@ -589,6 +589,14 @@ enum v4l2_vp8_golden_frame_sel {
> >>> #define V4L2_CID_MPEG_VIDEO_VPX_P_FRAME_QP (V4L2_CID_MPEG_BASE+510)
> >>> #define V4L2_CID_MPEG_VIDEO_VPX_PROFILE (V4L2_CID_MPEG_BASE+511)
> >>>
> >>> +#define V4L2_CID_MPEG_VIDEO_VP9_PROFILE (V4L2_CID_MPEG_BASE+512)
> >>> +enum v4l2_mpeg_video_vp9_profile {
> >>> + V4L2_MPEG_VIDEO_VP9_PROFILE_0 = 0,
> >>> + V4L2_MPEG_VIDEO_VP9_PROFILE_1 = 1,
> >>> + V4L2_MPEG_VIDEO_VP9_PROFILE_2 = 2,
> >>> + V4L2_MPEG_VIDEO_VP9_PROFILE_3 = 3,
> >>> +};
> >>> +
> >>> /* CIDs for HEVC encoding. */
> >>>
> >>> #define V4L2_CID_MPEG_VIDEO_HEVC_MIN_QP (V4L2_CID_MPEG_BASE + 600)
> >>> --
> >>> 2.17.0.921.gf22659ad46-goog
> >>>
> >>
>
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v2 1/2] media: v4l2-ctrl: Add control for VP9 profile
@ 2018-06-08 9:44 ` Tomasz Figa
0 siblings, 0 replies; 25+ messages in thread
From: Tomasz Figa @ 2018-06-08 9:44 UTC (permalink / raw)
To: Hans Verkuil
Cc: Andrew-CT Chen (陳智迪),
Tiffany Lin (林慧珊),
Sylwester Nawrocki, tom.saeger-QHcLZuEGTsvQT0dZR+AlfA,
smitha.t-Sze3O3UU22JBDgjK7y7TUQ,
svarbanov-NEYub+7Iv8PQT0dZR+AlfA, Linux Kernel Mailing List,
andriy.shevchenko-VuQAYsv1563Yd54FQh9/CA,
keiichiw-F7+t8E8rja9g9hUCZPvPmw, open list:IOMMU DRIVERS,
linux-mediatek-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
Matthias Brugger, Ricardo Ribalda Delgado, Mauro Carvalho Chehab,
list-Y9sIeH5OGRo@public.gmane.org:IOMMU DRIVERS
<iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org>,
Joerg Roedel <joro-zLv9SwRftAIdnm+yROfE0A@public.gmane.org>,
, Linux Media Mailing List
On Fri, Jun 8, 2018 at 6:40 PM Hans Verkuil <hverkuil-qWit8jRvyhVmR6Xm/wNWPw@public.gmane.org> wrote:
>
> On 06/08/2018 11:31 AM, Tomasz Figa wrote:
> > Hi Hans,
> >
> > On Fri, Jun 8, 2018 at 6:29 PM Hans Verkuil <hverkuil-qWit8jRvyhVmR6Xm/wNWPw@public.gmane.org> wrote:
> >>
> >> On 05/30/2018 09:16 AM, Keiichi Watanabe wrote:
> >>> Add a new control V4L2_CID_MPEG_VIDEO_VP9_PROFILE for selecting desired
> >>> profile for VP9 encoder and querying for supported profiles by VP9 encoder
> >>> or decoder.
> >>>
> >>> An existing control V4L2_CID_MPEG_VIDEO_VPX_PROFILE cannot be
> >>> used for querying since it is not a menu control but an integer
> >>> control, which cannot return an arbitrary set of supported profiles.
> >>>
> >>> The new control V4L2_CID_MPEG_VIDEO_VP9_PROFILE is a menu control as
> >>> with controls for other codec profiles. (e.g. H264)
> >>
> >> Please ignore my reply to patch 2/2. I looked at this a bit more and what you
> >> should do is to change the type of V4L2_CID_MPEG_VIDEO_VPX_PROFILE to enum.
> >
> > Note that we still need a way to query VP8 and VP9 separately, since
> > the supported profiles will differ. (Most of hardware we have today
> > support all 4 profiles of VP8 and only profile 0 of VP9.)
>
> Urgh. So V4L2_CID_MPEG_VIDEO_VPX_PROFILE is really just for VP8?
I don't know the background, but it's completely inconsistent to
similar controls we have for other codecs. (Also we don't have
V4L2_CID_MPEG_VIDEO_PROFILE, but rather 1 control for each codec).
>
> In that case I would suggest that we rename V4L2_CID_MPEG_VIDEO_VPX_PROFILE to
> V4L2_CID_MPEG_VIDEO_VP8_PROFILE and change it to an enum. Also add this line
> to v4l2-controls.h:
>
> #define V4L2_CID_MPEG_VIDEO_VPX_PROFILE V4L2_CID_MPEG_VIDEO_VP8_PROFILE
>
> And add a new V4L2_CID_MPEG_VIDEO_VP9_PROFILE (basically this patch).
>
> Would that work?
>
> This standardizes on enums for all profile controls (except s5p-mfc, which
> makes its own int control, up to samsung to decide whether or not to change
> that), and you have VP8 and VP9 specific profiles.
Sounds good to me, thanks.
Best regards,
Tomasz
>
> Regards,
>
> Hans
>
> >
> > Best regards,
> > Tomasz
> >
> >>
> >> All other codec profile controls are all enums, so the fact that VPX_PROFILE
> >> isn't is a bug. Changing the type should not cause any problems since the same
> >> control value is used when you set the control.
> >>
> >> Sylwester: I see that s5p-mfc uses this control, but it is explicitly added
> >> as an integer type control, so the s5p-mfc driver should not be affected by
> >> changing the type of this control.
> >>
> >> Stanimir: this will slightly change the venus driver, but since it is a very
> >> recent driver I think we can get away with changing the core type of the
> >> VPX_PROFILE control. I think that's better than ending up with two controls
> >> that do the same thing.
> >>
> >> Regards,
> >>
> >> Hans
> >>
> >>>
> >>> Signed-off-by: Keiichi Watanabe <keiichiw-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
> >>> ---
> >>> .../media/uapi/v4l/extended-controls.rst | 26 +++++++++++++++++++
> >>> drivers/media/v4l2-core/v4l2-ctrls.c | 12 +++++++++
> >>> include/uapi/linux/v4l2-controls.h | 8 ++++++
> >>> 3 files changed, 46 insertions(+)
> >>>
> >>> diff --git a/Documentation/media/uapi/v4l/extended-controls.rst b/Documentation/media/uapi/v4l/extended-controls.rst
> >>> index 03931f9b1285..4f7f128a4998 100644
> >>> --- a/Documentation/media/uapi/v4l/extended-controls.rst
> >>> +++ b/Documentation/media/uapi/v4l/extended-controls.rst
> >>> @@ -1959,6 +1959,32 @@ enum v4l2_vp8_golden_frame_sel -
> >>> Select the desired profile for VPx encoder. Acceptable values are 0,
> >>> 1, 2 and 3 corresponding to encoder profiles 0, 1, 2 and 3.
> >>>
> >>> +.. _v4l2-mpeg-video-vp9-profile:
> >>> +
> >>> +``V4L2_CID_MPEG_VIDEO_VP9_PROFILE``
> >>> + (enum)
> >>> +
> >>> +enum v4l2_mpeg_video_vp9_profile -
> >>> + This control allows to select the profile for VP9 encoder.
> >>> + This is also used to enumerate supported profiles by VP9 encoder or decoder.
> >>> + Possible values are:
> >>> +
> >>> +
> >>> +
> >>> +.. flat-table::
> >>> + :header-rows: 0
> >>> + :stub-columns: 0
> >>> +
> >>> + * - ``V4L2_MPEG_VIDEO_VP9_PROFILE_0``
> >>> + - Profile 0
> >>> + * - ``V4L2_MPEG_VIDEO_VP9_PROFILE_1``
> >>> + - Profile 1
> >>> + * - ``V4L2_MPEG_VIDEO_VP9_PROFILE_2``
> >>> + - Profile 2
> >>> + * - ``V4L2_MPEG_VIDEO_VP9_PROFILE_3``
> >>> + - Profile 3
> >>> +
> >>> +
> >>>
> >>> High Efficiency Video Coding (HEVC/H.265) Control Reference
> >>> -----------------------------------------------------------
> >>> diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c b/drivers/media/v4l2-core/v4l2-ctrls.c
> >>> index d29e45516eb7..401ce21c2e63 100644
> >>> --- a/drivers/media/v4l2-core/v4l2-ctrls.c
> >>> +++ b/drivers/media/v4l2-core/v4l2-ctrls.c
> >>> @@ -431,6 +431,13 @@ const char * const *v4l2_ctrl_get_menu(u32 id)
> >>> "Use Previous Specific Frame",
> >>> NULL,
> >>> };
> >>> + static const char * const vp9_profile[] = {
> >>> + "0",
> >>> + "1",
> >>> + "2",
> >>> + "3",
> >>> + NULL,
> >>> + };
> >>>
> >>> static const char * const flash_led_mode[] = {
> >>> "Off",
> >>> @@ -614,6 +621,8 @@ const char * const *v4l2_ctrl_get_menu(u32 id)
> >>> return mpeg4_profile;
> >>> case V4L2_CID_MPEG_VIDEO_VPX_GOLDEN_FRAME_SEL:
> >>> return vpx_golden_frame_sel;
> >>> + case V4L2_CID_MPEG_VIDEO_VP9_PROFILE:
> >>> + return vp9_profile;
> >>> case V4L2_CID_JPEG_CHROMA_SUBSAMPLING:
> >>> return jpeg_chroma_subsampling;
> >>> case V4L2_CID_DV_TX_MODE:
> >>> @@ -841,6 +850,8 @@ const char *v4l2_ctrl_get_name(u32 id)
> >>> case V4L2_CID_MPEG_VIDEO_VPX_P_FRAME_QP: return "VPX P-Frame QP Value";
> >>> case V4L2_CID_MPEG_VIDEO_VPX_PROFILE: return "VPX Profile";
> >>>
> >>> + case V4L2_CID_MPEG_VIDEO_VP9_PROFILE: return "VP9 Profile";
> >>> +
> >>> /* HEVC controls */
> >>> case V4L2_CID_MPEG_VIDEO_HEVC_I_FRAME_QP: return "HEVC I-Frame QP Value";
> >>> case V4L2_CID_MPEG_VIDEO_HEVC_P_FRAME_QP: return "HEVC P-Frame QP Value";
> >>> @@ -1180,6 +1191,7 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type,
> >>> case V4L2_CID_DEINTERLACING_MODE:
> >>> case V4L2_CID_TUNE_DEEMPHASIS:
> >>> case V4L2_CID_MPEG_VIDEO_VPX_GOLDEN_FRAME_SEL:
> >>> + case V4L2_CID_MPEG_VIDEO_VP9_PROFILE:
> >>> case V4L2_CID_DETECT_MD_MODE:
> >>> case V4L2_CID_MPEG_VIDEO_HEVC_PROFILE:
> >>> case V4L2_CID_MPEG_VIDEO_HEVC_LEVEL:
> >>> diff --git a/include/uapi/linux/v4l2-controls.h b/include/uapi/linux/v4l2-controls.h
> >>> index 8d473c979b61..56203b7b715c 100644
> >>> --- a/include/uapi/linux/v4l2-controls.h
> >>> +++ b/include/uapi/linux/v4l2-controls.h
> >>> @@ -589,6 +589,14 @@ enum v4l2_vp8_golden_frame_sel {
> >>> #define V4L2_CID_MPEG_VIDEO_VPX_P_FRAME_QP (V4L2_CID_MPEG_BASE+510)
> >>> #define V4L2_CID_MPEG_VIDEO_VPX_PROFILE (V4L2_CID_MPEG_BASE+511)
> >>>
> >>> +#define V4L2_CID_MPEG_VIDEO_VP9_PROFILE (V4L2_CID_MPEG_BASE+512)
> >>> +enum v4l2_mpeg_video_vp9_profile {
> >>> + V4L2_MPEG_VIDEO_VP9_PROFILE_0 = 0,
> >>> + V4L2_MPEG_VIDEO_VP9_PROFILE_1 = 1,
> >>> + V4L2_MPEG_VIDEO_VP9_PROFILE_2 = 2,
> >>> + V4L2_MPEG_VIDEO_VP9_PROFILE_3 = 3,
> >>> +};
> >>> +
> >>> /* CIDs for HEVC encoding. */
> >>>
> >>> #define V4L2_CID_MPEG_VIDEO_HEVC_MIN_QP (V4L2_CID_MPEG_BASE + 600)
> >>> --
> >>> 2.17.0.921.gf22659ad46-goog
> >>>
> >>
>
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH v2 1/2] media: v4l2-ctrl: Add control for VP9 profile
@ 2018-06-08 9:44 ` Tomasz Figa
0 siblings, 0 replies; 25+ messages in thread
From: Tomasz Figa @ 2018-06-08 9:44 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, Jun 8, 2018 at 6:40 PM Hans Verkuil <hverkuil@xs4all.nl> wrote:
>
> On 06/08/2018 11:31 AM, Tomasz Figa wrote:
> > Hi Hans,
> >
> > On Fri, Jun 8, 2018 at 6:29 PM Hans Verkuil <hverkuil@xs4all.nl> wrote:
> >>
> >> On 05/30/2018 09:16 AM, Keiichi Watanabe wrote:
> >>> Add a new control V4L2_CID_MPEG_VIDEO_VP9_PROFILE for selecting desired
> >>> profile for VP9 encoder and querying for supported profiles by VP9 encoder
> >>> or decoder.
> >>>
> >>> An existing control V4L2_CID_MPEG_VIDEO_VPX_PROFILE cannot be
> >>> used for querying since it is not a menu control but an integer
> >>> control, which cannot return an arbitrary set of supported profiles.
> >>>
> >>> The new control V4L2_CID_MPEG_VIDEO_VP9_PROFILE is a menu control as
> >>> with controls for other codec profiles. (e.g. H264)
> >>
> >> Please ignore my reply to patch 2/2. I looked at this a bit more and what you
> >> should do is to change the type of V4L2_CID_MPEG_VIDEO_VPX_PROFILE to enum.
> >
> > Note that we still need a way to query VP8 and VP9 separately, since
> > the supported profiles will differ. (Most of hardware we have today
> > support all 4 profiles of VP8 and only profile 0 of VP9.)
>
> Urgh. So V4L2_CID_MPEG_VIDEO_VPX_PROFILE is really just for VP8?
I don't know the background, but it's completely inconsistent to
similar controls we have for other codecs. (Also we don't have
V4L2_CID_MPEG_VIDEO_PROFILE, but rather 1 control for each codec).
>
> In that case I would suggest that we rename V4L2_CID_MPEG_VIDEO_VPX_PROFILE to
> V4L2_CID_MPEG_VIDEO_VP8_PROFILE and change it to an enum. Also add this line
> to v4l2-controls.h:
>
> #define V4L2_CID_MPEG_VIDEO_VPX_PROFILE V4L2_CID_MPEG_VIDEO_VP8_PROFILE
>
> And add a new V4L2_CID_MPEG_VIDEO_VP9_PROFILE (basically this patch).
>
> Would that work?
>
> This standardizes on enums for all profile controls (except s5p-mfc, which
> makes its own int control, up to samsung to decide whether or not to change
> that), and you have VP8 and VP9 specific profiles.
Sounds good to me, thanks.
Best regards,
Tomasz
>
> Regards,
>
> Hans
>
> >
> > Best regards,
> > Tomasz
> >
> >>
> >> All other codec profile controls are all enums, so the fact that VPX_PROFILE
> >> isn't is a bug. Changing the type should not cause any problems since the same
> >> control value is used when you set the control.
> >>
> >> Sylwester: I see that s5p-mfc uses this control, but it is explicitly added
> >> as an integer type control, so the s5p-mfc driver should not be affected by
> >> changing the type of this control.
> >>
> >> Stanimir: this will slightly change the venus driver, but since it is a very
> >> recent driver I think we can get away with changing the core type of the
> >> VPX_PROFILE control. I think that's better than ending up with two controls
> >> that do the same thing.
> >>
> >> Regards,
> >>
> >> Hans
> >>
> >>>
> >>> Signed-off-by: Keiichi Watanabe <keiichiw@chromium.org>
> >>> ---
> >>> .../media/uapi/v4l/extended-controls.rst | 26 +++++++++++++++++++
> >>> drivers/media/v4l2-core/v4l2-ctrls.c | 12 +++++++++
> >>> include/uapi/linux/v4l2-controls.h | 8 ++++++
> >>> 3 files changed, 46 insertions(+)
> >>>
> >>> diff --git a/Documentation/media/uapi/v4l/extended-controls.rst b/Documentation/media/uapi/v4l/extended-controls.rst
> >>> index 03931f9b1285..4f7f128a4998 100644
> >>> --- a/Documentation/media/uapi/v4l/extended-controls.rst
> >>> +++ b/Documentation/media/uapi/v4l/extended-controls.rst
> >>> @@ -1959,6 +1959,32 @@ enum v4l2_vp8_golden_frame_sel -
> >>> Select the desired profile for VPx encoder. Acceptable values are 0,
> >>> 1, 2 and 3 corresponding to encoder profiles 0, 1, 2 and 3.
> >>>
> >>> +.. _v4l2-mpeg-video-vp9-profile:
> >>> +
> >>> +``V4L2_CID_MPEG_VIDEO_VP9_PROFILE``
> >>> + (enum)
> >>> +
> >>> +enum v4l2_mpeg_video_vp9_profile -
> >>> + This control allows to select the profile for VP9 encoder.
> >>> + This is also used to enumerate supported profiles by VP9 encoder or decoder.
> >>> + Possible values are:
> >>> +
> >>> +
> >>> +
> >>> +.. flat-table::
> >>> + :header-rows: 0
> >>> + :stub-columns: 0
> >>> +
> >>> + * - ``V4L2_MPEG_VIDEO_VP9_PROFILE_0``
> >>> + - Profile 0
> >>> + * - ``V4L2_MPEG_VIDEO_VP9_PROFILE_1``
> >>> + - Profile 1
> >>> + * - ``V4L2_MPEG_VIDEO_VP9_PROFILE_2``
> >>> + - Profile 2
> >>> + * - ``V4L2_MPEG_VIDEO_VP9_PROFILE_3``
> >>> + - Profile 3
> >>> +
> >>> +
> >>>
> >>> High Efficiency Video Coding (HEVC/H.265) Control Reference
> >>> -----------------------------------------------------------
> >>> diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c b/drivers/media/v4l2-core/v4l2-ctrls.c
> >>> index d29e45516eb7..401ce21c2e63 100644
> >>> --- a/drivers/media/v4l2-core/v4l2-ctrls.c
> >>> +++ b/drivers/media/v4l2-core/v4l2-ctrls.c
> >>> @@ -431,6 +431,13 @@ const char * const *v4l2_ctrl_get_menu(u32 id)
> >>> "Use Previous Specific Frame",
> >>> NULL,
> >>> };
> >>> + static const char * const vp9_profile[] = {
> >>> + "0",
> >>> + "1",
> >>> + "2",
> >>> + "3",
> >>> + NULL,
> >>> + };
> >>>
> >>> static const char * const flash_led_mode[] = {
> >>> "Off",
> >>> @@ -614,6 +621,8 @@ const char * const *v4l2_ctrl_get_menu(u32 id)
> >>> return mpeg4_profile;
> >>> case V4L2_CID_MPEG_VIDEO_VPX_GOLDEN_FRAME_SEL:
> >>> return vpx_golden_frame_sel;
> >>> + case V4L2_CID_MPEG_VIDEO_VP9_PROFILE:
> >>> + return vp9_profile;
> >>> case V4L2_CID_JPEG_CHROMA_SUBSAMPLING:
> >>> return jpeg_chroma_subsampling;
> >>> case V4L2_CID_DV_TX_MODE:
> >>> @@ -841,6 +850,8 @@ const char *v4l2_ctrl_get_name(u32 id)
> >>> case V4L2_CID_MPEG_VIDEO_VPX_P_FRAME_QP: return "VPX P-Frame QP Value";
> >>> case V4L2_CID_MPEG_VIDEO_VPX_PROFILE: return "VPX Profile";
> >>>
> >>> + case V4L2_CID_MPEG_VIDEO_VP9_PROFILE: return "VP9 Profile";
> >>> +
> >>> /* HEVC controls */
> >>> case V4L2_CID_MPEG_VIDEO_HEVC_I_FRAME_QP: return "HEVC I-Frame QP Value";
> >>> case V4L2_CID_MPEG_VIDEO_HEVC_P_FRAME_QP: return "HEVC P-Frame QP Value";
> >>> @@ -1180,6 +1191,7 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type,
> >>> case V4L2_CID_DEINTERLACING_MODE:
> >>> case V4L2_CID_TUNE_DEEMPHASIS:
> >>> case V4L2_CID_MPEG_VIDEO_VPX_GOLDEN_FRAME_SEL:
> >>> + case V4L2_CID_MPEG_VIDEO_VP9_PROFILE:
> >>> case V4L2_CID_DETECT_MD_MODE:
> >>> case V4L2_CID_MPEG_VIDEO_HEVC_PROFILE:
> >>> case V4L2_CID_MPEG_VIDEO_HEVC_LEVEL:
> >>> diff --git a/include/uapi/linux/v4l2-controls.h b/include/uapi/linux/v4l2-controls.h
> >>> index 8d473c979b61..56203b7b715c 100644
> >>> --- a/include/uapi/linux/v4l2-controls.h
> >>> +++ b/include/uapi/linux/v4l2-controls.h
> >>> @@ -589,6 +589,14 @@ enum v4l2_vp8_golden_frame_sel {
> >>> #define V4L2_CID_MPEG_VIDEO_VPX_P_FRAME_QP (V4L2_CID_MPEG_BASE+510)
> >>> #define V4L2_CID_MPEG_VIDEO_VPX_PROFILE (V4L2_CID_MPEG_BASE+511)
> >>>
> >>> +#define V4L2_CID_MPEG_VIDEO_VP9_PROFILE (V4L2_CID_MPEG_BASE+512)
> >>> +enum v4l2_mpeg_video_vp9_profile {
> >>> + V4L2_MPEG_VIDEO_VP9_PROFILE_0 = 0,
> >>> + V4L2_MPEG_VIDEO_VP9_PROFILE_1 = 1,
> >>> + V4L2_MPEG_VIDEO_VP9_PROFILE_2 = 2,
> >>> + V4L2_MPEG_VIDEO_VP9_PROFILE_3 = 3,
> >>> +};
> >>> +
> >>> /* CIDs for HEVC encoding. */
> >>>
> >>> #define V4L2_CID_MPEG_VIDEO_HEVC_MIN_QP (V4L2_CID_MPEG_BASE + 600)
> >>> --
> >>> 2.17.0.921.gf22659ad46-goog
> >>>
> >>
>
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v2 1/2] media: v4l2-ctrl: Add control for VP9 profile
2018-06-08 9:29 ` Hans Verkuil
@ 2018-06-08 12:56 ` Stanimir Varbanov
-1 siblings, 0 replies; 25+ messages in thread
From: Stanimir Varbanov @ 2018-06-08 12:56 UTC (permalink / raw)
To: Hans Verkuil, Keiichi Watanabe, linux-arm-kernel
Cc: Mauro Carvalho Chehab, Tiffany Lin, Andrew-CT Chen,
Matthias Brugger, Sakari Ailus, Sylwester Nawrocki,
Smitha T Murthy, Tom Saeger, Andy Shevchenko, Tomasz Figa,
Ricardo Ribalda Delgado, linux-media, linux-kernel,
linux-mediatek
Hi Hans,
On 06/08/2018 12:29 PM, Hans Verkuil wrote:
> On 05/30/2018 09:16 AM, Keiichi Watanabe wrote:
>> Add a new control V4L2_CID_MPEG_VIDEO_VP9_PROFILE for selecting desired
>> profile for VP9 encoder and querying for supported profiles by VP9 encoder
>> or decoder.
>>
>> An existing control V4L2_CID_MPEG_VIDEO_VPX_PROFILE cannot be
>> used for querying since it is not a menu control but an integer
>> control, which cannot return an arbitrary set of supported profiles.
>>
>> The new control V4L2_CID_MPEG_VIDEO_VP9_PROFILE is a menu control as
>> with controls for other codec profiles. (e.g. H264)
>
> Please ignore my reply to patch 2/2. I looked at this a bit more and what you
> should do is to change the type of V4L2_CID_MPEG_VIDEO_VPX_PROFILE to enum.
>
> All other codec profile controls are all enums, so the fact that VPX_PROFILE
> isn't is a bug. Changing the type should not cause any problems since the same
> control value is used when you set the control.
>
> Sylwester: I see that s5p-mfc uses this control, but it is explicitly added
> as an integer type control, so the s5p-mfc driver should not be affected by
> changing the type of this control.
>
> Stanimir: this will slightly change the venus driver, but since it is a very
> recent driver I think we can get away with changing the core type of the
> VPX_PROFILE control. I think that's better than ending up with two controls
> that do the same thing.
I agree. Actually the changes shouldn't be so much in venus driver.
--
regards,
Stan
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH v2 1/2] media: v4l2-ctrl: Add control for VP9 profile
@ 2018-06-08 12:56 ` Stanimir Varbanov
0 siblings, 0 replies; 25+ messages in thread
From: Stanimir Varbanov @ 2018-06-08 12:56 UTC (permalink / raw)
To: linux-arm-kernel
Hi Hans,
On 06/08/2018 12:29 PM, Hans Verkuil wrote:
> On 05/30/2018 09:16 AM, Keiichi Watanabe wrote:
>> Add a new control V4L2_CID_MPEG_VIDEO_VP9_PROFILE for selecting desired
>> profile for VP9 encoder and querying for supported profiles by VP9 encoder
>> or decoder.
>>
>> An existing control V4L2_CID_MPEG_VIDEO_VPX_PROFILE cannot be
>> used for querying since it is not a menu control but an integer
>> control, which cannot return an arbitrary set of supported profiles.
>>
>> The new control V4L2_CID_MPEG_VIDEO_VP9_PROFILE is a menu control as
>> with controls for other codec profiles. (e.g. H264)
>
> Please ignore my reply to patch 2/2. I looked at this a bit more and what you
> should do is to change the type of V4L2_CID_MPEG_VIDEO_VPX_PROFILE to enum.
>
> All other codec profile controls are all enums, so the fact that VPX_PROFILE
> isn't is a bug. Changing the type should not cause any problems since the same
> control value is used when you set the control.
>
> Sylwester: I see that s5p-mfc uses this control, but it is explicitly added
> as an integer type control, so the s5p-mfc driver should not be affected by
> changing the type of this control.
>
> Stanimir: this will slightly change the venus driver, but since it is a very
> recent driver I think we can get away with changing the core type of the
> VPX_PROFILE control. I think that's better than ending up with two controls
> that do the same thing.
I agree. Actually the changes shouldn't be so much in venus driver.
--
regards,
Stan
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v2 1/2] media: v4l2-ctrl: Add control for VP9 profile
[not found] ` <CAKQmDh8MnArB1jCD6STh9FgLX+H4ZsEnH+coOJe-4A=FNPGshA@mail.gmail.com>
@ 2018-06-11 6:44 ` Keiichi Watanabe
0 siblings, 0 replies; 25+ messages in thread
From: Keiichi Watanabe @ 2018-06-11 6:44 UTC (permalink / raw)
To: nicolas
Cc: stanimir.varbanov, Hans Verkuil, linux-arm-kernel,
Mauro Carvalho Chehab, tiffany.lin, andrew-ct.chen, matthias.bgg,
Sakari Ailus, Sylwester Nawrocki, Smitha T Murthy, tom.saeger,
Andy Shevchenko, Tomasz Figa, Ricardo Ribalda Delgado,
linux-media, linux-kernel, linux-mediatek
Hi, Hans.
Thank you for the review.
Your idea sounds good.
However, I think that changing V4L2_CID_MPEG_VIDEO_VPX_PROFILE to an enum
breaks both of s5p-mfc and venus drivers. This is because they call
'v4l2_ctrl_new_std' for it. For menu controls,
'v4l2_ctrl_new_std_menu' must be used.
https://elixir.bootlin.com/linux/latest/source/drivers/media/platform/s5p-mfc/s5p_mfc_enc.c#L2678
https://elixir.bootlin.com/linux/latest/source/drivers/media/platform/qcom/venus/vdec_ctrls.c#L133
I can fix them within the commit for adding VP8_PROFILE control, but
cannot confirm that it works on real devices since I don't have them.
What should I do?
Best regards,
Keiichi
On Fri, Jun 8, 2018 at 10:00 PM Nicolas Dufresne <nicolas@ndufresne.ca> wrote:
>
>
>
> Le ven. 8 juin 2018 08:56, Stanimir Varbanov <stanimir.varbanov@linaro.org> a écrit :
>>
>> Hi Hans,
>>
>> On 06/08/2018 12:29 PM, Hans Verkuil wrote:
>> > On 05/30/2018 09:16 AM, Keiichi Watanabe wrote:
>> >> Add a new control V4L2_CID_MPEG_VIDEO_VP9_PROFILE for selecting desired
>> >> profile for VP9 encoder and querying for supported profiles by VP9 encoder
>> >> or decoder.
>> >>
>> >> An existing control V4L2_CID_MPEG_VIDEO_VPX_PROFILE cannot be
>> >> used for querying since it is not a menu control but an integer
>> >> control, which cannot return an arbitrary set of supported profiles.
>> >>
>> >> The new control V4L2_CID_MPEG_VIDEO_VP9_PROFILE is a menu control as
>> >> with controls for other codec profiles. (e.g. H264)
>> >
>> > Please ignore my reply to patch 2/2. I looked at this a bit more and what you
>> > should do is to change the type of V4L2_CID_MPEG_VIDEO_VPX_PROFILE to enum.
>> >
>> > All other codec profile controls are all enums, so the fact that VPX_PROFILE
>> > isn't is a bug. Changing the type should not cause any problems since the same
>> > control value is used when you set the control.
>> >
>> > Sylwester: I see that s5p-mfc uses this control, but it is explicitly added
>> > as an integer type control, so the s5p-mfc driver should not be affected by
>> > changing the type of this control.
>> >
>> > Stanimir: this will slightly change the venus driver, but since it is a very
>> > recent driver I think we can get away with changing the core type of the
>> > VPX_PROFILE control. I think that's better than ending up with two controls
>> > that do the same thing.
>>
>> I agree. Actually the changes shouldn't be so much in venus driver.
>
>
> Also fine on userspace side, since profiles enumeration isn't implemented yet in FFMPEG, GStreamer, Chrome
>
>
>>
>> --
>> regards,
>> Stan
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH v2 1/2] media: v4l2-ctrl: Add control for VP9 profile
@ 2018-06-11 6:44 ` Keiichi Watanabe
0 siblings, 0 replies; 25+ messages in thread
From: Keiichi Watanabe @ 2018-06-11 6:44 UTC (permalink / raw)
To: linux-arm-kernel
Hi, Hans.
Thank you for the review.
Your idea sounds good.
However, I think that changing V4L2_CID_MPEG_VIDEO_VPX_PROFILE to an enum
breaks both of s5p-mfc and venus drivers. This is because they call
'v4l2_ctrl_new_std' for it. For menu controls,
'v4l2_ctrl_new_std_menu' must be used.
https://elixir.bootlin.com/linux/latest/source/drivers/media/platform/s5p-mfc/s5p_mfc_enc.c#L2678
https://elixir.bootlin.com/linux/latest/source/drivers/media/platform/qcom/venus/vdec_ctrls.c#L133
I can fix them within the commit for adding VP8_PROFILE control, but
cannot confirm that it works on real devices since I don't have them.
What should I do?
Best regards,
Keiichi
On Fri, Jun 8, 2018 at 10:00 PM Nicolas Dufresne <nicolas@ndufresne.ca> wrote:
>
>
>
> Le ven. 8 juin 2018 08:56, Stanimir Varbanov <stanimir.varbanov@linaro.org> a ?crit :
>>
>> Hi Hans,
>>
>> On 06/08/2018 12:29 PM, Hans Verkuil wrote:
>> > On 05/30/2018 09:16 AM, Keiichi Watanabe wrote:
>> >> Add a new control V4L2_CID_MPEG_VIDEO_VP9_PROFILE for selecting desired
>> >> profile for VP9 encoder and querying for supported profiles by VP9 encoder
>> >> or decoder.
>> >>
>> >> An existing control V4L2_CID_MPEG_VIDEO_VPX_PROFILE cannot be
>> >> used for querying since it is not a menu control but an integer
>> >> control, which cannot return an arbitrary set of supported profiles.
>> >>
>> >> The new control V4L2_CID_MPEG_VIDEO_VP9_PROFILE is a menu control as
>> >> with controls for other codec profiles. (e.g. H264)
>> >
>> > Please ignore my reply to patch 2/2. I looked at this a bit more and what you
>> > should do is to change the type of V4L2_CID_MPEG_VIDEO_VPX_PROFILE to enum.
>> >
>> > All other codec profile controls are all enums, so the fact that VPX_PROFILE
>> > isn't is a bug. Changing the type should not cause any problems since the same
>> > control value is used when you set the control.
>> >
>> > Sylwester: I see that s5p-mfc uses this control, but it is explicitly added
>> > as an integer type control, so the s5p-mfc driver should not be affected by
>> > changing the type of this control.
>> >
>> > Stanimir: this will slightly change the venus driver, but since it is a very
>> > recent driver I think we can get away with changing the core type of the
>> > VPX_PROFILE control. I think that's better than ending up with two controls
>> > that do the same thing.
>>
>> I agree. Actually the changes shouldn't be so much in venus driver.
>
>
> Also fine on userspace side, since profiles enumeration isn't implemented yet in FFMPEG, GStreamer, Chrome
>
>
>>
>> --
>> regards,
>> Stan
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v2 1/2] media: v4l2-ctrl: Add control for VP9 profile
2018-06-11 6:44 ` Keiichi Watanabe
@ 2018-06-11 7:38 ` Hans Verkuil
-1 siblings, 0 replies; 25+ messages in thread
From: Hans Verkuil @ 2018-06-11 7:38 UTC (permalink / raw)
To: Keiichi Watanabe, nicolas
Cc: stanimir.varbanov, linux-arm-kernel, Mauro Carvalho Chehab,
tiffany.lin, andrew-ct.chen, matthias.bgg, Sakari Ailus,
Sylwester Nawrocki, Smitha T Murthy, tom.saeger, Andy Shevchenko,
Tomasz Figa, Ricardo Ribalda Delgado, linux-media, linux-kernel,
linux-mediatek
On 11/06/18 08:44, Keiichi Watanabe wrote:
> Hi, Hans.
> Thank you for the review.
> Your idea sounds good.
>
> However, I think that changing V4L2_CID_MPEG_VIDEO_VPX_PROFILE to an enum
> breaks both of s5p-mfc and venus drivers. This is because they call
> 'v4l2_ctrl_new_std' for it. For menu controls,
> 'v4l2_ctrl_new_std_menu' must be used.
>
> https://elixir.bootlin.com/linux/latest/source/drivers/media/platform/s5p-mfc/s5p_mfc_enc.c#L2678
> https://elixir.bootlin.com/linux/latest/source/drivers/media/platform/qcom/venus/vdec_ctrls.c#L133
>
> I can fix them within the commit for adding VP8_PROFILE control, but
> cannot confirm that it works on real devices since I don't have them.
> What should I do?
Fix it in both drivers with a Cc to stanimir.varbanov@linaro.org (venus) and
s.nawrocki@samsung.com (s5p) to let them test/ack the patch.
Venus is no problem, the s5p driver can decide to keep the old INT control
since it has been in use for such a long time, but that is up to Sylwester to
decide.
Regards,
Hans
>
> Best regards,
> Keiichi
> On Fri, Jun 8, 2018 at 10:00 PM Nicolas Dufresne <nicolas@ndufresne.ca> wrote:
>>
>>
>>
>> Le ven. 8 juin 2018 08:56, Stanimir Varbanov <stanimir.varbanov@linaro.org> a écrit :
>>>
>>> Hi Hans,
>>>
>>> On 06/08/2018 12:29 PM, Hans Verkuil wrote:
>>>> On 05/30/2018 09:16 AM, Keiichi Watanabe wrote:
>>>>> Add a new control V4L2_CID_MPEG_VIDEO_VP9_PROFILE for selecting desired
>>>>> profile for VP9 encoder and querying for supported profiles by VP9 encoder
>>>>> or decoder.
>>>>>
>>>>> An existing control V4L2_CID_MPEG_VIDEO_VPX_PROFILE cannot be
>>>>> used for querying since it is not a menu control but an integer
>>>>> control, which cannot return an arbitrary set of supported profiles.
>>>>>
>>>>> The new control V4L2_CID_MPEG_VIDEO_VP9_PROFILE is a menu control as
>>>>> with controls for other codec profiles. (e.g. H264)
>>>>
>>>> Please ignore my reply to patch 2/2. I looked at this a bit more and what you
>>>> should do is to change the type of V4L2_CID_MPEG_VIDEO_VPX_PROFILE to enum.
>>>>
>>>> All other codec profile controls are all enums, so the fact that VPX_PROFILE
>>>> isn't is a bug. Changing the type should not cause any problems since the same
>>>> control value is used when you set the control.
>>>>
>>>> Sylwester: I see that s5p-mfc uses this control, but it is explicitly added
>>>> as an integer type control, so the s5p-mfc driver should not be affected by
>>>> changing the type of this control.
>>>>
>>>> Stanimir: this will slightly change the venus driver, but since it is a very
>>>> recent driver I think we can get away with changing the core type of the
>>>> VPX_PROFILE control. I think that's better than ending up with two controls
>>>> that do the same thing.
>>>
>>> I agree. Actually the changes shouldn't be so much in venus driver.
>>
>>
>> Also fine on userspace side, since profiles enumeration isn't implemented yet in FFMPEG, GStreamer, Chrome
>>
>>
>>>
>>> --
>>> regards,
>>> Stan
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH v2 1/2] media: v4l2-ctrl: Add control for VP9 profile
@ 2018-06-11 7:38 ` Hans Verkuil
0 siblings, 0 replies; 25+ messages in thread
From: Hans Verkuil @ 2018-06-11 7:38 UTC (permalink / raw)
To: linux-arm-kernel
On 11/06/18 08:44, Keiichi Watanabe wrote:
> Hi, Hans.
> Thank you for the review.
> Your idea sounds good.
>
> However, I think that changing V4L2_CID_MPEG_VIDEO_VPX_PROFILE to an enum
> breaks both of s5p-mfc and venus drivers. This is because they call
> 'v4l2_ctrl_new_std' for it. For menu controls,
> 'v4l2_ctrl_new_std_menu' must be used.
>
> https://elixir.bootlin.com/linux/latest/source/drivers/media/platform/s5p-mfc/s5p_mfc_enc.c#L2678
> https://elixir.bootlin.com/linux/latest/source/drivers/media/platform/qcom/venus/vdec_ctrls.c#L133
>
> I can fix them within the commit for adding VP8_PROFILE control, but
> cannot confirm that it works on real devices since I don't have them.
> What should I do?
Fix it in both drivers with a Cc to stanimir.varbanov at linaro.org (venus) and
s.nawrocki at samsung.com (s5p) to let them test/ack the patch.
Venus is no problem, the s5p driver can decide to keep the old INT control
since it has been in use for such a long time, but that is up to Sylwester to
decide.
Regards,
Hans
>
> Best regards,
> Keiichi
> On Fri, Jun 8, 2018 at 10:00 PM Nicolas Dufresne <nicolas@ndufresne.ca> wrote:
>>
>>
>>
>> Le ven. 8 juin 2018 08:56, Stanimir Varbanov <stanimir.varbanov@linaro.org> a ?crit :
>>>
>>> Hi Hans,
>>>
>>> On 06/08/2018 12:29 PM, Hans Verkuil wrote:
>>>> On 05/30/2018 09:16 AM, Keiichi Watanabe wrote:
>>>>> Add a new control V4L2_CID_MPEG_VIDEO_VP9_PROFILE for selecting desired
>>>>> profile for VP9 encoder and querying for supported profiles by VP9 encoder
>>>>> or decoder.
>>>>>
>>>>> An existing control V4L2_CID_MPEG_VIDEO_VPX_PROFILE cannot be
>>>>> used for querying since it is not a menu control but an integer
>>>>> control, which cannot return an arbitrary set of supported profiles.
>>>>>
>>>>> The new control V4L2_CID_MPEG_VIDEO_VP9_PROFILE is a menu control as
>>>>> with controls for other codec profiles. (e.g. H264)
>>>>
>>>> Please ignore my reply to patch 2/2. I looked at this a bit more and what you
>>>> should do is to change the type of V4L2_CID_MPEG_VIDEO_VPX_PROFILE to enum.
>>>>
>>>> All other codec profile controls are all enums, so the fact that VPX_PROFILE
>>>> isn't is a bug. Changing the type should not cause any problems since the same
>>>> control value is used when you set the control.
>>>>
>>>> Sylwester: I see that s5p-mfc uses this control, but it is explicitly added
>>>> as an integer type control, so the s5p-mfc driver should not be affected by
>>>> changing the type of this control.
>>>>
>>>> Stanimir: this will slightly change the venus driver, but since it is a very
>>>> recent driver I think we can get away with changing the core type of the
>>>> VPX_PROFILE control. I think that's better than ending up with two controls
>>>> that do the same thing.
>>>
>>> I agree. Actually the changes shouldn't be so much in venus driver.
>>
>>
>> Also fine on userspace side, since profiles enumeration isn't implemented yet in FFMPEG, GStreamer, Chrome
>>
>>
>>>
>>> --
>>> regards,
>>> Stan
^ permalink raw reply [flat|nested] 25+ messages in thread
end of thread, other threads:[~2018-06-11 7:38 UTC | newest]
Thread overview: 25+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-05-30 7:16 [PATCH v2 0/2] Add control for VP9 profile Keiichi Watanabe
2018-05-30 7:16 ` Keiichi Watanabe
2018-05-30 7:16 ` [PATCH v2 1/2] media: v4l2-ctrl: " Keiichi Watanabe
2018-05-30 7:16 ` Keiichi Watanabe
2018-06-08 9:29 ` Hans Verkuil
2018-06-08 9:29 ` Hans Verkuil
2018-06-08 9:31 ` Tomasz Figa
2018-06-08 9:31 ` Tomasz Figa
2018-06-08 9:31 ` Tomasz Figa
2018-06-08 9:40 ` Hans Verkuil
2018-06-08 9:40 ` Hans Verkuil
2018-06-08 9:40 ` Hans Verkuil
2018-06-08 9:44 ` Tomasz Figa
2018-06-08 9:44 ` Tomasz Figa
2018-06-08 9:44 ` Tomasz Figa
2018-06-08 12:56 ` Stanimir Varbanov
2018-06-08 12:56 ` Stanimir Varbanov
[not found] ` <CAKQmDh8MnArB1jCD6STh9FgLX+H4ZsEnH+coOJe-4A=FNPGshA@mail.gmail.com>
2018-06-11 6:44 ` Keiichi Watanabe
2018-06-11 6:44 ` Keiichi Watanabe
2018-06-11 7:38 ` Hans Verkuil
2018-06-11 7:38 ` Hans Verkuil
2018-05-30 7:16 ` [PATCH v2 2/2] media: mtk-vcodec: Support VP9 profile in decoder Keiichi Watanabe
2018-05-30 7:16 ` Keiichi Watanabe
2018-06-08 9:14 ` Hans Verkuil
2018-06-08 9:14 ` Hans Verkuil
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.