From: Hans Verkuil <hverkuil-cisco@xs4all.nl>
To: "Jernej Škrabec" <jernej.skrabec@gmail.com>,
mchehab@kernel.org, ezequiel@vanguardiasur.com.ar,
p.zabel@pengutronix.de, gregkh@linuxfoundation.org,
mripard@kernel.org, paul.kocialkowski@bootlin.com, wens@csie.org,
jonas@kwiboo.se, nicolas@ndufresne.ca,
"Benjamin Gaignard" <benjamin.gaignard@collabora.com>
Cc: linux-media@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-staging@lists.linux.dev,
linux-arm-kernel@lists.infradead.org,
linux-sunxi@lists.linux.dev, kernel@collabora.com,
Alex Bee <knaerzche@gmail.com>,
jc@kynesim.co.uk
Subject: Re: [RFC] media: uapi: Move HEVC stateless controls out of staging
Date: Mon, 14 Feb 2022 11:01:36 +0100 [thread overview]
Message-ID: <713cee9b-c96c-119a-65be-3c289b361915@xs4all.nl> (raw)
In-Reply-To: <8038233.T7Z3S40VBb@kista>
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@collabora.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
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
>
>
next prev parent reply other threads:[~2022-02-14 10:38 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-02-01 12:34 [RFC] media: uapi: Move HEVC stateless controls out of staging Benjamin Gaignard
2022-02-13 11:33 ` Jernej Škrabec
2022-02-14 10:01 ` Hans Verkuil [this message]
2022-02-14 17:26 ` Benjamin Gaignard
2022-02-14 17:25 ` Benjamin Gaignard
2022-02-14 19:26 ` Jernej Škrabec
2022-02-15 14:37 ` John Cox
2022-02-15 14:51 ` Benjamin Gaignard
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=713cee9b-c96c-119a-65be-3c289b361915@xs4all.nl \
--to=hverkuil-cisco@xs4all.nl \
--cc=benjamin.gaignard@collabora.com \
--cc=ezequiel@vanguardiasur.com.ar \
--cc=gregkh@linuxfoundation.org \
--cc=jc@kynesim.co.uk \
--cc=jernej.skrabec@gmail.com \
--cc=jonas@kwiboo.se \
--cc=kernel@collabora.com \
--cc=knaerzche@gmail.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-media@vger.kernel.org \
--cc=linux-staging@lists.linux.dev \
--cc=linux-sunxi@lists.linux.dev \
--cc=mchehab@kernel.org \
--cc=mripard@kernel.org \
--cc=nicolas@ndufresne.ca \
--cc=p.zabel@pengutronix.de \
--cc=paul.kocialkowski@bootlin.com \
--cc=wens@csie.org \
/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: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).