* [PATCH] CHROMIUM: s5p-mfc: add controls to set vp8 enc profile
@ 2013-12-09 13:16 Arun Kumar K
2013-12-09 13:22 ` Hans Verkuil
0 siblings, 1 reply; 7+ messages in thread
From: Arun Kumar K @ 2013-12-09 13:16 UTC (permalink / raw)
To: linux-media, linux-samsung-soc
Cc: k.debski, hverkuil, avnd.kiran, posciak, arunkk.samsung
Add v4l2 controls to set desired profile for VP8 encoder.
Acceptable levels for VP8 encoder are
0: Version 0
1: Version 1
2: Version 2
3: Version 3
Signed-off-by: Pawel Osciak <posciak@chromium.org>
Signed-off-by: Kiran AVND <avnd.kiran@samsung.com>
Signed-off-by: Arun Kumar K <arun.kk@samsung.com>
---
This patch is rebased over another VP8 control patch from me:
https://linuxtv.org/patch/20733/
---
Documentation/DocBook/media/v4l/controls.xml | 9 +++++++++
drivers/media/platform/s5p-mfc/s5p_mfc_common.h | 1 +
drivers/media/platform/s5p-mfc/s5p_mfc_enc.c | 11 +++++++++++
drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c | 6 ++----
drivers/media/v4l2-core/v4l2-ctrls.c | 8 ++++++++
include/uapi/linux/v4l2-controls.h | 1 +
6 files changed, 32 insertions(+), 4 deletions(-)
diff --git a/Documentation/DocBook/media/v4l/controls.xml b/Documentation/DocBook/media/v4l/controls.xml
index e4db4ac..c1f7544 100644
--- a/Documentation/DocBook/media/v4l/controls.xml
+++ b/Documentation/DocBook/media/v4l/controls.xml
@@ -3193,6 +3193,15 @@ V4L2_CID_MPEG_VIDEO_VPX_GOLDEN_FRAME_REF_PERIOD as a golden frame.</entry>
<row><entry spanname="descr">Quantization parameter for a P frame for VP8.</entry>
</row>
+ <row><entry></entry></row>
+ <row>
+ <entry spanname="id"><constant>V4L2_CID_MPEG_VIDEO_VPX_PROFILE</constant> </entry>
+ <entry>integer</entry>
+ </row>
+ <row><entry spanname="descr">Select the desired profile for VP8 encoder.
+Acceptable values are 0, 1, 2 and 3 corresponding to encoder versions 0, 1, 2 and 3.</entry>
+ </row>
+
<row><entry></entry></row>
</tbody>
</tgroup>
diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_common.h b/drivers/media/platform/s5p-mfc/s5p_mfc_common.h
index d91f757..797e61d 100644
--- a/drivers/media/platform/s5p-mfc/s5p_mfc_common.h
+++ b/drivers/media/platform/s5p-mfc/s5p_mfc_common.h
@@ -426,6 +426,7 @@ struct s5p_mfc_vp8_enc_params {
u8 rc_max_qp;
u8 rc_frame_qp;
u8 rc_p_frame_qp;
+ u8 profile;
};
/**
diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_enc.c b/drivers/media/platform/s5p-mfc/s5p_mfc_enc.c
index 33e8ae3..ec0581c 100644
--- a/drivers/media/platform/s5p-mfc/s5p_mfc_enc.c
+++ b/drivers/media/platform/s5p-mfc/s5p_mfc_enc.c
@@ -650,6 +650,14 @@ static struct mfc_control controls[] = {
.step = 1,
.default_value = 10,
},
+ {
+ .id = V4L2_CID_MPEG_VIDEO_VPX_PROFILE,
+ .type = V4L2_CTRL_TYPE_INTEGER_MENU,
+ .minimum = 0,
+ .maximum = 3,
+ .step = 1,
+ .default_value = 0,
+ },
};
#define NUM_CTRLS ARRAY_SIZE(controls)
@@ -1601,6 +1609,9 @@ static int s5p_mfc_enc_s_ctrl(struct v4l2_ctrl *ctrl)
case V4L2_CID_MPEG_VIDEO_VPX_P_FRAME_QP:
p->codec.vp8.rc_p_frame_qp = ctrl->val;
break;
+ case V4L2_CID_MPEG_VIDEO_VPX_PROFILE:
+ p->codec.vp8.profile = ctrl->val;
+ break;
default:
v4l2_err(&dev->v4l2_dev, "Invalid control, id=%d, val=%d\n",
ctrl->id, ctrl->val);
diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c b/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c
index b4886d6..f6ff2db 100644
--- a/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c
+++ b/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c
@@ -1197,10 +1197,8 @@ static int s5p_mfc_set_enc_params_vp8(struct s5p_mfc_ctx *ctx)
reg |= ((p->num_b_frame & 0x3) << 16);
WRITEL(reg, S5P_FIMV_E_GOP_CONFIG_V6);
- /* profile & level */
- reg = 0;
- /** profile */
- reg |= (0x1 << 4);
+ /* profile - 0 ~ 3 */
+ reg = p_vp8->profile & 0x3;
WRITEL(reg, S5P_FIMV_E_PICTURE_PROFILE_V6);
/* rate control config. */
diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c b/drivers/media/v4l2-core/v4l2-ctrls.c
index 20840df..5069dd2 100644
--- a/drivers/media/v4l2-core/v4l2-ctrls.c
+++ b/drivers/media/v4l2-core/v4l2-ctrls.c
@@ -575,11 +575,17 @@ const s64 *v4l2_ctrl_get_int_menu(u32 id, u32 *len)
1, 2, 3,
};
+ static const s64 qmenu_int_vpx_profile[] = {
+ 0, 1, 2, 3,
+ };
+
switch (id) {
case V4L2_CID_MPEG_VIDEO_VPX_NUM_PARTITIONS:
return __v4l2_qmenu_int_len(qmenu_int_vpx_num_partitions, len);
case V4L2_CID_MPEG_VIDEO_VPX_NUM_REF_FRAMES:
return __v4l2_qmenu_int_len(qmenu_int_vpx_num_ref_frames, len);
+ case V4L2_CID_MPEG_VIDEO_VPX_PROFILE:
+ return __v4l2_qmenu_int_len(qmenu_int_vpx_profile, len);
default:
*len = 0;
return NULL;
@@ -749,6 +755,7 @@ const char *v4l2_ctrl_get_name(u32 id)
case V4L2_CID_MPEG_VIDEO_VPX_MAX_QP: return "VPX Maximum QP Value";
case V4L2_CID_MPEG_VIDEO_VPX_I_FRAME_QP: return "VPX I-Frame QP Value";
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";
/* CAMERA controls */
/* Keep the order of the 'case's the same as in videodev2.h! */
@@ -978,6 +985,7 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type,
case V4L2_CID_AUTO_EXPOSURE_BIAS:
case V4L2_CID_MPEG_VIDEO_VPX_NUM_PARTITIONS:
case V4L2_CID_MPEG_VIDEO_VPX_NUM_REF_FRAMES:
+ case V4L2_CID_MPEG_VIDEO_VPX_PROFILE:
*type = V4L2_CTRL_TYPE_INTEGER_MENU;
break;
case V4L2_CID_USER_CLASS:
diff --git a/include/uapi/linux/v4l2-controls.h b/include/uapi/linux/v4l2-controls.h
index 5b9dfc8..9970a9d 100644
--- a/include/uapi/linux/v4l2-controls.h
+++ b/include/uapi/linux/v4l2-controls.h
@@ -558,6 +558,7 @@ enum v4l2_vp8_golden_frame_sel {
#define V4L2_CID_MPEG_VIDEO_VPX_MAX_QP (V4L2_CID_MPEG_BASE+508)
#define V4L2_CID_MPEG_VIDEO_VPX_I_FRAME_QP (V4L2_CID_MPEG_BASE+509)
#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)
/* MPEG-class control IDs specific to the CX2341x driver as defined by V4L2 */
#define V4L2_CID_MPEG_CX2341X_BASE (V4L2_CTRL_CLASS_MPEG | 0x1000)
--
1.7.9.5
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] CHROMIUM: s5p-mfc: add controls to set vp8 enc profile
2013-12-09 13:16 [PATCH] CHROMIUM: s5p-mfc: add controls to set vp8 enc profile Arun Kumar K
@ 2013-12-09 13:22 ` Hans Verkuil
2013-12-10 4:44 ` Arun Kumar K
0 siblings, 1 reply; 7+ messages in thread
From: Hans Verkuil @ 2013-12-09 13:22 UTC (permalink / raw)
To: Arun Kumar K
Cc: linux-media, linux-samsung-soc, k.debski, avnd.kiran, posciak,
arunkk.samsung
Hi Arun,
Some comments below...
On 12/09/2013 02:16 PM, Arun Kumar K wrote:
> Add v4l2 controls to set desired profile for VP8 encoder.
> Acceptable levels for VP8 encoder are
> 0: Version 0
> 1: Version 1
> 2: Version 2
> 3: Version 3
>
> Signed-off-by: Pawel Osciak <posciak@chromium.org>
> Signed-off-by: Kiran AVND <avnd.kiran@samsung.com>
> Signed-off-by: Arun Kumar K <arun.kk@samsung.com>
> ---
> This patch is rebased over another VP8 control patch from me:
> https://linuxtv.org/patch/20733/
> ---
> Documentation/DocBook/media/v4l/controls.xml | 9 +++++++++
> drivers/media/platform/s5p-mfc/s5p_mfc_common.h | 1 +
> drivers/media/platform/s5p-mfc/s5p_mfc_enc.c | 11 +++++++++++
> drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c | 6 ++----
> drivers/media/v4l2-core/v4l2-ctrls.c | 8 ++++++++
> include/uapi/linux/v4l2-controls.h | 1 +
> 6 files changed, 32 insertions(+), 4 deletions(-)
>
> diff --git a/Documentation/DocBook/media/v4l/controls.xml b/Documentation/DocBook/media/v4l/controls.xml
> index e4db4ac..c1f7544 100644
> --- a/Documentation/DocBook/media/v4l/controls.xml
> +++ b/Documentation/DocBook/media/v4l/controls.xml
> @@ -3193,6 +3193,15 @@ V4L2_CID_MPEG_VIDEO_VPX_GOLDEN_FRAME_REF_PERIOD as a golden frame.</entry>
> <row><entry spanname="descr">Quantization parameter for a P frame for VP8.</entry>
> </row>
>
> + <row><entry></entry></row>
> + <row>
> + <entry spanname="id"><constant>V4L2_CID_MPEG_VIDEO_VPX_PROFILE</constant> </entry>
> + <entry>integer</entry>
This says 'integer' whereas the control is actually an integer menu.
Why did you choose 'integer menu' for this? Would a regular integer or perhaps a standard
menu be better?
> + </row>
> + <row><entry spanname="descr">Select the desired profile for VP8 encoder.
> +Acceptable values are 0, 1, 2 and 3 corresponding to encoder versions 0, 1, 2 and 3.</entry>
Is it a 'profile' or a 'version'? It looks a bit confusing. I don't have the VP8 standard,
so I can't really tell what the correct terminology is.
Also, does this control apply just to VP8 or also to other VP versions? The control name
says 'VPX' while the description says 'VP8' explicitly.
> + </row>
> +
> <row><entry></entry></row>
> </tbody>
> </tgroup>
> diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_common.h b/drivers/media/platform/s5p-mfc/s5p_mfc_common.h
> index d91f757..797e61d 100644
> --- a/drivers/media/platform/s5p-mfc/s5p_mfc_common.h
> +++ b/drivers/media/platform/s5p-mfc/s5p_mfc_common.h
> @@ -426,6 +426,7 @@ struct s5p_mfc_vp8_enc_params {
> u8 rc_max_qp;
> u8 rc_frame_qp;
> u8 rc_p_frame_qp;
> + u8 profile;
> };
>
> /**
> diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_enc.c b/drivers/media/platform/s5p-mfc/s5p_mfc_enc.c
> index 33e8ae3..ec0581c 100644
> --- a/drivers/media/platform/s5p-mfc/s5p_mfc_enc.c
> +++ b/drivers/media/platform/s5p-mfc/s5p_mfc_enc.c
> @@ -650,6 +650,14 @@ static struct mfc_control controls[] = {
> .step = 1,
> .default_value = 10,
> },
> + {
> + .id = V4L2_CID_MPEG_VIDEO_VPX_PROFILE,
> + .type = V4L2_CTRL_TYPE_INTEGER_MENU,
> + .minimum = 0,
> + .maximum = 3,
> + .step = 1,
> + .default_value = 0,
> + },
> };
>
> #define NUM_CTRLS ARRAY_SIZE(controls)
> @@ -1601,6 +1609,9 @@ static int s5p_mfc_enc_s_ctrl(struct v4l2_ctrl *ctrl)
> case V4L2_CID_MPEG_VIDEO_VPX_P_FRAME_QP:
> p->codec.vp8.rc_p_frame_qp = ctrl->val;
> break;
> + case V4L2_CID_MPEG_VIDEO_VPX_PROFILE:
> + p->codec.vp8.profile = ctrl->val;
> + break;
> default:
> v4l2_err(&dev->v4l2_dev, "Invalid control, id=%d, val=%d\n",
> ctrl->id, ctrl->val);
> diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c b/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c
> index b4886d6..f6ff2db 100644
> --- a/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c
> +++ b/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c
> @@ -1197,10 +1197,8 @@ static int s5p_mfc_set_enc_params_vp8(struct s5p_mfc_ctx *ctx)
> reg |= ((p->num_b_frame & 0x3) << 16);
> WRITEL(reg, S5P_FIMV_E_GOP_CONFIG_V6);
>
> - /* profile & level */
> - reg = 0;
> - /** profile */
> - reg |= (0x1 << 4);
> + /* profile - 0 ~ 3 */
> + reg = p_vp8->profile & 0x3;
> WRITEL(reg, S5P_FIMV_E_PICTURE_PROFILE_V6);
>
> /* rate control config. */
> diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c b/drivers/media/v4l2-core/v4l2-ctrls.c
> index 20840df..5069dd2 100644
> --- a/drivers/media/v4l2-core/v4l2-ctrls.c
> +++ b/drivers/media/v4l2-core/v4l2-ctrls.c
> @@ -575,11 +575,17 @@ const s64 *v4l2_ctrl_get_int_menu(u32 id, u32 *len)
> 1, 2, 3,
> };
>
> + static const s64 qmenu_int_vpx_profile[] = {
> + 0, 1, 2, 3,
> + };
> +
> switch (id) {
> case V4L2_CID_MPEG_VIDEO_VPX_NUM_PARTITIONS:
> return __v4l2_qmenu_int_len(qmenu_int_vpx_num_partitions, len);
> case V4L2_CID_MPEG_VIDEO_VPX_NUM_REF_FRAMES:
> return __v4l2_qmenu_int_len(qmenu_int_vpx_num_ref_frames, len);
> + case V4L2_CID_MPEG_VIDEO_VPX_PROFILE:
> + return __v4l2_qmenu_int_len(qmenu_int_vpx_profile, len);
> default:
> *len = 0;
> return NULL;
> @@ -749,6 +755,7 @@ const char *v4l2_ctrl_get_name(u32 id)
> case V4L2_CID_MPEG_VIDEO_VPX_MAX_QP: return "VPX Maximum QP Value";
> case V4L2_CID_MPEG_VIDEO_VPX_I_FRAME_QP: return "VPX I-Frame QP Value";
> 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";
>
> /* CAMERA controls */
> /* Keep the order of the 'case's the same as in videodev2.h! */
> @@ -978,6 +985,7 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type,
> case V4L2_CID_AUTO_EXPOSURE_BIAS:
> case V4L2_CID_MPEG_VIDEO_VPX_NUM_PARTITIONS:
> case V4L2_CID_MPEG_VIDEO_VPX_NUM_REF_FRAMES:
> + case V4L2_CID_MPEG_VIDEO_VPX_PROFILE:
> *type = V4L2_CTRL_TYPE_INTEGER_MENU;
> break;
> case V4L2_CID_USER_CLASS:
> diff --git a/include/uapi/linux/v4l2-controls.h b/include/uapi/linux/v4l2-controls.h
> index 5b9dfc8..9970a9d 100644
> --- a/include/uapi/linux/v4l2-controls.h
> +++ b/include/uapi/linux/v4l2-controls.h
> @@ -558,6 +558,7 @@ enum v4l2_vp8_golden_frame_sel {
> #define V4L2_CID_MPEG_VIDEO_VPX_MAX_QP (V4L2_CID_MPEG_BASE+508)
> #define V4L2_CID_MPEG_VIDEO_VPX_I_FRAME_QP (V4L2_CID_MPEG_BASE+509)
> #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)
>
> /* MPEG-class control IDs specific to the CX2341x driver as defined by V4L2 */
> #define V4L2_CID_MPEG_CX2341X_BASE (V4L2_CTRL_CLASS_MPEG | 0x1000)
>
Regards,
Hans
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] CHROMIUM: s5p-mfc: add controls to set vp8 enc profile
@ 2013-12-10 0:51 Pawel Osciak
2013-12-10 4:45 ` Arun Kumar K
0 siblings, 1 reply; 7+ messages in thread
From: Pawel Osciak @ 2013-12-10 0:51 UTC (permalink / raw)
To: Arun Kumar K
Cc: linux-media@vger.kernel.org, linux-samsung-soc, Kamil Debski,
Hans Verkuil, avnd.kiran, arunkk.samsung
Hi Arun,
On Mon, Dec 9, 2013 at 10:16 PM, Arun Kumar K <arun.kk@samsung.com> wrote:
> Add v4l2 controls to set desired profile for VP8 encoder.
> Acceptable levels for VP8 encoder are
> 0: Version 0
> 1: Version 1
> 2: Version 2
> 3: Version 3
>
> Signed-off-by: Pawel Osciak <posciak@chromium.org>
Sorry, but I'm not the author of this patch, Kiran is. I think the
confusion comes from the fact that I committed it to Chromium tree. I
think Kiran's sign-off should go first and the patch should contain:
"From: Kiran AVND <avnd.kiran@samsung.com>"
Thanks,
Pawel
> Signed-off-by: Kiran AVND <avnd.kiran@samsung.com>
> Signed-off-by: Arun Kumar K <arun.kk@samsung.com>
> ---
> This patch is rebased over another VP8 control patch from me:
> https://linuxtv.org/patch/20733/
> ---
> Documentation/DocBook/media/v4l/controls.xml | 9 +++++++++
> drivers/media/platform/s5p-mfc/s5p_mfc_common.h | 1 +
> drivers/media/platform/s5p-mfc/s5p_mfc_enc.c | 11 +++++++++++
> drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c | 6 ++----
> drivers/media/v4l2-core/v4l2-ctrls.c | 8 ++++++++
> include/uapi/linux/v4l2-controls.h | 1 +
> 6 files changed, 32 insertions(+), 4 deletions(-)
>
> diff --git a/Documentation/DocBook/media/v4l/controls.xml b/Documentation/DocBook/media/v4l/controls.xml
> index e4db4ac..c1f7544 100644
> --- a/Documentation/DocBook/media/v4l/controls.xml
> +++ b/Documentation/DocBook/media/v4l/controls.xml
> @@ -3193,6 +3193,15 @@ V4L2_CID_MPEG_VIDEO_VPX_GOLDEN_FRAME_REF_PERIOD as a golden frame.</entry>
> <row><entry spanname="descr">Quantization parameter for a P frame for VP8.</entry>
> </row>
>
> + <row><entry></entry></row>
> + <row>
> + <entry spanname="id"><constant>V4L2_CID_MPEG_VIDEO_VPX_PROFILE</constant> </entry>
> + <entry>integer</entry>
> + </row>
> + <row><entry spanname="descr">Select the desired profile for VP8 encoder.
> +Acceptable values are 0, 1, 2 and 3 corresponding to encoder versions 0, 1, 2 and 3.</entry>
> + </row>
> +
> <row><entry></entry></row>
> </tbody>
> </tgroup>
> diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_common.h b/drivers/media/platform/s5p-mfc/s5p_mfc_common.h
> index d91f757..797e61d 100644
> --- a/drivers/media/platform/s5p-mfc/s5p_mfc_common.h
> +++ b/drivers/media/platform/s5p-mfc/s5p_mfc_common.h
> @@ -426,6 +426,7 @@ struct s5p_mfc_vp8_enc_params {
> u8 rc_max_qp;
> u8 rc_frame_qp;
> u8 rc_p_frame_qp;
> + u8 profile;
> };
>
> /**
> diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_enc.c b/drivers/media/platform/s5p-mfc/s5p_mfc_enc.c
> index 33e8ae3..ec0581c 100644
> --- a/drivers/media/platform/s5p-mfc/s5p_mfc_enc.c
> +++ b/drivers/media/platform/s5p-mfc/s5p_mfc_enc.c
> @@ -650,6 +650,14 @@ static struct mfc_control controls[] = {
> .step = 1,
> .default_value = 10,
> },
> + {
> + .id = V4L2_CID_MPEG_VIDEO_VPX_PROFILE,
> + .type = V4L2_CTRL_TYPE_INTEGER_MENU,
> + .minimum = 0,
> + .maximum = 3,
> + .step = 1,
> + .default_value = 0,
> + },
> };
>
> #define NUM_CTRLS ARRAY_SIZE(controls)
> @@ -1601,6 +1609,9 @@ static int s5p_mfc_enc_s_ctrl(struct v4l2_ctrl *ctrl)
> case V4L2_CID_MPEG_VIDEO_VPX_P_FRAME_QP:
> p->codec.vp8.rc_p_frame_qp = ctrl->val;
> break;
> + case V4L2_CID_MPEG_VIDEO_VPX_PROFILE:
> + p->codec.vp8.profile = ctrl->val;
> + break;
> default:
> v4l2_err(&dev->v4l2_dev, "Invalid control, id=%d, val=%d\n",
> ctrl->id, ctrl->val);
> diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c b/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c
> index b4886d6..f6ff2db 100644
> --- a/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c
> +++ b/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c
> @@ -1197,10 +1197,8 @@ static int s5p_mfc_set_enc_params_vp8(struct s5p_mfc_ctx *ctx)
> reg |= ((p->num_b_frame & 0x3) << 16);
> WRITEL(reg, S5P_FIMV_E_GOP_CONFIG_V6);
>
> - /* profile & level */
> - reg = 0;
> - /** profile */
> - reg |= (0x1 << 4);
> + /* profile - 0 ~ 3 */
> + reg = p_vp8->profile & 0x3;
> WRITEL(reg, S5P_FIMV_E_PICTURE_PROFILE_V6);
>
> /* rate control config. */
> diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c b/drivers/media/v4l2-core/v4l2-ctrls.c
> index 20840df..5069dd2 100644
> --- a/drivers/media/v4l2-core/v4l2-ctrls.c
> +++ b/drivers/media/v4l2-core/v4l2-ctrls.c
> @@ -575,11 +575,17 @@ const s64 *v4l2_ctrl_get_int_menu(u32 id, u32 *len)
> 1, 2, 3,
> };
>
> + static const s64 qmenu_int_vpx_profile[] = {
> + 0, 1, 2, 3,
> + };
> +
> switch (id) {
> case V4L2_CID_MPEG_VIDEO_VPX_NUM_PARTITIONS:
> return __v4l2_qmenu_int_len(qmenu_int_vpx_num_partitions, len);
> case V4L2_CID_MPEG_VIDEO_VPX_NUM_REF_FRAMES:
> return __v4l2_qmenu_int_len(qmenu_int_vpx_num_ref_frames, len);
> + case V4L2_CID_MPEG_VIDEO_VPX_PROFILE:
> + return __v4l2_qmenu_int_len(qmenu_int_vpx_profile, len);
> default:
> *len = 0;
> return NULL;
> @@ -749,6 +755,7 @@ const char *v4l2_ctrl_get_name(u32 id)
> case V4L2_CID_MPEG_VIDEO_VPX_MAX_QP: return "VPX Maximum QP Value";
> case V4L2_CID_MPEG_VIDEO_VPX_I_FRAME_QP: return "VPX I-Frame QP Value";
> 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";
>
> /* CAMERA controls */
> /* Keep the order of the 'case's the same as in videodev2.h! */
> @@ -978,6 +985,7 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type,
> case V4L2_CID_AUTO_EXPOSURE_BIAS:
> case V4L2_CID_MPEG_VIDEO_VPX_NUM_PARTITIONS:
> case V4L2_CID_MPEG_VIDEO_VPX_NUM_REF_FRAMES:
> + case V4L2_CID_MPEG_VIDEO_VPX_PROFILE:
> *type = V4L2_CTRL_TYPE_INTEGER_MENU;
> break;
> case V4L2_CID_USER_CLASS:
> diff --git a/include/uapi/linux/v4l2-controls.h b/include/uapi/linux/v4l2-controls.h
> index 5b9dfc8..9970a9d 100644
> --- a/include/uapi/linux/v4l2-controls.h
> +++ b/include/uapi/linux/v4l2-controls.h
> @@ -558,6 +558,7 @@ enum v4l2_vp8_golden_frame_sel {
> #define V4L2_CID_MPEG_VIDEO_VPX_MAX_QP (V4L2_CID_MPEG_BASE+508)
> #define V4L2_CID_MPEG_VIDEO_VPX_I_FRAME_QP (V4L2_CID_MPEG_BASE+509)
> #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)
>
> /* MPEG-class control IDs specific to the CX2341x driver as defined by V4L2 */
> #define V4L2_CID_MPEG_CX2341X_BASE (V4L2_CTRL_CLASS_MPEG | 0x1000)
> --
> 1.7.9.5
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] CHROMIUM: s5p-mfc: add controls to set vp8 enc profile
2013-12-09 13:22 ` Hans Verkuil
@ 2013-12-10 4:44 ` Arun Kumar K
0 siblings, 0 replies; 7+ messages in thread
From: Arun Kumar K @ 2013-12-10 4:44 UTC (permalink / raw)
To: Hans Verkuil; +Cc: LMML, linux-samsung-soc, Kamil Debski, avnd.kiran, posciak
Hi Hans,
Thanks for the review.
On Mon, Dec 9, 2013 at 6:52 PM, Hans Verkuil <hverkuil@xs4all.nl> wrote:
> Hi Arun,
>
> Some comments below...
>
> On 12/09/2013 02:16 PM, Arun Kumar K wrote:
>> Add v4l2 controls to set desired profile for VP8 encoder.
>> Acceptable levels for VP8 encoder are
>> 0: Version 0
>> 1: Version 1
>> 2: Version 2
>> 3: Version 3
>>
>> Signed-off-by: Pawel Osciak <posciak@chromium.org>
>> Signed-off-by: Kiran AVND <avnd.kiran@samsung.com>
>> Signed-off-by: Arun Kumar K <arun.kk@samsung.com>
>> ---
>> This patch is rebased over another VP8 control patch from me:
>> https://linuxtv.org/patch/20733/
>> ---
>> Documentation/DocBook/media/v4l/controls.xml | 9 +++++++++
>> drivers/media/platform/s5p-mfc/s5p_mfc_common.h | 1 +
>> drivers/media/platform/s5p-mfc/s5p_mfc_enc.c | 11 +++++++++++
>> drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c | 6 ++----
>> drivers/media/v4l2-core/v4l2-ctrls.c | 8 ++++++++
>> include/uapi/linux/v4l2-controls.h | 1 +
>> 6 files changed, 32 insertions(+), 4 deletions(-)
>>
>> diff --git a/Documentation/DocBook/media/v4l/controls.xml b/Documentation/DocBook/media/v4l/controls.xml
>> index e4db4ac..c1f7544 100644
>> --- a/Documentation/DocBook/media/v4l/controls.xml
>> +++ b/Documentation/DocBook/media/v4l/controls.xml
>> @@ -3193,6 +3193,15 @@ V4L2_CID_MPEG_VIDEO_VPX_GOLDEN_FRAME_REF_PERIOD as a golden frame.</entry>
>> <row><entry spanname="descr">Quantization parameter for a P frame for VP8.</entry>
>> </row>
>>
>> + <row><entry></entry></row>
>> + <row>
>> + <entry spanname="id"><constant>V4L2_CID_MPEG_VIDEO_VPX_PROFILE</constant> </entry>
>> + <entry>integer</entry>
>
> This says 'integer' whereas the control is actually an integer menu.
>
> Why did you choose 'integer menu' for this? Would a regular integer or perhaps a standard
> menu be better?
>
I chose integer menu as it is standard set of values with only 4
options (integers).
Same thing is done in the controls - V4L2_CID_MPEG_VIDEO_VPX_NUM_PARTITIONS
and V4L2_CID_MPEG_VIDEO_VPX_NUM_REF_FRAMES. I felt this new controls is also
in-line with the requirement of a integer-menu type. What do you think?
>> + </row>
>> + <row><entry spanname="descr">Select the desired profile for VP8 encoder.
>> +Acceptable values are 0, 1, 2 and 3 corresponding to encoder versions 0, 1, 2 and 3.</entry>
>
> Is it a 'profile' or a 'version'? It looks a bit confusing. I don't have the VP8 standard,
> so I can't really tell what the correct terminology is.
>
Ok will make it more clear.
> Also, does this control apply just to VP8 or also to other VP versions? The control name
> says 'VPX' while the description says 'VP8' explicitly.
>
As of now its applicable to VP8, but I am not sure if the same thing
will apply to VP9 also.
Regards
Arun
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] CHROMIUM: s5p-mfc: add controls to set vp8 enc profile
2013-12-10 0:51 Pawel Osciak
@ 2013-12-10 4:45 ` Arun Kumar K
2013-12-10 5:08 ` Sachin Kamat
0 siblings, 1 reply; 7+ messages in thread
From: Arun Kumar K @ 2013-12-10 4:45 UTC (permalink / raw)
To: Pawel Osciak
Cc: linux-media@vger.kernel.org, linux-samsung-soc, Kamil Debski,
Hans Verkuil, avnd.kiran
Hi Pawel,
On Tue, Dec 10, 2013 at 6:21 AM, Pawel Osciak <posciak@chromium.org> wrote:
> Hi Arun,
>
> On Mon, Dec 9, 2013 at 10:16 PM, Arun Kumar K <arun.kk@samsung.com> wrote:
>> Add v4l2 controls to set desired profile for VP8 encoder.
>> Acceptable levels for VP8 encoder are
>> 0: Version 0
>> 1: Version 1
>> 2: Version 2
>> 3: Version 3
>>
>> Signed-off-by: Pawel Osciak <posciak@chromium.org>
>
> Sorry, but I'm not the author of this patch, Kiran is. I think the
> confusion comes from the fact that I committed it to Chromium tree. I
> think Kiran's sign-off should go first and the patch should contain:
> "From: Kiran AVND <avnd.kiran@samsung.com>"
>
Ok will make the change in next version.
Thanks & Regards
Arun
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] CHROMIUM: s5p-mfc: add controls to set vp8 enc profile
2013-12-10 4:45 ` Arun Kumar K
@ 2013-12-10 5:08 ` Sachin Kamat
2013-12-10 5:14 ` Arun Kumar K
0 siblings, 1 reply; 7+ messages in thread
From: Sachin Kamat @ 2013-12-10 5:08 UTC (permalink / raw)
To: Arun Kumar K
Cc: Pawel Osciak, linux-media@vger.kernel.org, linux-samsung-soc,
Kamil Debski, Hans Verkuil, avnd.kiran
Hi Arun,
On 10 December 2013 10:15, Arun Kumar K <arunkk.samsung@gmail.com> wrote:
> Hi Pawel,
>
> On Tue, Dec 10, 2013 at 6:21 AM, Pawel Osciak <posciak@chromium.org> wrote:
>> Hi Arun,
>>
>
> Ok will make the change in next version.
>
While at it also update the patch subject appropriately.
--
With warm regards,
Sachin
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] CHROMIUM: s5p-mfc: add controls to set vp8 enc profile
2013-12-10 5:08 ` Sachin Kamat
@ 2013-12-10 5:14 ` Arun Kumar K
0 siblings, 0 replies; 7+ messages in thread
From: Arun Kumar K @ 2013-12-10 5:14 UTC (permalink / raw)
To: Sachin Kamat
Cc: Pawel Osciak, linux-media@vger.kernel.org, linux-samsung-soc,
Kamil Debski, Hans Verkuil, avnd.kiran
Hi Sachin,
On Tue, Dec 10, 2013 at 10:38 AM, Sachin Kamat <sachin.kamat@linaro.org> wrote:
> Hi Arun,
>
> On 10 December 2013 10:15, Arun Kumar K <arunkk.samsung@gmail.com> wrote:
>> Hi Pawel,
>>
>> On Tue, Dec 10, 2013 at 6:21 AM, Pawel Osciak <posciak@chromium.org> wrote:
>>> Hi Arun,
>>>
>
>>
>> Ok will make the change in next version.
>>
>
> While at it also update the patch subject appropriately.
>
Oh I totally missed that. Thanks for pointing out.
Regards
Arun
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2013-12-10 5:14 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-12-09 13:16 [PATCH] CHROMIUM: s5p-mfc: add controls to set vp8 enc profile Arun Kumar K
2013-12-09 13:22 ` Hans Verkuil
2013-12-10 4:44 ` Arun Kumar K
-- strict thread matches above, loose matches on Subject: below --
2013-12-10 0:51 Pawel Osciak
2013-12-10 4:45 ` Arun Kumar K
2013-12-10 5:08 ` Sachin Kamat
2013-12-10 5:14 ` Arun Kumar K
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox