From: Smitha T Murthy <smitha.t@samsung.com> To: Sylwester Nawrocki <s.nawrocki@samsung.com> 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 Subject: Re: [Patch v4 10/12] [media] v4l2: Add v4l2 control IDs for HEVC encoder Date: Fri, 07 Apr 2017 14:01:34 +0530 [thread overview] Message-ID: <1491553894.15698.1142.camel@smitha-fedora> (raw) In-Reply-To: <374939c7-241a-fcca-c87e-5c4290bdb6aa@samsung.com> 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
WARNING: multiple messages have this Message-ID (diff)
From: smitha.t@samsung.com (Smitha T Murthy) To: linux-arm-kernel@lists.infradead.org Subject: [Patch v4 10/12] [media] v4l2: Add v4l2 control IDs for HEVC encoder Date: Fri, 07 Apr 2017 14:01:34 +0530 [thread overview] Message-ID: <1491553894.15698.1142.camel@smitha-fedora> (raw) In-Reply-To: <374939c7-241a-fcca-c87e-5c4290bdb6aa@samsung.com> 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
next prev parent reply other threads:[~2017-04-07 8:29 UTC|newest] Thread overview: 50+ messages / expand[flat|nested] mbox.gz Atom feed top [not found] <CGME20170406060957epcas1p36f883512ccfaf24359d1b31a6d199d87@epcas1p3.samsung.com> 2017-04-06 6:11 ` [Patch v4 00/12] Add MFC v10.10 support Smitha T Murthy 2017-04-06 6:11 ` Smitha T Murthy [not found] ` <CGME20170406060959epcas1p3dbc6a99eca8f84dfc16da8e288615faa@epcas1p3.samsung.com> 2017-04-06 6:11 ` [Patch v4 01/12] [media] s5p-mfc: Rename IS_MFCV8 macro Smitha T Murthy 2017-04-06 6:11 ` Smitha T Murthy [not found] ` <CGME20170406061002epcas5p25eecf7a82218d8affcd1938ff4a9a97b@epcas5p2.samsung.com> 2017-04-06 6:11 ` [Patch v4 02/12] [media] s5p-mfc: Adding initial support for MFC v10.10 Smitha T Murthy 2017-04-06 6:11 ` Smitha T Murthy [not found] ` <CGME20170406061004epcas5p2737089d747713898c1737ba939945e31@epcas5p2.samsung.com> 2017-04-06 6:11 ` [Patch v4 03/12] [media] s5p-mfc: Use min scratch buffer size as provided by F/W Smitha T Murthy 2017-04-06 6:11 ` Smitha T Murthy [not found] ` <CGME20170406061007epcas5p2e1dccf4e2e13ac002ac0b6bfa9d9695c@epcas5p2.samsung.com> 2017-04-06 6:11 ` [Patch v4 04/12] [media] s5p-mfc: Support MFCv10.10 buffer requirements Smitha T Murthy 2017-04-06 6:11 ` Smitha T Murthy [not found] ` <CGME20170406061010epcas5p22e8100bbd1456007fb4fff327d1f81c8@epcas5p2.samsung.com> 2017-04-06 6:11 ` [Patch v4 05/12] [media] videodev2.h: Add v4l2 definition for HEVC Smitha T Murthy 2017-04-06 6:11 ` Smitha T Murthy [not found] ` <CGME20170406061013epcas5p214b60d10e809974bcd0283319a66048e@epcas5p2.samsung.com> 2017-04-06 6:11 ` [Patch v4 06/12] [media] v4l2-ioctl: add HEVC format description Smitha T Murthy 2017-04-06 6:11 ` Smitha T Murthy [not found] ` <CGME20170406061015epcas1p3488865b9f19fee1a799867106729307f@epcas1p3.samsung.com> 2017-04-06 6:11 ` [Patch v4 07/12] Documentation: v4l: Documentation for HEVC v4l2 definition Smitha T Murthy 2017-04-06 6:11 ` Smitha T Murthy [not found] ` <CGME20170406061018epcas1p373b834b21fc7a39759c8424106215eb5@epcas1p3.samsung.com> 2017-04-06 6:11 ` [Patch v4 08/12] [media] s5p-mfc: Add support for HEVC decoder Smitha T Murthy 2017-04-06 6:11 ` Smitha T Murthy [not found] ` <CGME20170406061021epcas5p25af9b201e7e990bd7c2ade8ec765655a@epcas5p2.samsung.com> 2017-04-06 6:11 ` [Patch v4 09/12] [media] s5p-mfc: Add VP9 decoder support Smitha T Murthy 2017-04-06 6:11 ` Smitha T Murthy [not found] ` <CGME20170406061023epcas5p2a3fa65c4254e17a58f71c68d413e6bfd@epcas5p2.samsung.com> 2017-04-06 6:11 ` [Patch v4 10/12] [media] v4l2: Add v4l2 control IDs for HEVC encoder Smitha T Murthy 2017-04-06 6:11 ` Smitha T Murthy 2017-04-06 13:14 ` Sylwester Nawrocki 2017-04-06 13:14 ` Sylwester Nawrocki 2017-04-07 8:31 ` Smitha T Murthy [this message] 2017-04-07 8:31 ` Smitha T Murthy 2017-04-07 9:25 ` Sylwester Nawrocki 2017-04-07 9:25 ` Sylwester Nawrocki 2017-04-07 9:54 ` Smitha T Murthy 2017-04-07 9:54 ` Smitha T Murthy [not found] ` <CGME20170406061025epcas1p3d1124e640df01d2e8a28e12138371b2b@epcas1p3.samsung.com> 2017-04-06 6:11 ` [Patch v4 11/12] [media] s5p-mfc: Add support " Smitha T Murthy 2017-04-06 6:11 ` Smitha T Murthy [not found] ` <CGME20170406061027epcas5p2628e0a8e0fd76e2e267fad3ea1209f65@epcas5p2.samsung.com> 2017-04-06 6:11 ` [Patch v4 12/12] Documention: v4l: Documentation for HEVC CIDs Smitha T Murthy 2017-04-06 6:11 ` Smitha T Murthy 2017-04-06 13:33 ` Hans Verkuil 2017-04-06 13:33 ` Hans Verkuil 2017-04-07 8:37 ` Smitha T Murthy 2017-04-07 8:37 ` Smitha T Murthy 2017-04-06 14:47 ` Sylwester Nawrocki 2017-04-06 14:47 ` Sylwester Nawrocki 2017-04-07 10:03 ` Smitha T Murthy 2017-04-07 10:03 ` Smitha T Murthy 2017-04-07 10:26 ` Sylwester Nawrocki 2017-04-07 10:26 ` Sylwester Nawrocki 2017-04-07 12:08 ` Smitha T Murthy 2017-04-07 12:08 ` Smitha T Murthy 2017-06-09 17:36 ` [Patch v4 00/12] Add MFC v10.10 support Sylwester Nawrocki 2017-06-09 17:36 ` Sylwester Nawrocki 2017-06-13 5:24 ` Smitha T Murthy 2017-06-13 5:24 ` Smitha T Murthy
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=1491553894.15698.1142.camel@smitha-fedora \ --to=smitha.t@samsung.com \ --cc=a.hajda@samsung.com \ --cc=jtp.park@samsung.com \ --cc=kamil@wypas.org \ --cc=krzk@kernel.org \ --cc=kyungmin.park@samsung.com \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-media@vger.kernel.org \ --cc=m.szyprowski@samsung.com \ --cc=mchehab@kernel.org \ --cc=pankaj.dubey@samsung.com \ --cc=s.nawrocki@samsung.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
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.