linux-kernel - Re: [PATCH v4 8/9] media: hevc: Add scaling matrix control

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Thu, 15 Jul 2021 11:43:44 +0200
From:   Benjamin Gaignard <benjamin.gaignard@...labora.com>
To:     Ezequiel Garcia <ezequiel@...labora.com>, hverkuil@...all.nl,
        p.zabel@...gutronix.de, mchehab@...nel.org, shawnguo@...nel.org,
        s.hauer@...gutronix.de, festevam@...il.com,
        gregkh@...uxfoundation.org, mripard@...nel.org,
        paul.kocialkowski@...tlin.com, wens@...e.org,
        jernej.skrabec@...l.net, emil.l.velikov@...il.com,
        andrzej.p@...labora.com, jc@...esim.co.uk,
        jernej.skrabec@...il.com, nicolas@...fresne.ca, cphealy@...il.com
Cc:     kernel@...gutronix.de, linux-imx@....com,
        linux-media@...r.kernel.org, linux-rockchip@...ts.infradead.org,
        linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v4 8/9] media: hevc: Add scaling matrix control


Le 13/07/2021 à 00:21, Ezequiel Garcia a écrit :
> Hi Benjamin,
>
> I believe the scaling matrix uAPI patch(es) and the support
> in Hantro G2 could be moved to its own series and maybe
> merged sooner than the rest (which may need more discussion).
>
> Couple remarks below.
>   
> On Fri, 2021-06-25 at 16:11 +0200, Benjamin Gaignard wrote:
>> HEVC scaling lists are used for the scaling process for transform
>> coefficients.
>> V4L2_HEVC_SPS_FLAG_SCALING_LIST_ENABLED has to set when they are
>> encoded in the bitstream.
>>
>> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@...labora.com>
>> ---
>> Note: V4L2_HEVC_SPS_FLAG_SCALING_LIST_ENABLED is not change by this
>> patch. There is a thread about the naming/usage of this flag here:
>> https://lore.kernel.org/linux-arm-kernel/20210610154442.806107-8-benjamin.gaignard@collabora.com/
>> but that doesn't concern the scaling matrix control by itself.
>>
> If I am reading the spec correctly, we have fields in
> SPS (scaling_list_enabled_flag, scaling_list_data_present_flag)
> and PPS (scaling_list_data_present_flag).
>
> We don't need all that, since all a driver needs to know
> is whether a scaling list is to be applied for the current frame.
>    
> Would you mind adding a patch moving the flag to the PPS?

Extract from the spec:
- scaling_list_enabled_flag equal 1 specifies that a scaling list is used for scaling process for transform coefficients.
- sps_scaling_list_data_present_flag equal to 1 specifies that the scaling_list_data( ) syntax structure is present in the SPS.
- pps_scaling_list_data_present_flag equal to 1 specifies that parameters are present in the PPS to modify the scaling lists specified by the active SPS.

For me V4L2_HEVC_SPS_FLAG_SCALING_LIST_ENABLED represent scaling_list_enabled_flag while the other are
used to build the scaling lists values. So it is good named from my point of view.

>
>> version 2:
>>   - Fix structure name in ext-ctrls-codec.rst
>>
>>   .../media/v4l/ext-ctrls-codec.rst             | 45 +++++++++++++++++++
>>   .../media/v4l/vidioc-queryctrl.rst            |  6 +++
>>   drivers/media/v4l2-core/v4l2-ctrls-core.c     |  6 +++
>>   drivers/media/v4l2-core/v4l2-ctrls-defs.c     |  4 ++
>>   include/media/hevc-ctrls.h                    | 11 +++++
>>   5 files changed, 72 insertions(+)
>>
>> diff --git a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst b/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
>> index dc096a5562cd..3865acb9e0fd 100644
>> --- a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
>> +++ b/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
>> @@ -3071,6 +3071,51 @@ enum v4l2_mpeg_video_hevc_size_of_length_field -
>>   
>>       \normalsize
>>   
>> +``V4L2_CID_MPEG_VIDEO_HEVC_SCALING_MATRIX (struct)``
>> +    Specifies the HEVC scaling matrix parameters used for the scaling process
>> +    for transform coefficients.
>> +    These matrix and parameters are defined according to :ref:`hevc`.
>> +    They are described in section 7.4.5 "Scaling list data semantics" of
>> +    the specification.
>> +
> This needs some additional documentation about the order of the lists.
> See the docs that we've added for the scaling_list_{} fields in
> V4L2_CID_STATELESS_H264_SCALING_MATRIX.

I will do dedicated patches for scaling lists feature and add that.

Benjamin

>
> Thanks!