[libcamera-devel] [RFC PATCH v8 1/1] virtio-video: Add virtio video device specification

Albert Esteve aesteve at redhat.com
Wed Jul 26 16:32:44 CEST 2023


On Mon, Jul 10, 2023 at 10:52 AM Alexander Gordeev <
alexander.gordeev at opensynergy.com> wrote:

> Hi Albert,
>
> On 06.07.23 16:59, Albert Esteve wrote:
> > Hi Alexander,
> >
> > Thanks for the patch! It is a long document, so I skimmed a bit in the
> > first read. Find some comments/questions inlined.
> > I will give it a second deeper read soon, but overall I think is in
> > quite good shape. It feels really matured.
>
> Great! Thank you for taking the time to review it.
>
> > On Thu, Jun 29, 2023 at 4:49 PM Alexander Gordeev
> > <Alexander.Gordeev at opensynergy.com
> > <mailto:Alexander.Gordeev at opensynergy.com>> wrote:
> >
> >     Add the specification of the video decoder and encoder devices, which
> >     can be used to provide host-accelerated video operations to the
> guest.
> >
> >     Signed-off-by: Alexander Gordeev <Alexander.Gordeev at opensynergy.com
> >     <mailto:Alexander.Gordeev at opensynergy.com>>
> >     Co-authored-by: Keiichi Watanabe <keiichiw at chromium.org
> >     <mailto:keiichiw at chromium.org>>
> >     Co-authored-by: Alexandre Courbot <acourbot at chromium.org
> >     <mailto:acourbot at chromium.org>>
> >     ---
> >       conformance.tex                           |    4 +
> >       content.tex                               |    1 +
> >       device-types/video/description.tex        | 2040
> +++++++++++++++++++++
> >       device-types/video/device-conformance.tex |   22 +
> >       device-types/video/driver-conformance.tex |   16 +
> >       introduction.tex                          |   21 +
> >       6 files changed, 2104 insertions(+)
> >       create mode 100644 device-types/video/description.tex
> >       create mode 100644 device-types/video/device-conformance.tex
> >       create mode 100644 device-types/video/driver-conformance.tex
> >
> >     diff --git a/conformance.tex b/conformance.tex
> >     index 01ccd69..d719eda 100644
> >     --- a/conformance.tex
> >     +++ b/conformance.tex
> >     @@ -34,6 +34,7 @@ \section{Conformance
> >     Targets}\label{sec:Conformance / Conformance Targets}
> >       \ref{sec:Conformance / Driver Conformance / SCMI Driver
> Conformance},
> >       \ref{sec:Conformance / Driver Conformance / GPIO Driver
> >     Conformance} or
> >       \ref{sec:Conformance / Driver Conformance / PMEM Driver
> Conformance}.
> >     +\ref{sec:Conformance / Driver Conformance / Video Driver
> Conformance},
> >
> >           \item Clause \ref{sec:Conformance / Legacy Interface:
> >     Transitional Device and Transitional Driver Conformance}.
> >         \end{itemize}
> >     @@ -61,6 +62,7 @@ \section{Conformance
> >     Targets}\label{sec:Conformance / Conformance Targets}
> >       \ref{sec:Conformance / Device Conformance / SCMI Device
> Conformance},
> >       \ref{sec:Conformance / Device Conformance / GPIO Device
> >     Conformance} or
> >       \ref{sec:Conformance / Device Conformance / PMEM Device
> Conformance}.
> >     +\ref{sec:Conformance / Device Conformance / Video Device
> Conformance},
> >
> >           \item Clause \ref{sec:Conformance / Legacy Interface:
> >     Transitional Device and Transitional Driver Conformance}.
> >         \end{itemize}
> >     @@ -152,6 +154,7 @@ \section{Conformance
> >     Targets}\label{sec:Conformance / Conformance Targets}
> >       \input{device-types/scmi/driver-conformance.tex}
> >       \input{device-types/gpio/driver-conformance.tex}
> >       \input{device-types/pmem/driver-conformance.tex}
> >     +\input{device-types/video/driver-conformance.tex}
> >
> >       \conformance{\section}{Device Conformance}\label{sec:Conformance /
> >     Device Conformance}
> >
> >     @@ -238,6 +241,7 @@ \section{Conformance
> >     Targets}\label{sec:Conformance / Conformance Targets}
> >       \input{device-types/scmi/device-conformance.tex}
> >       \input{device-types/gpio/device-conformance.tex}
> >       \input{device-types/pmem/device-conformance.tex}
> >     +\input{device-types/video/device-conformance.tex}
> >
> >       \conformance{\section}{Legacy Interface: Transitional Device and
> >     Transitional Driver Conformance}\label{sec:Conformance / Legacy
> >     Interface: Transitional Device and Transitional Driver Conformance}
> >       A conformant implementation MUST be either transitional or
> >     diff --git a/content.tex b/content.tex
> >     index d2ab9eb..90708d7 100644
> >     --- a/content.tex
> >     +++ b/content.tex
> >     @@ -765,6 +765,7 @@ \chapter{Device Types}\label{sec:Device Types}
> >       \input{device-types/scmi/description.tex}
> >       \input{device-types/gpio/description.tex}
> >       \input{device-types/pmem/description.tex}
> >     +\input{device-types/video/description.tex}
> >
> >       \chapter{Reserved Feature Bits}\label{sec:Reserved Feature Bits}
> >
> >     diff --git a/device-types/video/description.tex
> >     b/device-types/video/description.tex
> >     new file mode 100644
> >     index 0000000..760df7f
> >     --- /dev/null
> >     +++ b/device-types/video/description.tex
> >     @@ -0,0 +1,2040 @@
> >     +\section{Video Device}
> >     +\label{sec:Device Types / Video Device}
> >     +
> >     +The virtio video encoder and decoder devices provide support for
> >     +host-accelerated video encoding and decoding. Despite being
> different
> >     +device types, they use the same protocol and general flow.
> >     +
> >     +\subsection{Device ID}
> >     +\label{sec:Device Types / Video Device / Device ID}
> >     +
> >     +\begin{description}
> >     +\item[30]
> >     +  encoder device
> >     +\item[31]
> >     +  decoder device
> >     +\end{description}
> >     +
> >     +\subsection{Virtqueues}
> >     +\label{sec:Device Types / Video Device / Virtqueues}
> >     +
> >     +\begin{description}
> >     +\item[0]
> >     +  commandq - queue for driver commands and device responses to
> >     these commands
> >     +\item[1]
> >     +  eventq - queue for device delayed responses to commands and
> >     standalone
> >     +  device events
> >     +\end{description}
> >     +
> >     +\subsection{Feature bits}
> >     +\label{sec:Device Types / Video Device / Feature bits}
> >     +
> >     +\begin{description}
> >     +\item[VIRTIO_VIDEO_F_RESOURCE_GUEST_PAGES (0)]
> >     +  Guest pages can be used as the backing memory of resources.
> >     +\item[VIRTIO_VIDEO_F_RESOURCE_NON_CONTIG (1)]
> >     +  The device can use non-contiguous guest memory as the backing
> >     memory of
> >     +  resources. Only meaningful if VIRTIO_VIDEO_F_RESOURCE_GUEST_PAGES
> >     is also
> >     +  set.
> >     +\item[VIRTIO_VIDEO_F_RESOURCE_VIRTIO_OBJECT (2)]
> >     +  Objects exported by another virtio device can be used as the
> >     backing memory
> >     +  of resources.
> >     +\item[VIRTIO_VIDEO_F_RESOURCE_DYNAMIC (3)]
> >     +  The device supports re-attaching memory to resources while
> streaming.
> >     +% TODO this flag is not used anywhere at the moment.
> >     +% Might be necessary with Android.
> >     +\end{description}
> >
> >
> > Maybe I am missing part of the previous dicussion, but is this
> > VIRTIO_VIDEO_F_RESOURCE_DYNAMIC flag new?
> > Not sure what re-attaching memory means and what the flag is supossed to
> do.
> > In the comment you mention specifically Android, so it got me curious.
> > What is the usecase for this feature flag?
>
> This flag first appeared in draft v6, but it is still not used anywhere
> in the text. I'm almost sure I know why it was added by Alexandre. Our
> experince with Android's Codec2 and v4l2_codec2 shows that something
> like this can happen:
>
> 1. The V4L2 device is opened. For example, a decoder.
> 2. All parameters and buffers are set up. Both queues are started.
> 3. A buffer is queued and then dequeued later. From this moment it
> "belongs" to the guest user-space.
> 4. This opportunity is used to detach the buffer's memory and attach
> some different chunks of memory.
> 5. Then the buffer is queued again. And so on.
>
> The problem here is that according to V4L2 conventions the old memory is
> expected to not be modified after it is dequeued because it could be
> used as a reference in the decoding process. Unfortunately these
> conventions were rather informal until recently AFAIU. Here is the
> patch, that adds this and an interesting discussuin related to the
> issue:
> https://lore.kernel.org/all/20211018091427.88468-1-acourbot@chromium.org/
>
> So the way it is done in Android could be problematic. So the driver
> should probably forbid the reattaching unless the queue is stopped. But
> if the virtio video device is, for example, always doing a pixel format
> conversion after the decoding, then it is OK to reattach the memory. In
> this case the device can advertise the VIRTIO_VIDEO_F_RESOURCE_DYNAMIC
> flag to notify the driver, that the memory could be safely reattached.
> Maybe this is not the best way to deal with this, see the discussion at
> the link above. At least it is not very invasive.
>
> >     +
> >     +\devicenormative{\subsubsection}{Feature bits}{Device Types / Video
> >     Device / Feature bits}
> >     +
> >     +The device MUST set at least one of
> >     VIRTIO_VIDEO_F_RESOURCE_GUEST_PAGES or
> >     +VIRTIO_VIDEO_F_RESOURCE_VIRTIO_OBJECT, since the absence of both
> >     bits would
> >     +mean that no memory can be used at all for resources.
> >     +
> >     +The device MUST NOT set VIRTIO_VIDEO_F_RESOURCE_NON_CONTIG unless
> >     it also sets
> >     +VIRTIO_VIDEO_F_RESOURCE_GUEST_PAGES.
> >     +
> >     +\drivernormative{\subsubsection}{Feature bits}{Device Types / Video
> >     Device / Feature bits}
> >     +
> >     +The driver MUST negotiate at least one of the
> >     +VIRTIO_VIDEO_F_RESOURCE_GUEST_PAGES and
> >     VIRTIO_VIDEO_F_RESOURCE_VIRTIO_OBJECT
> >     +features.
> >     +
> >     +If VIRTIO_VIDEO_F_RESOURCE_GUEST_PAGES has been negotiated, but not
> >     +VIRTIO_VIDEO_F_RESOURCE_NON_CONTIG, the driver MUST use physically
> >     +contiguous memory for all the buffers it allocates.
> >     +
> >     +\subsection{Device configuration layout}
> >     +\label{sec:Device Types / Video Device / Device configuration
> layout}
> >     +
> >     +The video device configuration space uses the following layout:
> >     +
> >     +\begin{lstlisting}
> >     +struct virtio_video_config {
> >     +        le32 caps_length;
> >     +};
> >     +\end{lstlisting}
> >     +
> >     +\begin{description}
> >     +\item[\field{caps_length}]
> >     +  is the minimum length in bytes that a device-writable buffer must
> >     have
> >     +  in order to receive the response to
> >     VIRTIO_VIDEO_CMD_DEVICE_QUERY_CAPS, see
> >     +  \ref{sec:Device Types / Video Device / Device Operation / Device
> >     Operation: Device Commands / VIRTIO_VIDEO_CMD_DEVICE_QUERY_CAPS}.
> >     +\end{description}
> >     +
> >     +\devicenormative{\subsubsection}{Device configuration
> >     layout}{Device Types / Video Device / Device configuration layout}
> >     +
> >     +The device MUST set the \field{caps_length} field to a value equal
> to
> >     +the response size of VIRTIO_VIDEO_CMD_DEVICE_QUERY_CAPS.
> >     +
> >     +\subsection{Supported parameters}
> >     +\label{sec:Device Types / Video Device / Supported parameters}
> >     +
> >     +\subsubsection{Supported coded formats}
> >     +\label{sec:Device Types / Video Device / Supported parameters /
> >     Supported coded formats}
> >     +
> >     +The following coded formats are defined:
> >     +\begin{lstlisting}
> >     +#define VIRTIO_VIDEO_CODED_FORMAT_MPEG2  1  /* MPEG-2 Part 2
> >     (V4L2_PIX_FMT_MPEG2) */
> >     +#define VIRTIO_VIDEO_CODED_FORMAT_MPEG4  2  /* MPEG-4 Part 2
> >     (V4L2_PIX_FMT_MPEG4) */
> >     +#define VIRTIO_VIDEO_CODED_FORMAT_H264   3  /* H.264
> >     (V4L2_PIX_FMT_H264)  */
> >     +#define VIRTIO_VIDEO_CODED_FORMAT_HEVC   4  /* HEVC aka H.265
> >     (V4L2_PIX_FMT_HEVC)  */
> >     +#define VIRTIO_VIDEO_CODED_FORMAT_VP8    5  /* VP8
> >     (V4L2_PIX_FMT_VP8)   */
> >     +#define VIRTIO_VIDEO_CODED_FORMAT_VP9    6  /* VP9
> >     (V4L2_PIX_FMT_VP9)   */
> >     +\end{lstlisting}
> >     +
> >     +The above constants have two usages:
> >     +\begin{enumerate}
> >     +\item As bit numbers, used to tell the driver which coded formats
> are
> >     +supported by the device, see
> >     +\ref{sec:Device Types / Video Device / Device Operation / Device
> >     Operation: Device Commands / VIRTIO_VIDEO_CMD_DEVICE_QUERY_CAPS}.
> >     +\item As values, used to designate the coded format when working
> with
> >     +stream parameters, see
> >     +\ref{sec:Device Types / Video Device / Device Operation / Device
> >     Operation: Stream commands / VIRTIO_VIDEO_CMD_STREAM_SET_PARAMS}.
> >     +\end{enumerate}
> >     +
> >     +The coded formats and the expected data units per buffer are
> >     documented in
> >     +\hyperref[intro:V4L2]{V4L2 header} and
> >     +\hyperref[intro:V4L2 compressed]{V4L2 compressed formats
> >     documentation}.
> >     +
> >     +\subsubsection{Supported raw formats}
> >     +\label{sec:Device Types / Video Device / Supported parameters /
> >     Supported raw formats}
> >     +
> >     +The following raw formats are defined:
> >     +\begin{lstlisting}
> >     +#define VIRTIO_VIDEO_RAW_FORMAT_ARGB8888  1  /* DRM_FORMAT_ARGB8888
> >     / V4L2_PIX_FMT_ABGR32 */
> >     +#define VIRTIO_VIDEO_RAW_FORMAT_BGRA8888  2  /* DRM_FORMAT_BGRA8888
> >     / V4L2_PIX_FMT_ARGB32 */
> >     +#define VIRTIO_VIDEO_RAW_FORMAT_RGBA8888  3  /* DRM_FORMAT_RGBA8888
> >     / V4L2_PIX_FMT_BGRA32 */
> >     +#define VIRTIO_VIDEO_RAW_FORMAT_NV12      4  /* DRM_FORMAT_NV12
> >       / V4L2_PIX_FMT_NV12   */
> >     +#define VIRTIO_VIDEO_RAW_FORMAT_YUV420    5  /* DRM_FORMAT_YUV420
> >       / V4L2_PIX_FMT_YUV420 */
> >     +#define VIRTIO_VIDEO_RAW_FORMAT_YVU420    6  /* DRM_FORMAT_YVU420
> >       / V4L2_PIX_FMT_YVU420 */
> >     +#define VIRTIO_VIDEO_RAW_FORMAT_YUYV      7  /* DRM_FORMAT_YUYV
> >       / V4L2_PIX_FMT_YUYV   */
> >     +\end{lstlisting}
> >     +
> >     +The above constants have two usages:
> >     +\begin{enumerate}
> >     +\item As bit numbers, used to tell the driver which raw formats are
> >     +supported by the device, see
> >     +\ref{sec:Device Types / Video Device / Device Operation / Device
> >     Operation: Device Commands / VIRTIO_VIDEO_CMD_DEVICE_QUERY_CAPS}.
> >     +\item As values, used to designate the raw format when working with
> >     +stream parameters, see
> >     +\ref{sec:Device Types / Video Device / Device Operation / Device
> >     Operation: Stream commands / VIRTIO_VIDEO_CMD_STREAM_SET_PARAMS}.
> >     +\end{enumerate}
> >     +
> >     +The layouts of raw formats are documented in \hyperref[intro:DRM
> >     formats]{DRM}
> >     +and \hyperref[intro:V4L2]{V4L2} headers, as well as in
> >     +\hyperref[intro:V4L2 RGB]{V4L2 RGB} and
> >     +\hyperref[intro:V4L2 YUV]{planar YUV} formats documentation.
> >     +
> >     +\subsubsection{Supported stream parameters}
> >     +\label{sec:Device Types / Video Device / Supported parameters /
> >     Supported stream parameters}
> >     +
> >     +The following stream parameters are defined:
> >     +\begin{lstlisting}
> >     +#define VIRTIO_VIDEO_PARAM_CODED_FORMAT       1
> >     +#define VIRTIO_VIDEO_PARAM_RAW_FORMAT         2
> >     +#define VIRTIO_VIDEO_PARAM_CODED_RESOURCES    3
> >     +#define VIRTIO_VIDEO_PARAM_RAW_RESOURCES      4
> >     +#define VIRTIO_VIDEO_PARAM_CROP               5
> >     +#define VIRTIO_VIDEO_PARAM_BITRATE            6  /* Same as
> >     V4L2_CID_MPEG_VIDEO_BITRATE */
> >     +#define VIRTIO_VIDEO_PARAM_GROUP_CODEC_MPEG2  7
> >     +#define VIRTIO_VIDEO_PARAM_GROUP_CODEC_MPEG4  8
> >     +#define VIRTIO_VIDEO_PARAM_GROUP_CODEC_H264   9
> >     +#define VIRTIO_VIDEO_PARAM_GROUP_CODEC_HEVC   10
> >     +#define VIRTIO_VIDEO_PARAM_GROUP_CODEC_VP8    11
> >     +#define VIRTIO_VIDEO_PARAM_GROUP_CODEC_VP9    12
> >     +\end{lstlisting}
> >     +% TODO acourbot: See b/241492607 (fractional frame rates??)
> >     +
> >     +The above constants have two usages:
> >     +\begin{enumerate}
> >     +\item As bit numbers, used to tell the driver which stream
> >     parameters are
> >     +supported by the device, see
> >     +\ref{sec:Device Types / Video Device / Device Operation / Device
> >     Operation: Device Commands / VIRTIO_VIDEO_CMD_DEVICE_QUERY_CAPS}.
> >     +\item As values, used to designate the stream parameters when
> >     working with
> >     +them, see
> >     +\ref{sec:Device Types / Video Device / Device Operation / Device
> >     Operation: Stream commands / VIRTIO_VIDEO_CMD_STREAM_SET_PARAMS}.
> >     +\end{enumerate}
> >     +
> >     +\subsection{Device Initialization}
> >     +\label{sec:Device Types / Video Device / Device Initialization}
> >     +
> >     +\begin{enumerate}
> >     +\def\labelenumi{\arabic{enumi}.}
> >     +\item
> >     +  The driver reads the feature bits and negotiates the features it
> >     needs.
> >     +\item
> >     +  The driver sets up the commandq and the eventq.
> >     +\item
> >     +  The driver reads the \field{caps_length} field of the
> configuration
> >     +  space and prepares a buffer of at least that size.
> >     +\item
> >     +  The driver sends that buffer on the commandq with the
> >     +  VIRTIO_VIDEO_CMD_DEVICE_QUERY_CAPS command.
> >     +\item
> >     +  The driver receives the response from the device, and parses its
> >     capabilities.
> >     +\end{enumerate}
> >     +
> >     +\subsection{Device Operation}
> >     +\label{sec:Device Types / Video Device / Device Operation}
> >     +
> >     +The commandq is used by the driver to send commands to the device
> >     and to
> >     +receive the device's responses via used buffers. The eventq is used
> >     by the
> >     +device to send the device's delayed responses to commands and
> >     standalone
> >     +device events.
> >     +
> >     +The driver can create new streams using the
> >     +VIRTIO_VIDEO_CMD_STREAM_CREATE command. Each stream has two resource
> >     +queues (not to be confused with the virtio queues) called INPUT and
> >     +OUTPUT, when the direction of the data flow matters. The INPUT
> >     queue accepts
> >     +driver-filled input data for the device (coded data for a decoder;
> >     +input frames for an encoder), while the OUTPUT queue receives
> >     resources to be
> >     +filled by the device as a result of processing the INPUT queue
> >     (decoded raw
> >     +frames for a decoder; encoded data for an encoder).
> >     +
> >     +These same queues can be also called CODED and RAW, when their
> >     content matters.
> >     +The CODED queue is used to transfer compressed video data (INPUT
> >     for a decoder;
> >     +OUTPUT for an encoder), while the RAW queue is used to transfer raw
> >     frames
> >     +(OUTPUT for a decoder; INPUT for an encoder).
> >     +
> >     +The INPUT and OUTPUT queues are effectively independent, and the
> driver
> >     +can fill them without caring about the other queue. In particular
> there
> >     +is no need to queue input and output resources in pairs: one input
> >     +resource can result in zero to many produced output resources.
> >     +
> >     +A resource is a set of memory buffers that contain a unit of data
> that
> >     +the device can process or produce. Most resources will only have one
> >     +buffer (like coded data and single-planar raw frames), but frames
> >     using a
> >     +multi-planar format can have several.
> >     +
> >     +Parameters allow the driver to configure the stream for the
> decoding or
> >     +encoding operation. The parameters can be obtained and configured
> using
> >     +VIRTIO_VIDEO_CMD_STREAM_SET_PARAMS. Available parameters depend on
> >     +the device type and are detailed in section
> >     +\ref{sec:Device Types / Video Device / Supported parameters /
> >     Supported stream parameters}.
> >     +
> >     +Before resources can be submitted to a queue, backing memory must be
> >     +attached to them using VIRTIO_VIDEO_CMD_RESOURCE_ATTACH_BACKING.
> >     +The exact form of that memory is negotiated using the feature flags.
> >     +
> >     +In the case of a decoder device, the decoded frames are made
> available
> >     +on the OUTPUT queue in presentation order.
> >     +
> >     +Resources are queued to the INPUT or OUTPUT queue using the
> >     +VIRTIO_VIDEO_CMD_RESOURCE_QUEUE command. The device sends a delayed
> >     response
> >     +to this command when an input resource has been fully processed and
> >     can be
> >     +reused by the driver, or when an output resource has been filled by
> the
> >     +device as a result of processing.
> >     +
> >     +The device can detect stream-related events that require
> intervention
> >     +from the driver and signals them on the eventq, see
> >     +\ref{sec:Device Types / Video Device / Device Operation / Device
> >     Operation: Standalone Events}.
> >     +One example is a dynamic parameters change while decoding a stream,
> >     which
> >     +may require the driver to reallocate the backing memory of its
> output
> >     +resources to fit the new resolution.
> >     +
> >     +% RESET and DRAIN have essentially the same outcome: all the input
> >     +% resources queued before the command are released, there are no
> >     related
> >     +% output resources in the decoder/encoder, the dequeued output
> >     resources
> >     +% can't be used as a reference by the device. So the other
> >     requirements should
> >     +% be reasonably similar.
> >     +% Use-case: playback in a loop from second 1 till the end of file.
> >     +
> >     +% TODO put some examples in the comments
> >     +
> >     +\subsubsection{Device Operation: Command Virtqueue}
> >     +\label{sec:Device Types / Video Device / Device Operation / Device
> >     Operation: Command Virtqueue}
> >     +
> >     +This section details the commands that can be sent on the commandq
> by
> >     +the driver, as well as the responses that the device will write.
> >     +
> >     +Different structures are used for each command and response. A
> command
> >     +structure starts with the requested command code, defined as
> follows:
> >     +
> >     +\begin{lstlisting}
> >     +/* Device */
> >     +#define VIRTIO_VIDEO_CMD_DEVICE_QUERY_CAPS       0x100
> >     +
> >     +/* Stream */
> >     +#define VIRTIO_VIDEO_CMD_STREAM_CREATE           0x200
> >     +#define VIRTIO_VIDEO_CMD_STREAM_DESTROY          0x201
> >     +#define VIRTIO_VIDEO_CMD_STREAM_SET_PARAMS       0x202
> >
> >
> > Why not having a GET_PARAMS counterpart?
>
> With the current approach to setting the parameters GET_PARAMS would be
> almost exactly the same as SET_PARAMS if params.stream_params_bitmask is
> set to zero. So I assumed that it is better to save the space and remove
> the GET_PARAMS counterpart since the length of the spec is already a
> sensitive topic.
>
> >     +#define VIRTIO_VIDEO_CMD_STREAM_DRAIN            0x203
> >     +#define VIRTIO_VIDEO_CMD_STREAM_RESET            0x204
> >     +
> >     +/* Resource */
> >     +#define VIRTIO_VIDEO_CMD_RESOURCE_ATTACH_BACKING 0x300
> >     +#define VIRTIO_VIDEO_CMD_RESOURCE_QUEUE          0x301
> >     +\end{lstlisting}
> >     +
> >     +A response structure starts with the result of the requested
> command,
> >     +defined as follows:
> >     +
> >     +\begin{lstlisting}
> >     +/* Success */
> >     +#define VIRTIO_VIDEO_RESULT_OK                          0x000
> >     +#define VIRTIO_VIDEO_RESULT_OK_DELAYED                  0x001
> >     +
> >     +/* Error */
> >     +#define VIRTIO_VIDEO_RESULT_ERR_INVALID_COMMAND         0x100
> >     +#define VIRTIO_VIDEO_RESULT_ERR_INVALID_OPERATION       0x101
> >     +#define VIRTIO_VIDEO_RESULT_ERR_INVALID_STREAM_ID       0x102
> >     +#define VIRTIO_VIDEO_RESULT_ERR_INVALID_RESOURCE_ID     0x103
> >     +#define VIRTIO_VIDEO_RESULT_ERR_INVALID_ARGUMENT        0x104
> >     +#define VIRTIO_VIDEO_RESULT_ERR_OUT_OF_MEMORY           0x105
> >     +\end{lstlisting}
> >     +
> >     +For response structures carrying an error code, the rest of the
> >     +structure is considered invalid.
> >     +
> >     +For all commands beginning background operations and returning
> delayed
> >     +responses over eventq, the command response is defined as follows:
> >     +
> >     +\begin{lstlisting}
> >     +#define VIRTIO_VIDEO_INVALID_RESPONSE_ID  0xffffffff
> >     +
> >     +struct virtio_video_command_resp_delayed_common {
> >     +        le32 result; /* VIRTIO_VIDEO_RESULT_* */
> >     +        le32 delayed_response_id;
> >     +};
> >     +\end{lstlisting}
> >     +
> >     +\begin{description}
> >     +\item[\field{result}]
> >     +  is
> >     +
> >     +  \begin{description}
> >     +  \item[VIRTIO_VIDEO_RESULT_OK_DELAYED]
> >     +    if the command started the desired background operation
> >     successfully,
> >     +  \item[VIRTIO_VIDEO_RESULT_ERR_INVALID_STREAM_ID]
> >     +    if the mentioned stream ID does not exist,
> >     +  \item[VIRTIO_VIDEO_RESULT_ERR_INVALID_RESOURCE_ID]
> >     +    if the mentioned resource ID does not exist,
> >     +  \item[VIRTIO_VIDEO_RESULT_ERR_INVALID_ARGUMENT]
> >     +    if other command parameters are not valid, e.g. not within the
> >     device's
> >     +    capabilities,
> >     +  \item[VIRTIO_VIDEO_RESULT_ERR_INVALID_OPERATION]
> >     +    if the command is performed at a time when it is not valid.
> >     +  \end{description}
> >     +\item[\field{delayed_response_id}]
> >     +  is an ID of the future delayed response provided by the device,
> >     that allows
> >     +  to relate it to the command.
> >     +\end{description}
> >     +
> >     +\devicenormative{\paragraph}{Device Operation: Command
> >     Virtqueue}{Device Types / Video Device / Device Operation / Device
> >     Operation: Command Virtqueue}
> >     +
> >     +Responses to a command MUST be written by the device in the first
> >     +device-writable descriptor of the descriptor chain from which the
> >     +command came.
> >     +
> >     +The device MUST return VIRTIO_VIDEO_RESULT_ERR_INVALID_COMMAND to
> >     +any command code it does not recognize.
> >     +
> >     +\field{delayed_response_id} MUST be set to a stream-unique
> >     identifier that
> >     +remains valid as long as the background operation hasn't finished.
> >
> >     +
> >     +\drivernormative{\paragraph}{Device Operation: Command
> >     Virtqueue}{Device Types / Video Device / Device Operation / Device
> >     Operation: Command Virtqueue}
> >     +
> >     +Descriptor chains sent to the commandq by the driver MUST include at
> >     +least one device-writable descriptor of a size sufficient to
> >     receive the
> >     +response to the queued command.
> >     +
> >     +The driver MUST NOT interpret the rest of a response whose result
> >     is not
> >     +VIRTIO_VIDEO_RESULT_OK or VIRTIO_VIDEO_RESULT_OK_DELAYED.
> >     +
> >     +\subsubsection{Device Operation: Event Virtqueue}
> >     +\label{sec:Device Types / Video Device / Device Operation / Device
> >     Operation: Event Virtqueue}
> >     +
> >     +The eventq is used by the device to send delayed responses to
> >     commands queued
> >     +by the driver on the commandq and standalone events. Stream errors
> >     and dynamic
> >     +parameters changes are caused by changes in the device's state, not
> by
> >     +commands, still they are delivered as
> >     VIRTIO_VIDEO_DELAYED_RESP_STREAM_DESTROY
> >     +and VIRTIO_VIDEO_DELAYED_RESP_STREAM_SET_PARAMS, respectively.
> >
> >
> > So, ERROR and DRC events trigger a
> VIRTIO_VIDEO_DELAYED_RESP_STREAM_DESTROY
> > and VIRTIO_VIDEO_DELAYED_RESP_STREAM_SET_PARAMS events, respectively.
> > And VIRTIO_VIDEO_DELAYED_RESP_STREAM_SET_PARAMS will have
> > a valid `delayed_response_id` only if it comes from a SET_PARAMS command
> > from the
> > driver. Otherwise, if it is due to a dynamic parameter change,
> > `delayed_response_id`
> > will be set to `VIRTIO_VIDEO_INVALID_RESPONSE_ID`. Is this correct?
>
> Yes, this is correct. Looks like it is better to rename
> VIRTIO_VIDEO_INVALID_RESPONSE_ID to VIRTIO_VIDEO_STANDALONE_EVENT_ID
> maybe. Thanks!
>
> >     +
> >     +The supported events are defined as follows:
> >     +
> >     +\begin{lstlisting}
> >     +#define VIRTIO_VIDEO_DELAYED_RESP_STREAM_DESTROY     1
> >     +#define VIRTIO_VIDEO_DELAYED_RESP_STREAM_SET_PARAMS  2
> >     +#define VIRTIO_VIDEO_DELAYED_RESP_STREAM_DRAIN       3
> >     +#define VIRTIO_VIDEO_DELAYED_RESP_STREAM_RESET       4
> >     +#define VIRTIO_VIDEO_DELAYED_RESP_RESOURCE_QUEUE     5
> >     +
> >     +#define VIRTIO_VIDEO_EVENT_FLAG_CANCELED             (1 << 0)
> >     +
> >     +struct virtio_video_event {
> >     +        le32 event_type; /* VIRTIO_VIDEO_DELAYED_RESP_* */
> >     +        le32 stream_id;
> >     +        le32 delayed_response_id;
> >     +        le32 event_flags; /* Bitmask of VIRTIO_VIDEO_EVENT_FLAG_* */
> >     +        union {
> >     +                struct virtio_video_stream_set_params_delayed_resp
> >     set_params;
> >     +                struct virtio_video_resource_queue_delayed_resp
> queue;
> >     +        };
> >     +};
> >     +\end{lstlisting}
> >     +
> >     +\begin{description}
> >     +\item[\field{event_type}]
> >     +  is the type of the event.
> >     +\item[\field{stream_id}]
> >     +  is the ID of a valid stream.
> >     +\item[\field{delayed_response_id}]
> >     +  is an ID of the delayed response, that allows to relate it to a
> >     previously
> >     +  submitted command. If it is set to
> >     VIRTIO_VIDEO_INVALID_RESPONSE_ID, then
> >     +  this is a standalone event, see
> >     +  \ref{sec:Device Types / Video Device / Device Operation / Device
> >     Operation: Standalone Events}.
> >     +\item[\field{event_flags}]
> >     +  is a bitmask of VIRTIO_VIDEO_EVENT_FLAG_* flags
> >     +
> >     +  \begin{description}
> >     +  \item[VIRTIO_VIDEO_EVENT_FLAG_CANCELED]
> >     +    is set if the command has been canceled by another command,
> >     that has
> >     +    higher priority. Doesn't make sense for standalone events.
> >     +  \end{description}
> >     +\end{description}
> >     +
> >     +The particular member of the union is selected according to the
> >     +\field{event_type} for some of the types.
> >     +
> >     +\drivernormative{\paragraph}{Device Operation: Event
> >     Virtqueue}{Device Types / Video Device / Device Operation / Device
> >     Operation: Event Virtqueue}
> >     +
> >     +The driver MUST at any time have at least one descriptor with a used
> >     +buffer large enough to contain a \field{struct virtio_video_event}
> >     +queued on the eventq.
> >     +
> >     +The driver MUST NOT put device-readable descriptors into the eventq.
> >     +
> >     +\subsubsection{Device Operation: Device Commands}
> >     +\label{sec:Device Types / Video Device / Device Operation / Device
> >     Operation: Device Commands}
> >     +
> >     +This command allows retrieving the device capabilities.
> >     +
> >     +\paragraph{VIRTIO_VIDEO_CMD_DEVICE_QUERY_CAPS}
> >     +\label{sec:Device Types / Video Device / Device Operation / Device
> >     Operation: Device Commands / VIRTIO_VIDEO_CMD_DEVICE_QUERY_CAPS}
> >     +
> >     +Retrieve device capabilities for all available stream parameters.
> >     +
> >     +The driver sends this command with
> >     +\field{struct virtio_video_device_query_caps}:
> >     +
> >     +\begin{lstlisting}
> >     +struct virtio_video_device_query_caps {
> >     +        le32 cmd_type; /* VIRTIO_VIDEO_CMD_DEVICE_QUERY_CAPS */
> >     +};
> >     +\end{lstlisting}
> >     +
> >     +The device responds with
> >     +\field{struct virtio_video_device_query_caps_resp}:
> >     +
> >     +\begin{lstlisting}
> >     +#define MASK(x) (1 << (x))
> >     +
> >     +struct virtio_video_device_query_caps_resp {
> >     +        le32 result; /* VIRTIO_VIDEO_RESULT_* */
> >     +        le32 stream_params_bitmask; /* Bitmask of
> >     MASK(VIRTIO_VIDEO_PARAM_*) */
> >     +        le32 coded_formats_bitmask; /* Bitmaks of
> >     MASK(VIRTIO_VIDEO_CODED_FORMAT_*) */
> >     +        le32 raw_formats_bitmask; /* Bitmask of
> >     MASK(VIRTIO_VIDEO_RAW_FORMAT_*) */
> >     +        le32 num_format_deps;
> >     +        le32 num_raw_format_caps;
> >     +        le32 num_coded_resources_caps;
> >     +        le32 num_raw_resources_caps;
> >     +        le32 num_bitrate_caps; /* If
> >     MASK(VIRTIO_VIDEO_PARAM_BITRATE) is set. */
> >     +        u8 padding[4];
> >     +        /* If corresponding MASK(VIRTIO_VIDEO_PARAM_GROUP_CODEC_*)
> >     is set. */
> >     +        struct virtio_video_mpeg2_caps mpeg2_caps;
> >     +        struct virtio_video_mpeg4_caps mpeg4_caps;
> >     +        struct virtio_video_h264_caps h264_caps;
> >     +        struct virtio_video_hevc_caps hevc_caps;
> >     +        struct virtio_video_vp8_caps vp8_caps;
> >     +        struct virtio_video_vp9_caps vp9_caps;
> >     +        /**
> >     +         * Followed by
> >     +         * struct virtio_video_format_dependency
> >     format_deps[num_format_deps];
> >     +         */
> >     +        /**
> >     +         * Followed by
> >     +         * struct virtio_video_raw_format_caps
> >     raw_format_caps[num_raw_format_caps];
> >     +         */
> >     +        /**
> >     +         * Followed by
> >     +         * struct virtio_video_coded_resources_caps
> >     +         * coded_resources_caps[num_coded_resources_caps];
> >     +         */
> >     +        /**
> >     +         * Followed by
> >     +         * struct virtio_video_raw_resources_caps
> >     raw_resources_caps[num_raw_resources_caps];
> >     +         */
> >     +        /**
> >     +         * Followed by if MASK(VIRTIO_VIDEO_PARAM_BITRATE) is set
> >     +         * struct virtio_video_bitrate_caps
> >     bitrate_caps[num_bitrate_caps];
> >     +         */
> >     +};
> >
> >
> > Maybe nitpicking, but some of the member structs are inside a comment
> > and some are not.
> > Does not seem to correlate with them being conditional.
> > I think is nice to have conditional fields in comment blocks to
> > highlight it, but then the
> > VIRTIO_VIDEO_PARAM_GROUP_CODEC_* structs need to be in their own comment
> > block.
>
> Yeah, this style comes from draft v5, then I added the conditional
> statementson top, so now it is harder to understand. I also would like
> to do this in a different way. I was thinking recently about
> extendability of this construct, it doesn't look good. If a new codec or
> a new codec-specific parameters is added, it has to be guarded by a new
> feature flag, say VIRTIO_VIDEO_F_CODECS_2024. Then the device will have
> to provide different structures depending on the negotiated flags and
> the driver will have to parse it. This looks quite painful and
> error-prone. My current idea is to replace this with something like FDT
> to make it much more flexible. The resulting blob with all the
> capabilities can even be mapped directly to the guest memory. I'm still
> exploring this idea. WDYT?
>
>
Yes, the struct looks a bit cumbersome and difficult to expand, as you
mention.
But I am not sure what do you mean by FDT, or how you plan to map it to the
guest
memory. Could you expand the idea?
But from what I see, structures for most formats have similar fields for
capabilities.
Couldn't this be unified into a single capabilities struct and fill it with
the raw data obtained
from the host device?


