From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751920AbdF3JIP (ORCPT ); Fri, 30 Jun 2017 05:08:15 -0400 Received: from mailout4.samsung.com ([203.254.224.34]:13952 "EHLO mailout4.samsung.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751847AbdF3JIN (ORCPT ); Fri, 30 Jun 2017 05:08:13 -0400 X-AuditID: b6c32a2c-f79e76d00000129c-5c-595614fba8d7 Subject: Re: [Patch v5 12/12] Documention: v4l: Documentation for HEVC CIDs From: Smitha T Murthy To: Kamil Debski Cc: linux-arm-kernel@lists.infradead.org, linux-media@vger.kernel.org, linux-kernel@vger.kernel.org, Kyungmin Park , jtp.park@samsung.com, a.hajda@samsung.com, mchehab@kernel.org, pankaj.dubey@samsung.com, krzk@kernel.org, Marek Szyprowski , Sylwester Nawrocki In-Reply-To: Date: Fri, 30 Jun 2017 08:54:20 +0530 Message-ID: <1498793060.22203.1034.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+NgFtrGKsWRmVeSWpSXmKPExsWy7bCmlu5vkbBIg663zBa31p1jtTiy9iqT xcwT7awW589vYLc42/SG3WLT42usFpd3zWGz6NmwldVi7ZG77BbLNv1hsli09Qu7xeE37awO PB6bVnWyeWxeUu/Rt2UVo8fnTXIeV440sgewRnHZpKTmZJalFunbJXBlzFxwg7HgpGTFq6kt rA2MO0S6GDk5JARMJNb0/GGEsMUkLtxbz9bFyMUhJLCUUWL7vkVQzidGiSv/p7LDOdtvrmCH aW9d1AaV2MkoMbH5N5TzllGiaf9UFpAqYQFviU8TbrCB2GwCOhLf3p8Gs0UElCQ2LH4FtoNZ 4A6TxLkfZ5lAEpwCgRIzz6wDu4pFQFWi5UETM4jNK2AuMe/yLiCbA8gWlPi7QxjiCm2JdQ1n wHYxC8hLbH87hxlkpoTAdzaJw993sEEUuUi0r5/EAmELS7w6vgXqBSmJl/1tUHa5xJ4/P1kg mhsYJeYeb4QGjb3EgStzWEAWMwtoSqzfpQ8RtpVYt+EnI8RiPone30+YIOJKEovOnGAHKZcQ 4JXoaBOCCHtILD83HWqio8TNw21sExgVZyF8MwvJB7MQdi1gZF7FKJZaUJybnlpsWmCoV5yY W1yal66XnJ+7iRGcmrR0djDeW+B9iFGAg1GJh3cDZ1ikEGtiWXFl7iFGCQ5mJRFe45+hkUK8 KYmVValF+fFFpTmpxYcYpTlYlMR5NVZeixASSE8sSc1OTS1ILYLJMnFwSjUwTmZ6LKboW2p4 ILBFTN6yjrU1bdaLS3/mx00R/7tyc/iVDaUHl72Y8/zEme9MyuIrJb2zNQ1tS1r6zipP37Fu qeUOBrtJEsvlHJZN5xGfdm+12tlkyZgFGh2nef9kzSvaZdLwYvvt7TtWXY1avitYbJuPpfbs t6H77T1dp0fucLZzfyRn1H8jQYmlOCPRUIu5qDgRADe6czFJAwAA X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFupikeLIzCtJLcpLzFFi42LZdlhJXneJSFikwbJGPYtb686xWhxZe5XJ YuaJdlaL8+c3sFucbXrDbrHp8TVWi8u75rBZ9GzYymqx9shddotlm/4wWSza+oXd4vCbdlYH Ho9NqzrZPDYvqffo27KK0ePzJjmPK0ca2QNYo7hsUlJzMstSi/TtErgyZi64wVhwUrLi1dQW 1gbGHSJdjJwcEgImEq2L2tghbDGJC/fWs3UxcnEICWxnlOh/cp8VIiEhsfL3JEYIW1hi5b/n YA1CAq8ZJZ4+9gOxhQW8JT5NuMEGYrMJ6Eh8e38azBYRUJLYsPgVmM0scJVJYu60MBCbUyBQ YuaZdYwQyxYySZxq6WGEKNKUaN3+G2wBi4CqRMuDJmYQm1fAXGLe5V1ANgeQLSjxd4cwxD3a EusazrBAtMpLbH87h3kCo9AsJJNmIXTMQlK1gJF5FaNoakFxbnpucYGRXnFibnFpXrpecn7u JkZwDGkl7WDcNCP8EKMAB6MSD2+EWmikEGtiWXFl7iFGCQ5mJRFe459AId6UxMqq1KL8+KLS nNTiQ4zSHCxK4rzbqzdECAmkJ5akZqemFqQWwWSZODilGhi75AuyOu8nsuwVtr6+5PH1pVyT b3Suuyj/1+HnUf0l7qqfJwrx+DgreS594934QO7NvLm5uVvXMMmmspwNDDr2RvTK1SXLJgQF bF1n8/wI27cE4wC9xSG6fu1WD32XRn7/2L9PXrh+3Vb79oQFlyyv5eUabXl2sSQ3O08h7+q5 /1MmvD9RGeCtxFKckWioxVxUnAgA5rGqPZ0CAAA= X-CMS-MailID: 20170630090810epcas5p12a3ead8738e99c80de9b46a7262027d1 X-Msg-Generator: CA X-Sender-IP: 182.195.40.13 X-Local-Sender: =?UTF-8?B?U21pdGhhIFQgTXVydGh5G1NTSVItRkRTIFNXG+yCvOyEsQ==?= =?UTF-8?B?7KCE7J6QG0xlYWQgRW5naW5lZXI=?= X-Global-Sender: =?UTF-8?B?U21pdGhhIFQgTXVydGh5G1NTSVItRkRTIFNXG1NhbXN1bmcg?= =?UTF-8?B?RWxlY3Ryb25pY3MbTGVhZCBFbmdpbmVlcg==?= X-Sender-Code: =?UTF-8?B?QzEwG1NXQUhRG0MxMElEMDdJRDAxMDk5Nw==?= Content-Type: text/plain; charset="utf-8" X-MTR: 20170630090810epcas5p12a3ead8738e99c80de9b46a7262027d1 CMS-TYPE: 105P X-CMS-RootMailID: 20170619052521epcas5p36a0bc384d10809dcfe775e6da87ed37b X-RootMTR: 20170619052521epcas5p36a0bc384d10809dcfe775e6da87ed37b References: <1497849055-26583-1-git-send-email-smitha.t@samsung.com> <1497849055-26583-13-git-send-email-smitha.t@samsung.com> Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, 2017-06-28 at 22:04 +0200, Kamil Debski wrote: > Hi, > > Please find my comments inline. > > On 19 June 2017 at 07:10, Smitha T Murthy wrote: > > Added V4l2 controls for HEVC encoder > > > > Signed-off-by: Smitha T Murthy > > --- > > Documentation/media/uapi/v4l/extended-controls.rst | 364 +++++++++++++++++++++ > > 1 file changed, 364 insertions(+) > > > > diff --git a/Documentation/media/uapi/v4l/extended-controls.rst b/Documentation/media/uapi/v4l/extended-controls.rst > > index abb1057..7767c70 100644 > > --- a/Documentation/media/uapi/v4l/extended-controls.rst > > +++ b/Documentation/media/uapi/v4l/extended-controls.rst > > @@ -1960,6 +1960,370 @@ enum v4l2_vp8_golden_frame_sel - > > 1, 2 and 3 corresponding to encoder profiles 0, 1, 2 and 3. > > > > > > [snip] > > > + > > + > > +``V4L2_CID_MPEG_VIDEO_HEVC_HIER_CODING_LAYER (integer)`` > > + Selects the hierarchical coding layer. In normal encoding > > + (non-hierarchial coding), it should be zero. Possible values are 0 ~ 6. > > + 0 indicates HIERARCHICAL CODING LAYER 0, 1 indicates HIERARCHICAL CODING > > + LAYER 1 and so on. > > I would like the above to be more consistent. If HIER is in the name > then HIER in the description should be used as well. Aside from that, > I would recommend using full HIERARCHICAL instead of HIER in the name > of the control. Why? Because it is HIERARCHICAL in controls already > present in V4L2, such as > V4L2_CID_MPEG_VIDEO_H264_HIERARCHICAL_CODING_LAYER_QP. > I had changed it from HIERARCHICAL to HIER as per suggestion by Sylwester Nawrocki. Here https://patchwork.kernel.org/patch/9666129/ > [snip] > > > + > > +``V4L2_CID_MPEG_VIDEO_HEVC_LF (boolean)`` > > + Indicates loop filtering. Control value 1 indicates loop filtering > > + is enabled and when set to 0 indicates loop filtering is disabled. > > + > > +``V4L2_CID_MPEG_VIDEO_HEVC_LF_SLICE_BOUNDARY (boolean)`` > > + Selects whether to apply the loop filter across the slice boundary or not. > > + If the value is 0, loop filter will not be applied across the slice boundary. > > + If the value is 1, loop filter will be applied across the slice boundary. > > Just a thought. Pretty much the same fucntionality is achieved via the > V4L2_CID_MPEG_VIDEO_H264_LOOP_FILTER_MODE control. It's an enum having > three states: enabled, disabled and disabled at slice boundary. Maybe > a single control could be introduced? With another legacy define for > API compatibility. Also, I don't like that controls are not consistent > between H264 and HEVC. I would opt for the enum option. > I will add enum options for the above control. > > + > > +``V4L2_CID_MPEG_VIDEO_HEVC_LF_BETA_OFFSET_DIV2 (integer)`` > > + Selects HEVC loop filter beta offset. The valid range is [-6, +6]. > > + > > +``V4L2_CID_MPEG_VIDEO_HEVC_LF_TC_OFFSET_DIV2 (integer)`` > > + Selects HEVC loop filter tc offset. The valid range is [-6, +6]. > > + > > +.. _v4l2-hevc-refresh-type: > > + > > +``V4L2_CID_MPEG_VIDEO_HEVC_REFRESH_TYPE`` > > + (enum) > > + > [snip] > > Best wishes, > Kamil > > Thank you for the review. Regards, Smitha