[1/7] venus : Add default values for the control V4L2_CID_COLORIMETRY_HDR10_MASTERING_DISPLAY
| Message ID | 20220712122347.6781-1-quic_vboma@quicinc.com (mailing list archive) |
|---|---|
| State | New |
| Delegated to: | Stanimir Varbanov |
| Headers |
Received: from vger.kernel.org ([23.128.96.18])
by www.linuxtv.org with esmtp (Exim 4.92)
(envelope-from <linux-media-owner@vger.kernel.org>)
id 1oBEw7-007fDm-Ag; Tue, 12 Jul 2022 12:24:19 +0000
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
id S229829AbiGLMYQ (ORCPT <rfc822;mkrufky@linuxtv.org> + 1 other);
Tue, 12 Jul 2022 08:24:16 -0400
Received: from lindbergh.monkeyblade.net ([23.128.96.19]:40602 "EHLO
lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
with ESMTP id S232693AbiGLMYP (ORCPT
<rfc822;linux-media@vger.kernel.org>);
Tue, 12 Jul 2022 08:24:15 -0400
Received: from alexa-out.qualcomm.com (alexa-out.qualcomm.com [129.46.98.28])
by lindbergh.monkeyblade.net (Postfix) with ESMTPS id D42D8A5E4B;
Tue, 12 Jul 2022 05:24:14 -0700 (PDT)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
d=quicinc.com; i=@quicinc.com; q=dns/txt; s=qcdkim;
t=1657628655; x=1689164655;
h=from:to:cc:subject:date:message-id;
bh=2yikxSz6ZKr6BUHyL9FxMlsxNJSMOiov0y0jJpxJzvk=;
b=rHT12QZlUxE/h+tZOj6Dd7zXCJjMU+3Wo7q2BcCLLhtyrIRyKA70bnMh
CSDtb8xjNtuqceizI5fGWGni6PI23eDdk4Y9O3OUMopn4ZL7f3OfU8P/q
LcrcWGqaQD8wmFAzfzdP7H3Po19m6d5o1forg4MmRpDQJfc/kyHRfeSL9
4=;
Received: from ironmsg09-lv.qualcomm.com ([10.47.202.153])
by alexa-out.qualcomm.com with ESMTP; 12 Jul 2022 05:24:14 -0700
X-QCInternal: smtphost
Received: from ironmsg01-blr.qualcomm.com ([10.86.208.130])
by ironmsg09-lv.qualcomm.com with ESMTP/TLS/AES256-SHA;
12 Jul 2022 05:24:13 -0700
X-QCInternal: smtphost
Received: from vboma-linux.qualcomm.com ([10.204.65.94])
by ironmsg01-blr.qualcomm.com with ESMTP; 12 Jul 2022 17:54:00 +0530
Received: by vboma-linux.qualcomm.com (Postfix, from userid 72083)
id B69F1900883; Tue, 12 Jul 2022 17:53:59 +0530 (IST)
From: Viswanath Boma <quic_vboma@quicinc.com>
To: video.upstream.external@qti.qualcomm.com,
Stanimir Varbanov <stanimir.varbanov@linaro.org>,
Andy Gross <agross@kernel.org>,
Bjorn Andersson <bjorn.andersson@linaro.org>,
Mauro Carvalho Chehab <mchehab@kernel.org>,
linux-media@vger.kernel.org, linux-arm-msm@vger.kernel.org,
linux-kernel@vger.kernel.org
Cc: Viswanath Boma <quic_vboma@quicinc.com>
Subject: [PATCH 1/7] venus : Add default values for the control
V4L2_CID_COLORIMETRY_HDR10_MASTERING_DISPLAY
Date: Tue, 12 Jul 2022 17:53:41 +0530
Message-Id: <20220712122347.6781-1-quic_vboma@quicinc.com>
X-Mailer: git-send-email 2.17.1
X-Spam-Status: No, score=-4.1 required=5.0 tests=BAYES_00,DKIM_SIGNED,
DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,
RCVD_IN_DNSWL_MED,SPF_HELO_NONE,SPF_NONE,T_SCC_BODY_TEXT_LINE
autolearn=ham autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-media.vger.kernel.org>
X-Mailing-List: linux-media@vger.kernel.org
X-LSpam-Score: -2.5 (--)
X-LSpam-Report: No,
score=-2.5 required=5.0 tests=BAYES_00=-1.9,DKIM_SIGNED=0.1,DKIM_VALID=-0.1,DKIM_VALID_AU=-0.1,HEADER_FROM_DIFFERENT_DOMAINS=0.5,MAILING_LIST_MULTI=-1
autolearn=ham autolearn_force=no
|
| Series |
[1/7] venus : Add default values for the control V4L2_CID_COLORIMETRY_HDR10_MASTERING_DISPLAY
|
|
Commit Message
Viswanath Boma
July 12, 2022, 12:23 p.m. UTC
From: Stanimir Varbanov <stanimir.varbanov@linaro.org> V4l2 encoder compliance expecting default values of colormetry for the control. Change-Id: I1db0d4940b54e033d646ce39d60dc488afba8d58 Signed-off-by: Viswanath Boma <quic_vboma@quicinc.com> --- drivers/media/platform/qcom/venus/venc_ctrls.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
Comments
On Tue, Jul 12, 2022 at 05:53:41PM +0530, Viswanath Boma wrote: > From: Stanimir Varbanov <stanimir.varbanov@linaro.org> > > V4l2 encoder compliance expecting default values of colormetry for the control. > > Change-Id: I1db0d4940b54e033d646ce39d60dc488afba8d58 What does this represent here? I'm pretty sure it is meaningless to the upstream kernel, so please get rid of it. > Signed-off-by: Viswanath Boma <quic_vboma@quicinc.com> Since Stan is the original author of this and following patches, there should be a s-o-b tag from him. After that you should add yours indicating that you are carrying the patches from Stan. Also, please add a cover letter stating the purpose of this series, how it is tested, and with any other relevant information. Thanks, Mani > --- > drivers/media/platform/qcom/venus/venc_ctrls.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/drivers/media/platform/qcom/venus/venc_ctrls.c b/drivers/media/platform/qcom/venus/venc_ctrls.c > index ea5805e71c143..37ba7d97f99b2 100644 > --- a/drivers/media/platform/qcom/venus/venc_ctrls.c > +++ b/drivers/media/platform/qcom/venus/venc_ctrls.c > @@ -352,6 +352,8 @@ static const struct v4l2_ctrl_ops venc_ctrl_ops = { > int venc_ctrl_init(struct venus_inst *inst) > { > int ret; > + struct v4l2_ctrl_hdr10_mastering_display p_hdr10_mastering = { {34000, 13250, 7500 }, > + { 16000, 34500, 3000 }, 15635, 16450, 10000000, 500 }; > > ret = v4l2_ctrl_handler_init(&inst->ctrl_handler, 58); > if (ret) > @@ -580,7 +582,7 @@ int venc_ctrl_init(struct venus_inst *inst) > > v4l2_ctrl_new_std_compound(&inst->ctrl_handler, &venc_ctrl_ops, > V4L2_CID_COLORIMETRY_HDR10_MASTERING_DISPLAY, > - v4l2_ctrl_ptr_create(NULL)); > + v4l2_ctrl_ptr_create((void *)&p_hdr10_mastering)); > > v4l2_ctrl_new_std(&inst->ctrl_handler, &venc_ctrl_ops, > V4L2_CID_MPEG_VIDEO_INTRA_REFRESH_PERIOD, 0, > -- > 2.17.1 >
Le mardi 12 juillet 2022 à 17:53 +0530, Viswanath Boma a écrit : > From: Stanimir Varbanov <stanimir.varbanov@linaro.org> > > V4l2 encoder compliance expecting default values of colormetry for the control. nit: colormetry -> colorimetry > > Change-Id: I1db0d4940b54e033d646ce39d60dc488afba8d58 > Signed-off-by: Viswanath Boma <quic_vboma@quicinc.com> > --- > drivers/media/platform/qcom/venus/venc_ctrls.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/drivers/media/platform/qcom/venus/venc_ctrls.c b/drivers/media/platform/qcom/venus/venc_ctrls.c > index ea5805e71c143..37ba7d97f99b2 100644 > --- a/drivers/media/platform/qcom/venus/venc_ctrls.c > +++ b/drivers/media/platform/qcom/venus/venc_ctrls.c > @@ -352,6 +352,8 @@ static const struct v4l2_ctrl_ops venc_ctrl_ops = { > int venc_ctrl_init(struct venus_inst *inst) > { > int ret; > + struct v4l2_ctrl_hdr10_mastering_display p_hdr10_mastering = { {34000, 13250, 7500 }, > + { 16000, 34500, 3000 }, 15635, 16450, 10000000, 500 }; What is the origin of these values ? Should this be done in the control framework instead ? > > ret = v4l2_ctrl_handler_init(&inst->ctrl_handler, 58); > if (ret) > @@ -580,7 +582,7 @@ int venc_ctrl_init(struct venus_inst *inst) > > v4l2_ctrl_new_std_compound(&inst->ctrl_handler, &venc_ctrl_ops, > V4L2_CID_COLORIMETRY_HDR10_MASTERING_DISPLAY, > - v4l2_ctrl_ptr_create(NULL)); > + v4l2_ctrl_ptr_create((void *)&p_hdr10_mastering)); > > v4l2_ctrl_new_std(&inst->ctrl_handler, &venc_ctrl_ops, > V4L2_CID_MPEG_VIDEO_INTRA_REFRESH_PERIOD, 0,
-----Original Message----- From: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> Sent: Tuesday, July 12, 2022 6:05 PM To: Viswanath Boma (Temp) (QUIC) <quic_vboma@quicinc.com> Cc: video.upstream.external <video.upstream.external@qti.qualcomm.com>; stanimir.varbanov@linaro.org; Andy Gross <agross@kernel.org>; bjorn.andersson@linaro.org; Mauro Carvalho Chehab <mchehab@kernel.org>; linux-media@vger.kernel.org; linux-arm-msm@vger.kernel.org; linux-kernel@vger.kernel.org Subject: Re: [PATCH 1/7] venus : Add default values for the control V4L2_CID_COLORIMETRY_HDR10_MASTERING_DISPLAY WARNING: This email originated from outside of Qualcomm. Please be wary of any links or attachments, and do not enable macros. On Tue, Jul 12, 2022 at 05:53:41PM +0530, Viswanath Boma wrote: > From: Stanimir Varbanov <stanimir.varbanov@linaro.org> > > V4l2 encoder compliance expecting default values of colormetry for the control. > > Change-Id: I1db0d4940b54e033d646ce39d60dc488afba8d58 What does this represent here? I'm pretty sure it is meaningless to the upstream kernel, so please get rid of it. [vboma] Will ensure with next patch set > Signed-off-by: Viswanath Boma <quic_vboma@quicinc.com> Since Stan is the original author of this and following patches, there should be a s-o-b tag from him. After that you should add yours indicating that you are carrying the patches from Stan. Also, please add a cover letter stating the purpose of this series, how it is tested, and with any other relevant information. Thanks, Mani [vboma] Will ensure with next patch set > --- > drivers/media/platform/qcom/venus/venc_ctrls.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/drivers/media/platform/qcom/venus/venc_ctrls.c > b/drivers/media/platform/qcom/venus/venc_ctrls.c > index ea5805e71c143..37ba7d97f99b2 100644 > --- a/drivers/media/platform/qcom/venus/venc_ctrls.c > +++ b/drivers/media/platform/qcom/venus/venc_ctrls.c > @@ -352,6 +352,8 @@ static const struct v4l2_ctrl_ops venc_ctrl_ops = > { int venc_ctrl_init(struct venus_inst *inst) { > int ret; > + struct v4l2_ctrl_hdr10_mastering_display p_hdr10_mastering = { {34000, 13250, 7500 }, > + { 16000, 34500, 3000 }, 15635, 16450, 10000000, 500 }; > > ret = v4l2_ctrl_handler_init(&inst->ctrl_handler, 58); > if (ret) > @@ -580,7 +582,7 @@ int venc_ctrl_init(struct venus_inst *inst) > > v4l2_ctrl_new_std_compound(&inst->ctrl_handler, &venc_ctrl_ops, > V4L2_CID_COLORIMETRY_HDR10_MASTERING_DISPLAY, > - v4l2_ctrl_ptr_create(NULL)); > + v4l2_ctrl_ptr_create((void > + *)&p_hdr10_mastering)); > > v4l2_ctrl_new_std(&inst->ctrl_handler, &venc_ctrl_ops, > V4L2_CID_MPEG_VIDEO_INTRA_REFRESH_PERIOD, 0, > -- > 2.17.1 >
-----Original Message----- From: Nicolas Dufresne <nicolas@ndufresne.ca> Sent: Tuesday, July 12, 2022 7:04 PM To: Viswanath Boma (Temp) (QUIC) <quic_vboma@quicinc.com>; video.upstream.external <video.upstream.external@qti.qualcomm.com>; stanimir.varbanov@linaro.org; Andy Gross <agross@kernel.org>; bjorn.andersson@linaro.org; Mauro Carvalho Chehab <mchehab@kernel.org>; linux-media@vger.kernel.org; linux-arm-msm@vger.kernel.org; linux-kernel@vger.kernel.org Subject: Re: [PATCH 1/7] venus : Add default values for the control V4L2_CID_COLORIMETRY_HDR10_MASTERING_DISPLAY WARNING: This email originated from outside of Qualcomm. Please be wary of any links or attachments, and do not enable macros. Le mardi 12 juillet 2022 à 17:53 +0530, Viswanath Boma a écrit : > From: Stanimir Varbanov <stanimir.varbanov@linaro.org> > > V4l2 encoder compliance expecting default values of colormetry for the control. nit: colormetry -> colorimetry [vboma] Will ensure with next patch set . > > Change-Id: I1db0d4940b54e033d646ce39d60dc488afba8d58 > Signed-off-by: Viswanath Boma <quic_vboma@quicinc.com> > --- > drivers/media/platform/qcom/venus/venc_ctrls.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/drivers/media/platform/qcom/venus/venc_ctrls.c > b/drivers/media/platform/qcom/venus/venc_ctrls.c > index ea5805e71c143..37ba7d97f99b2 100644 > --- a/drivers/media/platform/qcom/venus/venc_ctrls.c > +++ b/drivers/media/platform/qcom/venus/venc_ctrls.c > @@ -352,6 +352,8 @@ static const struct v4l2_ctrl_ops venc_ctrl_ops = > { int venc_ctrl_init(struct venus_inst *inst) { > int ret; > + struct v4l2_ctrl_hdr10_mastering_display p_hdr10_mastering = { {34000, 13250, 7500 }, > + { 16000, 34500, 3000 }, 15635, 16450, 10000000, 500 }; What is the origin of these values ? Should this be done in the control framework instead ? [vboma] Control frame work handling the min/max value that required with the default control object. As per @Stanimir Varbanov (Consultant) inputs ,added these values for the default . HI , @Stanimir Varbanov (Consultant) , could you please share more info as required . > > ret = v4l2_ctrl_handler_init(&inst->ctrl_handler, 58); > if (ret) > @@ -580,7 +582,7 @@ int venc_ctrl_init(struct venus_inst *inst) > > v4l2_ctrl_new_std_compound(&inst->ctrl_handler, &venc_ctrl_ops, > V4L2_CID_COLORIMETRY_HDR10_MASTERING_DISPLAY, > - v4l2_ctrl_ptr_create(NULL)); > + v4l2_ctrl_ptr_create((void > + *)&p_hdr10_mastering)); > > v4l2_ctrl_new_std(&inst->ctrl_handler, &venc_ctrl_ops, > V4L2_CID_MPEG_VIDEO_INTRA_REFRESH_PERIOD, 0,
On 7/12/22 16:34, Nicolas Dufresne wrote: > Le mardi 12 juillet 2022 à 17:53 +0530, Viswanath Boma a écrit : >> From: Stanimir Varbanov <stanimir.varbanov@linaro.org> >> >> V4l2 encoder compliance expecting default values of colormetry for the control. > > nit: colormetry -> colorimetry > >> >> Change-Id: I1db0d4940b54e033d646ce39d60dc488afba8d58 >> Signed-off-by: Viswanath Boma <quic_vboma@quicinc.com> >> --- >> drivers/media/platform/qcom/venus/venc_ctrls.c | 4 +++- >> 1 file changed, 3 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/media/platform/qcom/venus/venc_ctrls.c b/drivers/media/platform/qcom/venus/venc_ctrls.c >> index ea5805e71c143..37ba7d97f99b2 100644 >> --- a/drivers/media/platform/qcom/venus/venc_ctrls.c >> +++ b/drivers/media/platform/qcom/venus/venc_ctrls.c >> @@ -352,6 +352,8 @@ static const struct v4l2_ctrl_ops venc_ctrl_ops = { >> int venc_ctrl_init(struct venus_inst *inst) >> { >> int ret; >> + struct v4l2_ctrl_hdr10_mastering_display p_hdr10_mastering = { {34000, 13250, 7500 }, >> + { 16000, 34500, 3000 }, 15635, 16450, 10000000, 500 }; > > What is the origin of these values ? Should this be done in the control > framework instead ? We needed some safe default values to satisfy v4l2 control. I took the values from here [1]. I'm not sure do we need them in v4l2 control core (most probably it is a good to have it), Hans ? > >> >> ret = v4l2_ctrl_handler_init(&inst->ctrl_handler, 58); >> if (ret) >> @@ -580,7 +582,7 @@ int venc_ctrl_init(struct venus_inst *inst) >> >> v4l2_ctrl_new_std_compound(&inst->ctrl_handler, &venc_ctrl_ops, >> V4L2_CID_COLORIMETRY_HDR10_MASTERING_DISPLAY, >> - v4l2_ctrl_ptr_create(NULL)); >> + v4l2_ctrl_ptr_create((void *)&p_hdr10_mastering)); >> >> v4l2_ctrl_new_std(&inst->ctrl_handler, &venc_ctrl_ops, >> V4L2_CID_MPEG_VIDEO_INTRA_REFRESH_PERIOD, 0, >
Hi Viswanath, On 7/12/22 15:23, Viswanath Boma wrote: > From: Stanimir Varbanov <stanimir.varbanov@linaro.org> In fact this comes from you, please drop 'From'. > > V4l2 encoder compliance expecting default values of colormetry for the control. > > Change-Id: I1db0d4940b54e033d646ce39d60dc488afba8d58 Drop this. > Signed-off-by: Viswanath Boma <quic_vboma@quicinc.com> > --- > drivers/media/platform/qcom/venus/venc_ctrls.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/drivers/media/platform/qcom/venus/venc_ctrls.c b/drivers/media/platform/qcom/venus/venc_ctrls.c > index ea5805e71c143..37ba7d97f99b2 100644 > --- a/drivers/media/platform/qcom/venus/venc_ctrls.c > +++ b/drivers/media/platform/qcom/venus/venc_ctrls.c > @@ -352,6 +352,8 @@ static const struct v4l2_ctrl_ops venc_ctrl_ops = { > int venc_ctrl_init(struct venus_inst *inst) > { > int ret; > + struct v4l2_ctrl_hdr10_mastering_display p_hdr10_mastering = { {34000, 13250, 7500 }, > + { 16000, 34500, 3000 }, 15635, 16450, 10000000, 500 }; Please do this with proper indentation. > > ret = v4l2_ctrl_handler_init(&inst->ctrl_handler, 58); > if (ret) > @@ -580,7 +582,7 @@ int venc_ctrl_init(struct venus_inst *inst) > > v4l2_ctrl_new_std_compound(&inst->ctrl_handler, &venc_ctrl_ops, > V4L2_CID_COLORIMETRY_HDR10_MASTERING_DISPLAY, > - v4l2_ctrl_ptr_create(NULL)); > + v4l2_ctrl_ptr_create((void *)&p_hdr10_mastering)); I think you do not need to cast to (void *). > > v4l2_ctrl_new_std(&inst->ctrl_handler, &venc_ctrl_ops, > V4L2_CID_MPEG_VIDEO_INTRA_REFRESH_PERIOD, 0,
From: Viswanath Boma <quic_vboma@quicinc.com>
Fixes for Video Encoder complaince issues .
Tested on v5.15 kernel and the chrome/client binary details -
v4l2-compliance 1.22.1
Dikshita Agarwal (1):
venus : Addition of support for VIDIOC_TRY_ENCODER_CMD
Stanimir Varbanov (2):
venus : Add default values for the control
V4L2_CID_COLORIMETRY_HDR10_MASTERING_DISPLAY
venus : CAPTURE Plane width/height alignment with OUT plane.
Vikash Garodia (1):
venus : Allow MIN/MAX settings for the v4l2 encoder controls defined
range.
Viswanath Boma (3):
venus : Addition of control support - V4L2_CID_MIN_BUFFERS_FOR_OUTPUT
venus : Addition of EOS Event support for Encoder
venus : Remove the capture plane settings for venc_g_parm/venc_s_parm
drivers/media/platform/qcom/venus/venc.c | 26 ++++++++++----
.../media/platform/qcom/venus/venc_ctrls.c | 35 ++++++++++++++++---
2 files changed, 50 insertions(+), 11 deletions(-)
diff --git a/drivers/media/platform/qcom/venus/venc_ctrls.c b/drivers/media/platform/qcom/venus/venc_ctrls.c index ea5805e71c143..37ba7d97f99b2 100644 --- a/drivers/media/platform/qcom/venus/venc_ctrls.c +++ b/drivers/media/platform/qcom/venus/venc_ctrls.c @@ -352,6 +352,8 @@ static const struct v4l2_ctrl_ops venc_ctrl_ops = { int venc_ctrl_init(struct venus_inst *inst) { int ret; + struct v4l2_ctrl_hdr10_mastering_display p_hdr10_mastering = { {34000, 13250, 7500 }, + { 16000, 34500, 3000 }, 15635, 16450, 10000000, 500 }; ret = v4l2_ctrl_handler_init(&inst->ctrl_handler, 58); if (ret) @@ -580,7 +582,7 @@ int venc_ctrl_init(struct venus_inst *inst) v4l2_ctrl_new_std_compound(&inst->ctrl_handler, &venc_ctrl_ops, V4L2_CID_COLORIMETRY_HDR10_MASTERING_DISPLAY, - v4l2_ctrl_ptr_create(NULL)); + v4l2_ctrl_ptr_create((void *)&p_hdr10_mastering)); v4l2_ctrl_new_std(&inst->ctrl_handler, &venc_ctrl_ops, V4L2_CID_MPEG_VIDEO_INTRA_REFRESH_PERIOD, 0,