Re: [RFC/WIP 0/4] HEIC image encoder

[Nicolas Dufresne <nicolas@ndufresne.ca>]

Le jeudi 29 avril 2021 à 16:28 +0300, Stanimir Varbanov a écrit :
> Hi,
> 
> HEIC (High-Efficiency Image Container) is a variant of HEIF (High
> Efficiency Image File Format) where HEVC/h265 codec is used to encode
> images.  For more info see [1].
> 
> In this RFC we propose a new compressed pixel format V4L2_PIX_FMT_HEIC.
> The name is debatable and could be changed (V4L2_PIX_FMT_HEVC_HEIF is
> also an option).
> 
> There are two encoding modes which should be selectable by clients:
>     1. Regular image encoding
>     2. Grid image encoding
> 
> 1. Regular image encoding
> 
> Propose to reuse stateful video encoder spec [2].
> 
> - queuing one OUTPUT buffer will produce one CAPTURE buffer.  The
> client could trigger Drain sequence at any point of time.

Do you know the rationale why the normal HEVC encoder isn't used and then muxed
into the HEIF container ? It is these days quite atypical for HW to handle the
container part. Perhaps they hacked the header, I am not so familiar with these
new ISOMBF based image format (AV1 got something very similar, though less
convoluted).

> 
> 2. Grid image encoding
> 
> Propose to reuse stateful video encoder spec [2].
> 
> - queuing one OUTPUT buffer will produce a number of grids CAPTURE
> buffers.  The client could trigger Drain sequence at any point of time.
> 
> This image encoding mode is used when the input image resolution is
> bigger then the hardware can support and/or for compatibility reasons
> (for exmaple, the HEIC decoding hardware is not capable to decode higher
> than VGA resolutions).
> 
> In this mode the input image is divided on square blocks (we call them grids)
> and every block is encoded separately (the Venus driver presently supports 
> grid size of 512x512 but that could be changed in the future).

Are the blocks always power of two ? Or multiple of 16 ? How do you expose this
information to application ? It sounds like an HW decoder would also need to
expose these restrictions. Userspace will need to be able to check without
trying if the HW decoder handles the grid side to be able to fallback to
software decoding.

> 
> To set the grid size we use a new v4l2 control.
> 
> The side effect of this mode is that the client have to set the v4l2
> control and thus enable grid encoding before setting the formats on
> CAPTURE and OUTPUT queues, because the grid size reflects on the
> selected resolutions. Also the horizontal and vertical strides will
> also be affected because thеy have to be aligned to the grid size
> in order to satisfy DMA alignment restrictions.
> 
> Using of v4l2 control to set up Grid mode and Grid size above looks
> inpractical and somehow breaks the v4l2 and v4l2 control rules, so
> I'd give one more option. 

The stasteless decoders uses a control that must be set after the OUTPUT format,
but before enumerating capture formats. Not exactly the same, but there is a
control that interact with the format negotiation.

> 
> Encoding the Grid mode/size in the new proposed HEIC pixel format:
> 
>    V4L2_PIX_FMT_HEIC - Regular HEIC image encoding
>    V4L2_PIX_FMT_HEIC_GRID_512x512 - Grid HEIC image encoding, grid size of 512x512 
>    and so on ...

I would be worry of the about of formats that may generates.

> 
> Comments and suggestions are welcome!
> 
> regards,
> Stan
> 
> [1] https://en.wikipedia.org/wiki/High_Efficiency_Image_File_Format
> [2] https://linuxtv.org/downloads/v4l-dvb-apis-new/userspace-api/v4l/dev-encoder.html
> 
> 
> Stanimir Varbanov (4):
>   media: Add HEIC compressed pixel format
>   v4l2-ctrls: Add HEIC grid size control
>   venus: helpers: Add a new helper for buffer processing
>   venus: Add HEIC image encoder
> 
>  .../media/v4l/pixfmt-compressed.rst           |   12 +
>  drivers/media/platform/qcom/venus/Makefile    |    2 +
>  drivers/media/platform/qcom/venus/core.h      |   10 +
>  drivers/media/platform/qcom/venus/helpers.c   |   20 +
>  drivers/media/platform/qcom/venus/helpers.h   |    1 +
>  drivers/media/platform/qcom/venus/hfi_cmds.c  |   10 +-
>  .../media/platform/qcom/venus/hfi_helper.h    |    5 +
>  drivers/media/platform/qcom/venus/ienc.c      | 1348 +++++++++++++++++
>  drivers/media/platform/qcom/venus/ienc.h      |   14 +
>  .../media/platform/qcom/venus/ienc_ctrls.c    |   83 +
>  drivers/media/v4l2-core/v4l2-ctrls.c          |    3 +
>  drivers/media/v4l2-core/v4l2-ioctl.c          |    1 +
>  include/uapi/linux/v4l2-controls.h            |    1 +
>  include/uapi/linux/videodev2.h                |    1 +
>  14 files changed, 1510 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/media/platform/qcom/venus/ienc.c
>  create mode 100644 drivers/media/platform/qcom/venus/ienc.h
>  create mode 100644 drivers/media/platform/qcom/venus/ienc_ctrls.c
>