linux-kernel - Re: [RFC PATCH 1/2] media: Add AV1 uAPI

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: <731ff6d4-ba1c-1918-e134-013786be473a@xs4all.nl>
Date:   Thu, 2 Sep 2021 17:10:02 +0200
From:   Hans Verkuil <hverkuil-cisco@...all.nl>
To:     daniel.almeida@...labora.com, stevecho@...gle.com,
        shawnku@...gle.com, tzungbi@...gle.com, mcasas@...gle.com,
        nhebert@...gle.com, abodenha@...gle.com, randy.wu@...iatek.com,
        yunfei.dong@...iatek.com, gustavo.padovan@...labora.com,
        andrzej.pietrasiewicz@...labora.com, enric.balletbo@...labora.com,
        ezequiel@...labora.com, nicolas.dufresne@...labora.com,
        tomeu.vizoso@...labora.com, nick.milner@...labora.com,
        xiaoyong.lu@...iatek.com, mchehab@...nel.org
Cc:     linux-media@...r.kernel.org, linux-kernel@...r.kernel.org,
        kernel@...labora.com
Subject: Re: [RFC PATCH 1/2] media: Add AV1 uAPI

Hi Daniel!

Two small comments below:

On 11/08/2021 00:05, daniel.almeida@...labora.com wrote:
> From: Daniel Almeida <daniel.almeida@...labora.com>
> 
> This patch adds the  AOMedia Video 1 (AV1) kernel uAPI.
> 
> This design is based on currently available AV1 API implementations and
> aims to support the development of AV1 stateless video codecs
> on Linux.
> 
> Signed-off-by: Daniel Almeida <daniel.almeida@...labora.com>
> ---
>  .../userspace-api/media/v4l/biblio.rst        |   10 +
>  .../media/v4l/ext-ctrls-codec-stateless.rst   | 1268 +++++++++++++++++
>  .../media/v4l/pixfmt-compressed.rst           |   21 +
>  .../media/v4l/vidioc-g-ext-ctrls.rst          |   36 +
>  .../media/v4l/vidioc-queryctrl.rst            |   54 +
>  .../media/videodev2.h.rst.exceptions          |    9 +
>  drivers/media/v4l2-core/v4l2-ctrls-core.c     |  286 +++-
>  drivers/media/v4l2-core/v4l2-ctrls-defs.c     |   79 +
>  drivers/media/v4l2-core/v4l2-ioctl.c          |    1 +
>  include/media/v4l2-ctrls.h                    |   12 +
>  include/uapi/linux/v4l2-controls.h            |  796 +++++++++++
>  include/uapi/linux/videodev2.h                |   15 +
>  12 files changed, 2586 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/userspace-api/media/v4l/biblio.rst b/Documentation/userspace-api/media/v4l/biblio.rst
> index 7b8e6738ff9e..7061144d10bb 100644
> --- a/Documentation/userspace-api/media/v4l/biblio.rst
> +++ b/Documentation/userspace-api/media/v4l/biblio.rst
> @@ -417,3 +417,13 @@ VP8
>  :title:     RFC 6386: "VP8 Data Format and Decoding Guide"
>  
>  :author:    J. Bankoski et al.
> +
> +.. _av1:
> +
> +AV1
> +===
> +
> +
> +:title:     AV1 Bitstream & Decoding Process Specification
> +
> +:author:    Peter de Rivaz, Argon Design Ltd, Jack Haughton, Argon Design Ltd

<snip>

> diff --git a/Documentation/userspace-api/media/v4l/vidioc-queryctrl.rst b/Documentation/userspace-api/media/v4l/vidioc-queryctrl.rst
> index 819a70a26e18..73ff5311b7ae 100644
> --- a/Documentation/userspace-api/media/v4l/vidioc-queryctrl.rst
> +++ b/Documentation/userspace-api/media/v4l/vidioc-queryctrl.rst
> @@ -507,6 +507,60 @@ See also the examples in :ref:`control`.
>        - n/a
>        - A struct :c:type:`v4l2_ctrl_hevc_decode_params`, containing HEVC
>  	decoding parameters for stateless video decoders.
> +    * - ``V4L2_CTRL_TYPE_AV1_SEQUENCE``
> +      - n/a
> +      - n/a
> +      - n/a
> +      - A struct :c:type:`v4l2_ctrl_av1_sequence`, containing AV1 Sequence OBU
> +	decoding parameters for stateless video decoders.
> +    * - ``V4L2_CTRL_TYPE_AV1_TILE_GROUP``
> +      - n/a
> +      - n/a
> +      - n/a
> +      - A struct :c:type:`v4l2_ctrl_av1_tile_group`, containing AV1 Tile Group
> +	OBU decoding parameters for stateless video decoders.
> +    * - ``V4L2_CTRL_TYPE_AV1_TILE_GROUP_ENTRY``
> +      - n/a
> +      - n/a
> +      - n/a
> +      - A struct :c:type:`v4l2_ctrl_av1_tile_group`, containing AV1 Tile Group

I guess this should be:

Tile Group -> Tile Group Entry

> +	OBU decoding parameters for stateless video decoders.
> +    * - ``V4L2_CTRL_TYPE_AV1_TILE_LIST``
> +      - n/a
> +      - n/a
> +      - n/a
> +      - A struct :c:type:`v4l2_ctrl_av1_tile_list`, containing AV1 Tile List
> +	OBU decoding parameters for stateless video decoders.
> +    * - ``V4L2_CTRL_TYPE_AV1_TILE_LIST_ENTRY``
> +      - n/a
> +      - n/a
> +      - n/a
> +      - A struct :c:type:`v4l2_ctrl_av1_tile_list_entry`, containing AV1 Tile List

Also missing 'Entry'

> +	OBU decoding parameters for stateless video decoders.
> +    * - ``V4L2_CTRL_TYPE_AV1_FRAME_HEADER``
> +      - n/a
> +      - n/a
> +      - n/a
> +      - A struct :c:type:`v4l2_ctrl_av1_frame_header`, containing AV1 Frame/Frame
> +	Header OBU decoding parameters for stateless video decoders.
> +    * - ``V4L2_CTRL_TYPE_AV1_PROFILE``
> +      - n/a
> +      - n/a
> +      - n/a
> +      - A enum :c:type:`v4l2_ctrl_av1_profile`, indicating what AV1 profiles
> +	an AV1 stateless decoder might support.
> +    * - ``V4L2_CTRL_TYPE_AV1_LEVEL``
> +      - n/a
> +      - n/a
> +      - n/a
> +      - A enum :c:type:`v4l2_ctrl_av1_level`, indicating what AV1 levels
> +	an AV1 stateless decoder might support.
> +    * - ``V4L2_CTRL_TYPE_AV1_OPERATING_MODE``
> +      - n/a
> +      - n/a
> +      - n/a
> +      - A enum :c:type:`v4l2_ctrl_av1_operating_mode`, indicating what AV1
> +	operating modes an AV1 stateless decoder might support.
>  
>  .. raw:: latex
>  

Regards,

	Hans