> >     +\end{lstlisting}
> >     +
> >     +\begin{description}
> >     +\item[\field{result}]
> >     +  is
> >     +
> >     +  \begin{description}
> >     +  \item[VIRTIO_VIDEO_RESULT_OK]
> >     +    if the operation succeeded,
> >     +  \item[VIRTIO_VIDEO_RESULT_ERR_OUT_OF_MEMORY]
> >     +    if the descriptor was smaller than the defined
> >     \field{caps_length} in
> >     +    the video device configuration.
> >     +  \end{description}
> >     +\item[\field{stream_params_bitmask}]
> >     +  is a bitmask of supported stream parameters.
> >     +\item[\field{coded_formats_bitmask}]
> >     +  is a bitmask of supported coded formats.
> >     +\item[\field{raw_formats_bitmask}]
> >     +  is a bitmask of supported raw formats.
> >     +\item[\field{num_format_deps}]
> >     +  is the number of elements in the format_deps array.
> >     +\item[\field{num_raw_format_caps}]
> >     +  is the number of elements in the raw_format_caps array.
> >     +\item[\field{num_coded_resources_caps}]
> >     +  is the number of elements in the coded_resources_caps array.
> >     +\item[\field{num_raw_resources_caps}]
> >     +  is the number of elements in the raw_resources_caps array.
> >     +\item[\field{num_bitrate_caps}]
> >     +  is the number of elements in the bitrate_caps array.
> >     +\item[\field{mpeg2_caps}]
> >     +  groups the capabilities of MPEG2 specific parameters.
> >     +\item[\field{mpeg4_caps}]
> >     +  groups the capabilities of MPEG4 specific parameters.
> >     +\item[\field{h264_caps}]
> >     +  groups the capabilities of H.264 specific parameters.
> >     +\item[\field{hevc_caps}]
> >     +  groups the capabilities of HEVC specific parameters.
> >     +\item[\field{vp8_caps}]
> >     +  groups the capabilities of VP8 specific parameters.
> >     +\item[\field{vp9_caps}]
> >     +  groups the capabilities of VP9 specific parameters.
> >     +\item[\field{format_deps}]
> >     +  is an array of size \field{num_format_deps} establishing
> dependencies
> >     +  between coded and raw formats.
> >     +\item[\field{raw_format_caps}]
> >     +  is an array of size \field{num_raw_format_caps} containing the
> >     supported
> >     +  raw formats capabilities.
> >     +\item[\field{coded_resources_caps}]
> >     +  is an array of size \field{num_coded_resources_caps}, that sets
> >     bounds for
> >     +  the number of resources in the CODED queue.
> >     +\item[\field{raw_resources_caps}]
> >     +  is an array of size \field{num_raw_resources_caps}, that sets
> >     bounds for
> >     +  the number of resources in the RAW queue.
> >     +\item[\field{bitrate_caps}]
> >     +  is an array of size \field{num_bitrate_caps} containing the
> supported
> >     +  bitrates.
> >     +\end{description}
> >     +
> >     +% TODO: V4L2 flow: 1. coded format without variants (maybe these
> flags
> >     +% are relevant too: V4L2_FMT_FLAG_CONTINUOUS_BYTESTREAM?,
> >     +% V4L2_FMT_FLAG_DYN_RESOLUTION?,
> >     V4L2_FMT_FLAG_ENC_CAP_FRAME_INTERVAL???),
> >     +% also include variants (see VIDIOC_QUERYCTRL), then raw formats,
> >     +% then resolutions (discrete or stepwise, see
> VIDIOC_ENUM_FRAMESIZES),
> >     +% intervals are optional (see VIDIOC_ENUM_FRAMEINTERVALS)
> >     +
> >
>  +\devicenormative{\subparagraph}{VIRTIO_VIDEO_CMD_DEVICE_QUERY_CAPS}{Device
> Types / Video Device / Device Operation / Device Operation: Device Commands
> / VIRTIO_VIDEO_CMD_DEVICE_QUERY_CAPS}
> >     +
> >     +The device MUST support at least these parameters:
> >     +VIRTIO_VIDEO_PARAM_CODED_FORMAT, VIRTIO_VIDEO_PARAM_RAW_FORMAT,
> >     +VIRTIO_VIDEO_PARAM_CODED_RESOURCES,
> VIRTIO_VIDEO_PARAM_RAW_RESOURCES.
> >     +
> >     +The device MUST NOT mark codec-specific parameters
> >     +(VIRTIO_VIDEO_PARAM_GROUP_CODEC_*) as supported unless the
> >     corresponding
> >     +codecs are supported as well.
> >     +
> >     +The device MUST set to zero all fields with capabilities of
> unsupported
> >     +parameters.
> >     +
> >     +The lengths \field{num_format_deps}, \field{num_raw_format_caps},
> >     +\field{num_coded_resources_caps} and \field{num_raw_resources_caps}
> >     MUST be
> >     +positive.
> >     +
> >     +The device MUST write the five \field{format_deps},
> >     +\field{raw_format_caps}, \field{coded_resources_caps},
> >     +\field{raw_resources_caps} and \field{bitrate_caps} arrays, of
> length
> >     +\field{num_format_deps}, \field{num_raw_format_caps},
> >     +\field{num_coded_resources_caps}, \field{num_raw_resources_caps} and
> >     +\field{num_bitrate_caps}, respectively.
> >     +
> >     +For each coded format in the \field{coded_formats_bitmask} there
> >     MUST be
> >     +at least one element of \field{format_deps} referencing it.
> >     +
> >     +For each raw format in the \field{raw_formats_bitmask} there MUST be
> >     +at least one element of \field{format_deps} referencing it.
> >     +
> >     +For any coded and any raw format there MUST be at most one element
> of
> >     +\field{format_deps} referencing both of them.
> >     +
> >     +Elements of \field{format_deps} SHOULD be ordered according to raw
> >     format
> >     +preferences of the device from preferred to not preferred ones.
> >     +
> >     +For each raw format in the \field{raw_formats_bitmask} there MUST be
> >     +exactly one element of \field{raw_format_caps} referencing it.
> >     +
> >     +For each coded format in the \field{coded_formats_bitmask} there
> >     MUST be
> >     +exactly one element of \field{coded_resources_caps} referencing it.
> >     +
> >     +For each raw format in the \field{raw_formats_bitmask} there MUST be
> >     +exactly one element of \field{raw_resources_caps} referencing it.
> >     +
> >     +If VIRTIO_VIDEO_PARAM_BITRATE is supported, then for each coded
> >     format in
> >     +the \field{coded_formats_bitmask} there MUST be exactly one element
> of
> >     +\field{bitrate_caps} referencing it.
> >     +
> >     +The total size of the response MUST be equal to \field{caps_length}
> >     +bytes, as reported by the device configuration.
> >     +
> >     +\subparagraph{VIRTIO_VIDEO_CMD_DEVICE_QUERY_CAPS: Format
> dependencies}
> >     +
> >     +The description of dependencies between coded and raw formats
> >     +\field{virtio_video_format_dependency} is defined as follows:
> >     +
> >     +\begin{lstlisting}
> >     +struct virtio_video_format_dependency {
> >     +        le32 coded_formats_bitmask; /* Bitmask of
> >     MASK(VIRTIO_VIDEO_CODED_FORMAT_*) */
> >     +        le32 raw_format; /* VIRTIO_VIDEO_RAW_FORMAT_* */
> >     +};
> >     +\end{lstlisting}
> >     +
> >     +\begin{description}
> >     +\item[\field{coded_formats_bitmask}]
> >     +  specifies coded formats, see
> >     +  \ref{sec:Device Types / Video Device / Supported parameters /
> >     Supported coded formats}.
> >     +  If a bit for a specific coded format is set, then this coded
> >     format can be
> >     +  decoded into the specified raw format or encoded from it.
> >     +\item[\field{raw_format}]
> >     +  is a raw format, see
> >     +  \ref{sec:Device Types / Video Device / Supported parameters /
> >     Supported raw formats}.
> >     +\end{description}
> >     +
> >     +\devicenormative{\subparagraph}{VIRTIO_VIDEO_CMD_DEVICE_QUERY_CAPS:
> >     Format dependencies}{Device Types / Video Device / Device Operation
> >     / Device Operation: Device Commands /
> >     VIRTIO_VIDEO_CMD_DEVICE_QUERY_CAPS /
> >     VIRTIO_VIDEO_CMD_DEVICE_QUERY_CAPS: Format dependencies}
> >     +
> >     +\field{coded_formats_bitmask} MUST be a subset of
> >     \field{coded_formats_bitmask}
> >     +field of \field{struct virtio_video_device_query_caps_resp}.
> >     +
> >     +\field{coded_formats_bitmask} MUST specify at least one coded
> format.
> >     +
> >     +\field{raw_format} MUST be set to one of the supported raw formats
> >     according to
> >     +the \field{raw_formats_bitmask} field of
> >     +\field{struct virtio_video_device_query_caps_resp}.
> >     +
> >     +\subparagraph{VIRTIO_VIDEO_CMD_DEVICE_QUERY_CAPS: Raw format
> >     capabilities}
> >     +\label{sec:Device Types / Video Device / Device Operation / Device
> >     Operation: Device Commands / VIRTIO_VIDEO_CMD_DEVICE_QUERY_CAPS /
> >     VIRTIO_VIDEO_CMD_DEVICE_QUERY_CAPS: Raw format capabilities}
> >     +
> >     +The raw format capability description
> >     \field{virtio_video_raw_format_caps} is
> >     +defined as follows:
> >     +
> >     +\begin{lstlisting}
> >     +enum virtio_video_planes_layout {
> >     +        VIRTIO_VIDEO_PLANES_LAYOUT_SINGLE_BUFFER = 1,
> >     +        VIRTIO_VIDEO_PLANES_LAYOUT_MULTI_BUFFERS = 2,
> >     +};
> >     +
> >     +struct virtio_video_range {
> >     +        le32 min;
> >     +        le32 max;
> >     +        le32 step;
> >     +        u8 padding[4];
> >     +};
> >     +
> >     +struct virtio_video_raw_format_caps {
> >     +        le32 raw_formats_bitmask; /* Bitmask of
> >     MASK(VIRTIO_VIDEO_RAW_FORMAT_*) */
> >     +        le32 planes_layouts; /* Bitmask of
> >     VIRTIO_VIDEO_PLANES_LAYOUT_* */
> >     +        le32 plane_align;
> >     +        le32 stride_align_mask;
> >     +        struct virtio_video_range width_range;
> >     +        struct virtio_video_range height_range;
> >     +};
> >     +\end{lstlisting}
> >     +
> >     +\field{struct virtio_video_range} is used to represent a range of
> >     values.
> >     +An integer \(x\) is within the range \field{r} if
> >     +\(\field{r.min} \le x \le \field{r.max}\) holds and \(x\) equals to
> >     +\((\field{min} + \field{step} * n)\) for some integer \(n\).
> >     +
> >     +\begin{description}
> >     +\item[\field{raw_formats_bitmask}]
> >     +  specifies raw formats, see
> >     +  \ref{sec:Device Types / Video Device / Supported parameters /
> >     Supported raw formats},
> >     +  to which these capabilities apply.
> >     +\item[\field{planes_layouts}]
> >     +  is a bitmask with the set of plane layout types from
> >     +  \field{enum virtio_video_planes_layout}.
> >     +\item[\field{plane_align}]
> >     +  is the alignment of planes within a buffer in bytes. This field
> >     is valid
> >     +  only if \field{planes_layouts} has the
> >     +  \field{VIRTIO_VIDEO_PLANES_LAYOUT_SINGLE_BUFFER} bit set.
> >     +\item[\field{stride_align_mask}]
> >     +  is a mask of all supported power of two stride alignments.
> >     +\item[\field{width_range}]
> >     +  is a range of widths in pixels.
> >     +\item[\field{height_range}]
> >     +  is a range of heights in pixels.
> >     +\end{description}
> >     +
> >     +\devicenormative{\subparagraph}{VIRTIO_VIDEO_CMD_DEVICE_QUERY_CAPS:
> >     Raw format capabilities}{Device Types / Video Device / Device
> >     Operation / Device Operation: Device Commands /
> >     VIRTIO_VIDEO_CMD_DEVICE_QUERY_CAPS /
> >     VIRTIO_VIDEO_CMD_DEVICE_QUERY_CAPS: Raw format capabilities}
> >     +
> >     +\field{raw_formats_bitmask} MUST be a subset of
> >     \field{raw_formats_bitmask}
> >     +field of \field{struct virtio_video_device_query_caps_resp}.
> >     +
> >     +\field{raw_formats_bitmask} MUST specify at least one raw format.
> >     +
> >     +The device MUST set
> >     \field{VIRTIO_VIDEO_PLANES_LAYOUT_SINGLE_BUFFER} bit in
> >     +\field{planes_layouts} if the plane layout with planes of a frame
> >     laid out one
> >     +after another in the same buffer is supported.
> >     +
> >     +The device MUST set
> >     \field{VIRTIO_VIDEO_PLANES_LAYOUT_MULTI_BUFFERS} bit in
> >     +\field{planes_layouts} if the plane layout with planes of a frame
> >     laid out in
> >     +separate buffers is supported.
> >     +
> >     +\field{plane_align} MUST be set to a power of two according to the
> >     device
> >     +plane alignment requirements if \field{planes_layouts} has the
> >     +\field{VIRTIO_VIDEO_PLANES_LAYOUT_SINGLE_BUFFER} bit set or to zero
> >     otherwise.
> >     +
> >     +\field{min}, \field{step} and \field{max} MUST be positive.
> >     +
> >     +\field{min} MUST be less then or equal to \field{max} within the
> >     same range.
> >     +
> >     +\subparagraph{VIRTIO_VIDEO_CMD_DEVICE_QUERY_CAPS: Resource
> >     capabilities}
> >     +
> >     +The CODED resource capabilities
> >     \field{virtio_video_coded_resources_caps} is
> >     +defined as follows:
> >     +
> >     +\begin{lstlisting}
> >     +struct virtio_video_coded_resources_caps {
> >     +        le32 coded_formats_bitmask; /* Bitmask of
> >     MASK(VIRTIO_VIDEO_CODED_FORMAT_*) */
> >     +        le32 min_resources;
> >     +        le32 max_resources;
> >     +        le32 buffer_size;
> >     +};
> >     +\end{lstlisting}
> >     +
> >     +\begin{description}
> >     +\item[\field{coded_formats_bitmask}]
> >     +  specifies coded formats, see
> >     +  \ref{sec:Device Types / Video Device / Supported parameters /
> >     Supported coded formats},
> >     +  to which these capabilities apply.
> >     +\item[\field{min_resources}]
> >     +  is the minimum number of resources that the CODED queue supports
> >     for all
> >     +  the specified coded formats.
> >     +\item[\field{max_resources}]
> >     +  is the maximum number of resources that the CODED queue supports
> >     for all
> >     +  the specified coded formats.
> >     +\item[\field{buffer_size}]
> >     +  is the minimum size of the buffers that will back resources to be
> >     queued.
> >     +\end{description}
> >     +
> >     +The RAW resource capabilities
> >     \field{virtio_video_raw_resources_caps} is
> >     +defined as follows:
> >     +
> >     +\begin{lstlisting}
> >     +struct virtio_video_raw_resources_caps {
> >     +        le32 raw_formats_bitmask; /* Bitmask of
> >     MASK(VIRTIO_VIDEO_RAW_FORMAT_*) */
> >     +        le32 min_resources;
> >     +        le32 max_resources;
> >     +        u8 padding[4];
> >     +};
> >     +\end{lstlisting}
> >     +
> >     +\begin{description}
> >     +\item[\field{raw_formats_bitmask}]
> >     +  specifies raw formats, see
> >     +  \ref{sec:Device Types / Video Device / Supported parameters /
> >     Supported raw formats},
> >     +  to which these capabilities apply.
> >     +\item[\field{min_resources}]
> >     +  is the minimum number of resources that the RAW queue supports
> >     for all
> >     +  the specified raw formats.
> >     +\item[\field{max_resources}]
> >     +  is the maximum number of resources that the RAW queue supports
> >     for all
> >     +  the specified raw formats.
> >     +\end{description}
> >     +
> >     +\devicenormative{\subparagraph}{VIRTIO_VIDEO_CMD_DEVICE_QUERY_CAPS:
> >     Resource capabilities}{Device Types / Video Device / Device
> >     Operation / Device Operation: Device Commands /
> >     VIRTIO_VIDEO_CMD_DEVICE_QUERY_CAPS /
> >     VIRTIO_VIDEO_CMD_DEVICE_QUERY_CAPS: Resource capabilities}
> >     +
> >     +\field{coded_formats_bitmask} MUST be a subset of
> >     \field{coded_formats_bitmask}
> >     +field of \field{struct virtio_video_device_query_caps_resp}.
> >     +
> >     +\field{coded_formats_bitmask} MUST specify at least one coded
> format.
> >     +
> >     +\field{raw_formats_bitmask} MUST be a subset of
> >     \field{raw_formats_bitmask}
> >     +field of \field{struct virtio_video_device_query_caps_resp}.
> >     +
> >     +\field{raw_formats_bitmask} MUST specify at least one raw format.
> >     +
> >     +\field{min_resources} MUST NOT be negative.
> >     +
> >     +\field{max_resources} MUST be greater then or equal to
> >     \field{min_resources}
> >     +within the same struct instance.
> >     +
> >     +\subparagraph{VIRTIO_VIDEO_CMD_DEVICE_QUERY_CAPS: Bitrates}
> >     +
> >     +The bitrate capabilities \field{virtio_video_bitrate_caps} is
> >     +defined as follows:
> >     +
> >     +\begin{lstlisting}
> >     +struct virtio_video_bitrate_caps {
> >     +        le32 coded_formats_bitmask; /* Bitmask of
> >     MASK(VIRTIO_VIDEO_CODED_FORMAT_*) */
> >     +        le32 min_bitrate;
> >     +        le32 max_bitrate;
> >     +        u8 padding[4];
> >     +};
> >     +\end{lstlisting}
> >     +
> >     +\begin{description}
> >     +\item[\field{coded_formats_bitmask}]
> >     +  specifies coded formats, see
> >     +  \ref{sec:Device Types / Video Device / Supported parameters /
> >     Supported coded formats},
> >     +  to which these capabilities apply.
> >     +\item[\field{min_bitrate}]
> >     +  is the minimum bitrate in bits per second supported by the
> >     encoder for all the specified coded
> >     +  formats.
> >     +\item[\field{max_bitrate}]
> >     +  is the maximum bitrate in bits per second supported by the
> >     encoder for all the specified coded
> >     +  formats.
> >     +\end{description}
> >     +
> >     +\devicenormative{\subparagraph}{VIRTIO_VIDEO_CMD_DEVICE_QUERY_CAPS:
> >     Bitrates}{Device Types / Video Device / Device Operation / Device
> >     Operation: Device Commands / VIRTIO_VIDEO_CMD_DEVICE_QUERY_CAPS /
> >     VIRTIO_VIDEO_CMD_DEVICE_QUERY_CAPS: Bitrates}
> >     +
> >     +\field{coded_formats_bitmask} MUST be a subset of
> >     \field{coded_formats_bitmask}
> >     +field of \field{struct virtio_video_device_query_caps_resp}.
> >     +
> >     +\field{coded_formats_bitmask} MUST specify at least one coded
> format.
> >     +
> >     +\field{min_bitrate} MUST NOT be negative.
> >     +
> >     +\field{max_bitrate} MUST be greater then or equal to
> >     \field{min_bitrate}
> >     +within the same \field{struct virtio_video_bitrate_caps} instance.
> >     +
> >     +\subsubsection{Device Operation: Stream commands}
> >     +\label{sec:Device Types / Video Device / Device Operation / Device
> >     Operation: Stream commands}
> >     +
> >     +Stream commands allow the creation, destruction, and flow control
> of a
> >     +stream.
> >     +
> >     +\paragraph{VIRTIO_VIDEO_CMD_STREAM_CREATE}
> >     +\label{sec:Device Types / Video Device / Device Operation / Device
> >     Operation: Stream commands / VIRTIO_VIDEO_CMD_STREAM_CREATE}
> >     +
> >     +Create a new stream using the device.
> >     +
> >     +The driver sends this command with
> >     +\field{struct virtio_video_stream_create}:
> >     +
> >     +\begin{lstlisting}
> >     +struct virtio_video_stream_create {
> >     +        le32 cmd_type; /* VIRTIO_VIDEO_CMD_STREAM_CREATE */
> >     +};
> >     +\end{lstlisting}
> >     +
> >     +The device responds with \field{struct
> >     virtio_video_stream_create_resp}:
> >     +
> >     +\begin{lstlisting}
> >     +struct virtio_video_stream_create_resp {
> >     +        le32 result; /* VIRTIO_VIDEO_RESULT_* */
> >     +        le32 stream_id;
> >     +};
> >     +\end{lstlisting}
> >     +
> >     +\begin{description}
> >     +\item[\field{result}]
> >     +  is
> >     +
> >     +  \begin{description}
> >     +  \item[VIRTIO_VIDEO_RESULT_OK]
> >     +    if the operation succeeded,
> >     +  \item[VIRTIO_VIDEO_RESULT_ERR_OUT_OF_MEMORY]
> >     +    if the limit of simultaneous streams has been reached by the
> >     device and
> >     +    no more can be created.
> >     +  \item[VIRTIO_VIDEO_RESULT_ERR_INVALID_OPERATION]
> >     +    if the stream cannot be created due to an unexpected device
> issue.
> >     +  \end{description}
> >     +\item[\field{stream_id}]
> >     +  is the ID of the created stream allocated by the device.
> >     +\end{description}
> >     +
> >
>  +\devicenormative{\subparagraph}{VIRTIO_VIDEO_CMD_STREAM_CREATE}{Device
> Types / Video Device / Device Operation / Device Operation: Stream commands
> / VIRTIO_VIDEO_CMD_STREAM_CREATE}
> >     +
> >     +\field{stream_id} MUST be set to a device-unique identifier that
> >     remains
> >     +valid as long as the stream is alive.
> >     +
> >     +\paragraph{VIRTIO_VIDEO_CMD_STREAM_DESTROY}
> >     +\label{sec:Device Types / Video Device / Device Operation / Device
> >     Operation: Stream commands / VIRTIO_VIDEO_CMD_STREAM_DESTROY}
> >     +
> >     +% DESTROY has to be more like RESET, not DRAIN, because it is
> >     called, for
> >     +% example, when the guest user-space app closes a file descriptor.
> >     So there
> >     +% is no sense in continuing the processing.
> >     +
> >     +Destroy a video stream and all its resources. Any activity on the
> >     stream
> >     +is halted and all resources are released by the time the delayed
> >     response is
> >     +received by the driver.
> >     +
> >     +The driver sends this command with
> >     +\field{struct virtio_video_stream_destroy}:
> >     +
> >     +\begin{lstlisting}
> >     +struct virtio_video_stream_destroy {
> >     +        le32 cmd_type; /* VIRTIO_VIDEO_CMD_STREAM_DESTROY */
> >     +        le32 stream_id;
> >     +};
> >     +\end{lstlisting}
> >     +
> >     +\begin{description}
> >     +\item[\field{stream_id}]
> >     +  is the ID of the stream to be destroyed, as previously returned by
> >     +  VIRTIO_VIDEO_CMD_STREAM_CREATE.
> >     +\end{description}
> >     +
> >     +The device responds as described in
> >     +\ref{sec:Device Types / Video Device / Device Operation / Device
> >     Operation: Command Virtqueue}
> >     +and begins the background DESTROY operation.
> >     +
> >     +When the command is completed the device sends the
> >     +VIRTIO_VIDEO_DELAYED_RESP_STREAM_DESTROY delayed response, see
> >     +\ref{sec:Device Types / Video Device / Device Operation / Device
> >     Operation: Event Virtqueue}.
> >     +The delayed response can also come in case of unrecoverable stream
> >     error, see
> >     +\ref{sec:Device Types / Video Device / Device Operation / Device
> >     Operation: Standalone Events / Error Event}.
> >     +
> >
>  +\devicenormative{\subparagraph}{VIRTIO_VIDEO_CMD_STREAM_DESTROY}{Device
> Types / Video Device / Device Operation / Device Operation: Stream commands
> / VIRTIO_VIDEO_CMD_STREAM_DESTROY}
> >     +
> >     +Before the device sends a delayed response to
> >     VIRTIO_VIDEO_CMD_STREAM_DESTROY,
> >     +it MUST send all other pending delayed responses with
> >     +VIRTIO_VIDEO_EVENT_FLAG_CANCELED flag set and detach all resources.
> >     +
> >     +After VIRTIO_VIDEO_CMD_STREAM_DESTROY is queued, the device MUST
> >     reply with
> >     +VIRTIO_VIDEO_RESULT_ERR_INVALID_STREAM_ID to any subsequently
> >     queued command
> >     +with this stream ID.
> >     +
> >     +The DESTROY operation MUST NOT be canceled.
> >     +
> >
>  +\drivernormative{\subparagraph}{VIRTIO_VIDEO_CMD_STREAM_DESTROY}{Device
> Types / Video Device / Device Operation / Device Operation: Stream commands
> / VIRTIO_VIDEO_CMD_STREAM_DESTROY}
> >     +
> >     +\field{stream_id} MUST be set to a valid stream ID previously
> returned
> >     +by VIRTIO_VIDEO_CMD_STREAM_CREATE.
> >     +
> >     +The driver MUST stop using \field{stream_id} as a valid stream
> after it
> >     +received the delayed response to this command.
> >     +
> >     +\paragraph{VIRTIO_VIDEO_CMD_STREAM_SET_PARAMS}
> >     +\label{sec:Device Types / Video Device / Device Operation / Device
> >     Operation: Stream commands / VIRTIO_VIDEO_CMD_STREAM_SET_PARAMS}
> >     +
> >     +Write values of selected parameters of a given stream, and receive
> >     back the
> >     +values for all the parameters supported by the device as reported by
> >     +\ref{sec:Device Types / Video Device / Device Operation / Device
> >     Operation: Device Commands / VIRTIO_VIDEO_CMD_DEVICE_QUERY_CAPS}.
> >     +The operation can be either executed immediately, or queued into
> >     the INPUT
> >     +queue, i.e. after processing all the INPUT queue elements that are
> >     queued
> >     +before the command.
> >     +
> >     +The driver sends this command with
> >     +\field{struct virtio_video_stream_set_params}:
> >     +
> >     +\begin{lstlisting}
> >     +#define VIRTIO_VIDEO_SET_PARAMS_FLAG_IN_BAND  (1 << 0)
> >     +
> >     +struct virtio_video_stream_set_params {
> >     +        le32 cmd_type; /* VIRTIO_VIDEO_CMD_STREAM_SET_PARAMS */
> >     +        le32 stream_id;
> >     +        le32 flags; /* Bitmask of VIRTIO_VIDEO_SET_PARAMS_FLAG_* */
> >     +        u8 padding[4];
> >     +        struct virtio_video_params params;
> >     +};
> >     +\end{lstlisting}
> >     +
> >     +\begin{description}
> >     +\item[\field{stream_id}]
> >     +  is the ID of the stream we want to set a parameter for.
> >     +\item[\field{flags}]
> >     +  is a bitmask of VIRTIO_VIDEO_SET_PARAMS_FLAG_* values.
> >     +
> >     +  \begin{description}
> >     +  \item[\field{VIRTIO_VIDEO_SET_PARAMS_FLAG_IN_BAND}]
> >     +    The submitted parameters are to be set only after all of the
> >     previously
> >     +    queued INPUT queue elements are processed. Without this flag the
> >     +    parameters are set Immediately.
> >     +  \end{description}
> >     +\item[\field{params}]
> >     +  is a container for the selected stream parameters to be set.
> >     +\end{description}
> >     +
> >     +The device responds as described in
> >     +\ref{sec:Device Types / Video Device / Device Operation / Device
> >     Operation: Command Virtqueue}
> >     +and begins the background SET_PARAMS operation.
> >     +
> >     +When the background processing of the resource is completed the
> >     device sends
> >     +the VIRTIO_VIDEO_DELAYED_RESP_STREAM_SET_PARAMS delayed response,
> see
> >     +\ref{sec:Device Types / Video Device / Device Operation / Device
> >     Operation: Event Virtqueue}.
> >     +The delayed response can also come in case of dynamic parameters
> >     change, see
> >     +\ref{sec:Device Types / Video Device / Device Operation / Device
> >     Operation: Standalone Events / Dynamic Parameters Change Event}.
> >     +
> >     +The command-specific delayed response
> >     +\field{struct virtio_video_stream_set_params_delayed_resp} is
> defined
> >     +as follows:
> >     +
> >     +\begin{lstlisting}
> >     +struct virtio_video_stream_set_params_delayed_resp {
> >     +        struct virtio_video_params params;
> >     +};
> >     +\end{lstlisting}
> >     +
> >     +\begin{description}
> >     +\item[\field{params}]
> >     +  is a container for the actual values of all the parameters
> >     supported by the
> >     +  device. The values set by the device may differ from the
> >     requested values
> >     +  depending on the device's capabilities.
> >     +\end{description}
> >     +
> >     +The \field{struct virtio_video_params} is defined as follows:
> >     +
> >     +\begin{lstlisting}
> >     +struct virtio_video_raw_format {
> >     +        le32 format;
> >     +        le32 planes_layout; /* VIRTIO_VIDEO_PLANES_LAYOUT_* */
> >     +        le32 stride;
> >     +        le32 width;
> >     +        le32 height;
> >     +        u8 padding[4];
> >     +};
> >     +
> >     +struct virtio_video_param_crop {
> >     +        le32 left;
> >     +        le32 top;
> >     +        le32 width;
> >     +        le32 height;
> >     +};
> >     +
> >     +union virtio_video_codec_params {
> >     +        struct virtio_video_mpeg2_params mpeg2;
> >     +        struct virtio_video_mpeg4_params mpeg4;
> >     +        struct virtio_video_h264_params h264;
> >     +        struct virtio_video_hevc_params hevc;
> >     +        struct virtio_video_vp8_params vp8;
> >     +        struct virtio_video_vp9_params vp9;
> >     +};
> >     +
> >     +struct virtio_video_params {
> >     +        le32 stream_params_bitmask; /* Bitmask of
> >     MASK(VIRTIO_VIDEO_PARAM_*) */
> >     +        le32 coded_format; /* If
> >     MASK(VIRTIO_VIDEO_PARAM_CODED_FORMAT) is set. */
> >     +        /* If MASK(VIRTIO_VIDEO_PARAM_RAW_FORMAT) is set. */
> >     +        struct virtio_video_raw_format raw_format;
> >     +        /* If MASK(VIRTIO_VIDEO_PARAM_CODED_RESOURCES) is set. */
> >     +        struct virtio_video_param_resources coded_resources;
> >     +        /* If MASK(VIRTIO_VIDEO_PARAM_RAW_RESOURCES) is set. */
> >     +        struct virtio_video_param_resources raw_resources;
> >     +        struct virtio_video_param_crop crop; /* If
> >     MASK(VIRTIO_VIDEO_PARAM_CROP) is set. */
> >     +        le32 bitrate; /* If MASK(VIRTIO_VIDEO_PARAM_BITRATE) is
> set. */
> >     +        u8 padding[4];
> >     +        /* If the corresponding
> >     MASK(VIRTIO_VIDEO_PARAM_GROUP_CODEC_*) is set
> >     +        * depending on the coded_format. */
> >     +        union virtio_video_codec_params codec;
> >     +};
> >     +\end{lstlisting}
> >
> >
> > This is a bit difficult to read for me, as some of the *if* comments are
> > written AFTER
> > the member definition, and some others BEFORE the member defintion.
> > Above, when introducing the VIRTIO_VIDEO_CMD_DEVICE_QUERY_CAPS command,
> > you use this format for the conditional fields:
> > /**
> > * Followed by if MASK(VIRTIO_VIDEO_PARAM_BITRATE) is set
> > * struct virtio_video_bitrate_caps bitrate_caps[num_bitrate_caps];
> > */
> > It spaces a bit more the members, and leaves them as part of the
> > comment block. I would suggest to keep the format consistent.
>
> Ack, I'll try to make it more readable.
> Also given the extendability concerns, that I explained above, I'd like
> to maybe finish combining all the parameters into groups and then to
> make the SET_PARAMS command operating on the parameter group level.
> Still thinking about this...
>
> --
> Alexander Gordeev
> Senior Software Engineer
>
> OpenSynergy GmbH
> Rotherstr. 20, 10245 Berlin
>
> Phone: +49 30 60 98 54 0 - 88
> Fax: +49 (30) 60 98 54 0 - 99
> EMail: alexander.gordeev at opensynergy.com
>
> www.opensynergy.com
>
> Handelsregister/Commercial Registry: Amtsgericht Charlottenburg, HRB
> 108616B
> Geschäftsführer/Managing Director: Régis Adjamah
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20230726/9f180b8e/attachment.htm>


More information about the libcamera-devel mailing list