linux-kernel - Re: [RFC] media: uapi: Move HEVC stateless controls out of staging

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <89812c02-d316-d561-6e6c-543a92c5bc50@collabora.com>
Date:   Mon, 14 Feb 2022 18:26:07 +0100
From:   Benjamin Gaignard <benjamin.gaignard@...labora.com>
To:     Hans Verkuil <hverkuil-cisco@...all.nl>,
        Jernej Škrabec <jernej.skrabec@...il.com>,
        mchehab@...nel.org, ezequiel@...guardiasur.com.ar,
        p.zabel@...gutronix.de, gregkh@...uxfoundation.org,
        mripard@...nel.org, paul.kocialkowski@...tlin.com, wens@...e.org,
        jonas@...boo.se, nicolas@...fresne.ca
Cc:     linux-media@...r.kernel.org, linux-kernel@...r.kernel.org,
        linux-staging@...ts.linux.dev,
        linux-arm-kernel@...ts.infradead.org, linux-sunxi@...ts.linux.dev,
        kernel@...labora.com, Alex Bee <knaerzche@...il.com>,
        jc@...esim.co.uk
Subject: Re: [RFC] media: uapi: Move HEVC stateless controls out of staging


Le 14/02/2022 à 11:01, Hans Verkuil a écrit :
> Hi all,
>
> On 2/13/22 12:33, Jernej Škrabec wrote:
>> Hi Benjamin,
>>
>> CC: Alex, John
>>
>> Sorry for late response, but I've been very busy last week.
>>
>> First of all, thank you for doing this! It's about time that HEVC moves
>> forward.
>>
>> Dne torek, 01. februar 2022 ob 13:34:39 CET je Benjamin Gaignard napisal(a):
>>> The HEVC stateless 'uAPI' was staging and marked explicitly in the
>>> V4L2 specification that it will change and is unstable.
>>>
>>> Note that these control IDs were never exported as a public API,
>>> they were only defined in kernel-local headers (hevc-ctrls.h).
>>>
>>> While moving the controls out of staging they are renamed and
>>> control IDs get new numbers.
>>> Drivers (Hantro, Cedrus) and Documentation are updated accordaly.
>> accordaly -> accordingly
>>
>>> Additional structures fields has been added for RKVDEC driver usage.
>> You should do separate patch for that, preceding this one. One patch should
>> only do one thing.
>>
>> I also suggest that you add additional patch for removing bit_size field in
>> struct v4l2_ctrl_hevc_slice_params. Similar fields were already removed from
>> MPEG2 and H264 structures. Bit size can be deduced from output buffer size and
>> it doesn't hurt if bit size in Cedrus is set to bigger value than actual slice
>> bit size.
>>
>>> Hantro dedicated control is moving to hantro-media.h
>>> Since hevc-ctrls.h content has been dispatched in others file, remove it.
>>>
>>> fluster tests results on IMX8MQ is 77/147 for HEVC codec.
>>>
>>> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@...labora.com>
>> Note that Cedrus still needs additional information in order to decode some
>> HEVC videos. Missing info is num_entry_point_offsets and list of all
>> entry_point_offset_minus1 (obviously, num_entry_point_offsets in size).
>>
>> I suggest that this is represented in a new control, which would use dynamic
>> array feature, written by Hans. While Cedrus supports max. 256 entries, it can
>> be much bigger in theory, but in reality, it's much smaller (like 4-8
>> entries).
> I've rebased my dynarray tree, so it is up to date again:
>
> https://git.linuxtv.org/hverkuil/media_tree.git/log/?h=dynarray

I will cheery-pick the 3 needed patches in my next version of the RFC

Thanks,
Benjamin

>
> Regards,
>
> 	Hans
>
>> Last but not least, data_bit_offset should be better defined. Currently it
>> points right after last header bit, just like Cedrus needs it. However, there
>> is padding after that, at least 1 bit and 8 bits at most, so slice data always
>> starts from byte aligned address. It probably make sense to rework that field
>> to be byte offset, not bit, just like in VA-API. Note that RPi HEVC driver also
>> uses byte aligned address directly. Cedrus would need some kind of workaround
>> and only one that works is this one:
>> https://github.com/bootlin/libva-v4l2-request/blob/master/src/h265.c#L191-L209
>>
>> Best regards,
>> Jernej
>>
>>