From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755461AbdDGI3w (ORCPT ); Fri, 7 Apr 2017 04:29:52 -0400 Received: from mailout1.samsung.com ([203.254.224.24]:33730 "EHLO epoutp01.samsung.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1754338AbdDGI3r (ORCPT ); Fri, 7 Apr 2017 04:29:47 -0400 X-AuditID: b6c32a2d-f792a6d0000055a1-42-58e74df92710 Subject: Re: [Patch v4 10/12] [media] v4l2: Add v4l2 control IDs for HEVC encoder From: Smitha T Murthy To: Sylwester Nawrocki Cc: linux-media@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, a.hajda@samsung.com, pankaj.dubey@samsung.com, kamil@wypas.org, krzk@kernel.org, jtp.park@samsung.com, kyungmin.park@samsung.com, mchehab@kernel.org, m.szyprowski@samsung.com In-Reply-To: <374939c7-241a-fcca-c87e-5c4290bdb6aa@samsung.com> Date: Fri, 07 Apr 2017 14:01:34 +0530 Message-ID: <1491553894.15698.1142.camel@smitha-fedora> Mime-Version: 1.0 X-Mailer: Evolution 2.28.3 (2.28.3-1.fc12) Content-Transfer-Encoding: 7bit X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFtrIKsWRmVeSWpSXmKPExsWy7bCmhu5P3+cRBpc38lvcWneO1eLI2qtM FjNPtLNanD+/gd3ibNMbdotNj6+xWlzeNYfNomfDVlaLtUfuslss2/SHyWLR1i/sFofftLM6 8HhsWtXJ5rF5Sb1H35ZVjB6fN8l5XDnSyB7AGsVlk5Kak1mWWqRvl8CVsbDhIEvBU9mKy0uv sDcwTpHsYuTgkBAwkejdVNbFyAlkiklcuLeerYuRi0NIYCmjxKFNr1kgnE+MEpMf3mGHc15c amOFaDGRuHL+LDNEYiejxLY3vWwgCSGBt4wSb68YgtjCAsESqzoawBrYBHQkvr0/DVYjIqAv sWTVRTCbWWAWk8T6bicQm1PAXuLcoz8sIDaLgKrEiY7PYL28AuYSJ5etZQM5m1dAUOLvDmGI G7Ql1jWcYYEYIy+x/e0csHskBH6zSRze950F4k0XicmP+SHqhSVeHd/CDmFLSXx+t5cNor6Z UWLutmYWiMQERomVraUQtr3EgStzwOYwC2hKrN+lDxG2lXh88jI7xHg+iRtvBSFO4JPo/f2E CaJESWLRmRNQJbwSHW1CEGEPiRvNZ1knMCrNQvhlFpL7ZyGsWsDIvIpRLLWgODc9tdi0wEiv ODG3uDQvXS85P3cTIzghaenuYPyywPsQowAHoxIPbwbP8wgh1sSy4srcQ4wSHMxKIrzmbkAh 3pTEyqrUovz4otKc1OJDjNIcLErivOorr0UICaQnlqRmp6YWpBbBZJk4OKUaGOUD1iz6HjIp ZUFD4fkOp3XJrNyXXKT+q+WKFhu35kTPerz5FNtVrp0tmqH3ojpmi1iFa23OPa9zwerYenO2 fWuidG0++E+bvzVn9R5F71XHJi8TTJ4S5DdLaqn2uQlZ9g5Lwr87TrsX9XGZJcvWDyvmc+r7 hKSzsWodOFuxd6Nv2bYn/VkfC5RYijMSDbWYi4oTAczaUj9EAwAA X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFuphkeLIzCtJLcpLzFFi42LZdlhJTnez7/MIg10d1ha31p1jtTiy9iqT xcwT7awW589vYLc42/SG3WLT42usFpd3zWGz6NmwldVi7ZG77BbLNv1hsli09Qu7xeE37awO PB6bVnWyeWxeUu/Rt2UVo8fnTXIeV440sgewRnHZpKTmZJalFunbJXBlLGw4yFLwVLbi8tIr 7A2MUyS7GDk5JARMJK6cP8sMYYtJXLi3nq2LkYtDSGA7o0RXw052iISKxOQn99kgbGGJlf+e s0MUvWaU+PRvI1hCWCBYYlVHAyuIzSagI/Ht/WmwuIiAvsSSVRfBpjILTGOS+PBtK1gRp4C9 xLlHf1ggJvUwScxc/5AJJMEsoCnRuv032GoWAVWJEx2fwRp4BcwlTi5bCzSJA8gWlPi7Qxji Im2JdQ1nWCBa5SW2v53DPIFRaBaSSbMQOmYhqVrAyLyKUTS1oDg3Pbe4wEivODG3uDQvXS85 P3cTIziOtJJ2MG6aEX6IUYCDUYmHd8KTZxFCrIllxZW5hxglOJiVRHjN3Z5HCPGmJFZWpRbl xxeV5qQWH2KU5mBREufdXr0hQkggPbEkNTs1tSC1CCbLxMEp1cAYGV2guiAx892RAobzGs+V jJQX3cr/7GTCcD8xr+2T/93eWOtIOweztu8fi2Omr92zPODVK6tzgrmxwQaT+mubqnq0Yx5+ qVyb7qEresbodQfPlPNzr/ILPxCynXRKK9yzgnOKlvnqs09kLTwsnh1teb4i6M8CsbrPdqu3 tpqwfFi5rqGtYq0SS3FGoqEWc1FxIgD2YfM1nwIAAA== X-CMS-MailID: 20170407082944epcas5p35377f097691e898848ccd4e407dae4dd X-Msg-Generator: CA X-Sender-IP: 182.195.40.14 X-Local-Sender: =?UTF-8?B?U21pdGhhIFQgTXVydGh5G1NTSVItVHVybiBLZXkgU29sdXRp?= =?UTF-8?B?b25zG+yCvOyEseyghOyekBtMZWFkIEVuZ2luZWVy?= X-Global-Sender: =?UTF-8?B?U21pdGhhIFQgTXVydGh5G1NTSVItVHVybiBLZXkgU29sdXRp?= =?UTF-8?B?b25zG1NhbXN1bmcgRWxlY3Ryb25pY3MbTGVhZCBFbmdpbmVlcg==?= X-Sender-Code: =?UTF-8?B?QzEwG1NXQUhRG0MxMElEMDdJRDAxMDk5Nw==?= Content-Type: text/plain; charset="utf-8" X-MTR: 20170407082944epcas5p35377f097691e898848ccd4e407dae4dd X-EPHeader: CA CMS-TYPE: 105P X-CMS-RootMailID: 20170406061023epcas5p2a3fa65c4254e17a58f71c68d413e6bfd X-RootMTR: 20170406061023epcas5p2a3fa65c4254e17a58f71c68d413e6bfd References: <1491459105-16641-1-git-send-email-smitha.t@samsung.com> <1491459105-16641-11-git-send-email-smitha.t@samsung.com> <374939c7-241a-fcca-c87e-5c4290bdb6aa@samsung.com> Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, 2017-04-06 at 15:14 +0200, Sylwester Nawrocki wrote: > On 04/06/2017 08:11 AM, Smitha T Murthy wrote: > > @@ -775,6 +832,47 @@ 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"; > > > > + /* HEVC controls */ > [...] > > + case V4L2_CID_MPEG_VIDEO_HEVC_LF_SLICE_BOUNDARY: return "HEVC LF Across Slice Boundary or Not"; > > Please make sure the names are no longer than 31 characters to avoid > truncation during control enumeration in user space. > Data structures like struct v4l2_queryctrl, struct v4l2_query_ext_ctrl > have only 32 bytes long array dedicated for the control name. I will try to make the names less than 31 characters long without losing the context. But there are many control names in this file which are longer than 31 characters like V4L2_CID_MPEG_VIDEO_H264_HIERARCHICAL_CODING_LAYER_QP, V4L2_CID_MPEG_VIDEO_VPX_GOLDEN_FRAME_REF_PERIOD etc so I assumed it was alright to have such long names. But I will shorten them as per your suggestion. > > > + case V4L2_CID_MPEG_VIDEO_HEVC_HIERARCHICAL_QP: return "HEVC QP Values"; > > > + case V4L2_CID_MPEG_VIDEO_HEVC_HIERARCHICAL_CODING_TYPE: return "HEVC Hierarchical Coding Type"; > > + case V4L2_CID_MPEG_VIDEO_HEVC_HIERARCHICAL_CODING_LAYER:return "HEVC Hierarchical Coding Layer"; > > + case V4L2_CID_MPEG_VIDEO_HEVC_HIERARCHICAL_CODING_LAYER_QP:return "HEVC Hierarchical Layer QP"; > > How about s/HIERARCHICAL_/HIER_ for the above 3 control IDs? > Ok I will change it. > > + case V4L2_CID_MPEG_VIDEO_HEVC_HIER_CODING_LAYER0_BITRATE:return "HEVC Hierarchical Lay 0 Bit Rate"; > > + case V4L2_CID_MPEG_VIDEO_HEVC_HIER_CODING_LAYER1_BITRATE:return "HEVC Hierarchical Lay 1 Bit Rate"; > > + case V4L2_CID_MPEG_VIDEO_HEVC_HIER_CODING_LAYER2_BITRATE:return "HEVC Hierarchical Lay 2 Bit Rate"; > > + case V4L2_CID_MPEG_VIDEO_HEVC_HIER_CODING_LAYER3_BITRATE:return "HEVC Hierarchical Lay 3 Bit Rate"; > > + case V4L2_CID_MPEG_VIDEO_HEVC_HIER_CODING_LAYER4_BITRATE:return "HEVC Hierarchical Lay 4 Bit Rate"; > > + case V4L2_CID_MPEG_VIDEO_HEVC_HIER_CODING_LAYER5_BITRATE:return "HEVC Hierarchical Lay 5 Bit Rate"; > > + case V4L2_CID_MPEG_VIDEO_HEVC_HIER_CODING_LAYER6_BITRATE:return "HEVC Hierarchical Lay 6 Bit Rate"; > > Using single letter L instead of LAYER would make the control ID shorter > and more consistent with existing controls, e.g. > V4L2_CID_MPEG_VIDEO_HEVC_HIER_CODING_L6_BITRATE. > Ok I will change it. > > + case V4L2_CID_MPEG_VIDEO_HEVC_SIGN_DATA_HIDING: return "HEVC Sign Data Hiding"; > > + case V4L2_CID_MPEG_VIDEO_HEVC_GENERAL_PB: return "HEVC General PB"; > > + case V4L2_CID_MPEG_VIDEO_HEVC_TEMPORAL_ID: return "HEVC Temporal ID"; > > + case V4L2_CID_MPEG_VIDEO_HEVC_STRONG_SMOOTHING: return "HEVC Strong Intra Smoothing"; > > + case V4L2_CID_MPEG_VIDEO_HEVC_INTRA_PU_SPLIT: return "HEVC Intra PU Split"; > > + case V4L2_CID_MPEG_VIDEO_HEVC_TMV_PREDICTION: return "HEVC TMV Prediction"; > > + case V4L2_CID_MPEG_VIDEO_HEVC_MAX_NUM_MERGE_MV_MINUS1: return "HEVC Max Number of Candidate MVs"; > > + case V4L2_CID_MPEG_VIDEO_HEVC_WITHOUT_STARTCODE: return "HEVC ENC Without Startcode"; > > + case V4L2_CID_MPEG_VIDEO_HEVC_REFRESH_PERIOD: return "HEVC Num of I Frame b/w 2 IDR"; > > + case V4L2_CID_MPEG_VIDEO_HEVC_LF_BETA_OFFSET_DIV2: return "HEVC Loop Filter Beta Offset"; > > + case V4L2_CID_MPEG_VIDEO_HEVC_LF_TC_OFFSET_DIV2: return "HEVC Loop Filter tc Offset"; > > s/tc/Tc or s/tc/TC ? > I will correct it. > > + case V4L2_CID_MPEG_VIDEO_HEVC_SIZE_OF_LENGTH_FIELD: return "HEVC Size of Length Field"; > > -- > Thanks, > Sylwester > Thank you for the review. Regards, Smitha From mboxrd@z Thu Jan 1 00:00:00 1970 From: smitha.t@samsung.com (Smitha T Murthy) Date: Fri, 07 Apr 2017 14:01:34 +0530 Subject: [Patch v4 10/12] [media] v4l2: Add v4l2 control IDs for HEVC encoder In-Reply-To: <374939c7-241a-fcca-c87e-5c4290bdb6aa@samsung.com> References: <1491459105-16641-1-git-send-email-smitha.t@samsung.com> <1491459105-16641-11-git-send-email-smitha.t@samsung.com> <374939c7-241a-fcca-c87e-5c4290bdb6aa@samsung.com> Message-ID: <1491553894.15698.1142.camel@smitha-fedora> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On Thu, 2017-04-06 at 15:14 +0200, Sylwester Nawrocki wrote: > On 04/06/2017 08:11 AM, Smitha T Murthy wrote: > > @@ -775,6 +832,47 @@ 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"; > > > > + /* HEVC controls */ > [...] > > + case V4L2_CID_MPEG_VIDEO_HEVC_LF_SLICE_BOUNDARY: return "HEVC LF Across Slice Boundary or Not"; > > Please make sure the names are no longer than 31 characters to avoid > truncation during control enumeration in user space. > Data structures like struct v4l2_queryctrl, struct v4l2_query_ext_ctrl > have only 32 bytes long array dedicated for the control name. I will try to make the names less than 31 characters long without losing the context. But there are many control names in this file which are longer than 31 characters like V4L2_CID_MPEG_VIDEO_H264_HIERARCHICAL_CODING_LAYER_QP, V4L2_CID_MPEG_VIDEO_VPX_GOLDEN_FRAME_REF_PERIOD etc so I assumed it was alright to have such long names. But I will shorten them as per your suggestion. > > > + case V4L2_CID_MPEG_VIDEO_HEVC_HIERARCHICAL_QP: return "HEVC QP Values"; > > > + case V4L2_CID_MPEG_VIDEO_HEVC_HIERARCHICAL_CODING_TYPE: return "HEVC Hierarchical Coding Type"; > > + case V4L2_CID_MPEG_VIDEO_HEVC_HIERARCHICAL_CODING_LAYER:return "HEVC Hierarchical Coding Layer"; > > + case V4L2_CID_MPEG_VIDEO_HEVC_HIERARCHICAL_CODING_LAYER_QP:return "HEVC Hierarchical Layer QP"; > > How about s/HIERARCHICAL_/HIER_ for the above 3 control IDs? > Ok I will change it. > > + case V4L2_CID_MPEG_VIDEO_HEVC_HIER_CODING_LAYER0_BITRATE:return "HEVC Hierarchical Lay 0 Bit Rate"; > > + case V4L2_CID_MPEG_VIDEO_HEVC_HIER_CODING_LAYER1_BITRATE:return "HEVC Hierarchical Lay 1 Bit Rate"; > > + case V4L2_CID_MPEG_VIDEO_HEVC_HIER_CODING_LAYER2_BITRATE:return "HEVC Hierarchical Lay 2 Bit Rate"; > > + case V4L2_CID_MPEG_VIDEO_HEVC_HIER_CODING_LAYER3_BITRATE:return "HEVC Hierarchical Lay 3 Bit Rate"; > > + case V4L2_CID_MPEG_VIDEO_HEVC_HIER_CODING_LAYER4_BITRATE:return "HEVC Hierarchical Lay 4 Bit Rate"; > > + case V4L2_CID_MPEG_VIDEO_HEVC_HIER_CODING_LAYER5_BITRATE:return "HEVC Hierarchical Lay 5 Bit Rate"; > > + case V4L2_CID_MPEG_VIDEO_HEVC_HIER_CODING_LAYER6_BITRATE:return "HEVC Hierarchical Lay 6 Bit Rate"; > > Using single letter L instead of LAYER would make the control ID shorter > and more consistent with existing controls, e.g. > V4L2_CID_MPEG_VIDEO_HEVC_HIER_CODING_L6_BITRATE. > Ok I will change it. > > + case V4L2_CID_MPEG_VIDEO_HEVC_SIGN_DATA_HIDING: return "HEVC Sign Data Hiding"; > > + case V4L2_CID_MPEG_VIDEO_HEVC_GENERAL_PB: return "HEVC General PB"; > > + case V4L2_CID_MPEG_VIDEO_HEVC_TEMPORAL_ID: return "HEVC Temporal ID"; > > + case V4L2_CID_MPEG_VIDEO_HEVC_STRONG_SMOOTHING: return "HEVC Strong Intra Smoothing"; > > + case V4L2_CID_MPEG_VIDEO_HEVC_INTRA_PU_SPLIT: return "HEVC Intra PU Split"; > > + case V4L2_CID_MPEG_VIDEO_HEVC_TMV_PREDICTION: return "HEVC TMV Prediction"; > > + case V4L2_CID_MPEG_VIDEO_HEVC_MAX_NUM_MERGE_MV_MINUS1: return "HEVC Max Number of Candidate MVs"; > > + case V4L2_CID_MPEG_VIDEO_HEVC_WITHOUT_STARTCODE: return "HEVC ENC Without Startcode"; > > + case V4L2_CID_MPEG_VIDEO_HEVC_REFRESH_PERIOD: return "HEVC Num of I Frame b/w 2 IDR"; > > + case V4L2_CID_MPEG_VIDEO_HEVC_LF_BETA_OFFSET_DIV2: return "HEVC Loop Filter Beta Offset"; > > + case V4L2_CID_MPEG_VIDEO_HEVC_LF_TC_OFFSET_DIV2: return "HEVC Loop Filter tc Offset"; > > s/tc/Tc or s/tc/TC ? > I will correct it. > > + case V4L2_CID_MPEG_VIDEO_HEVC_SIZE_OF_LENGTH_FIELD: return "HEVC Size of Length Field"; > > -- > Thanks, > Sylwester > Thank you for the review. Regards, Smitha