OASIS Mailing List ArchivesView the OASIS mailing list archive below
or browse/search using MarkMail.

 


Help: OASIS Mailing Lists Help | MarkMail Help

virtio-comment message

[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]


Subject: Re: [RFC PATCH v8 1/1] virtio-video: Add virtio video device specification


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@opensynergy.com <mailto:Alexander.Gordeev@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@opensynergy.com
    <mailto:Alexander.Gordeev@opensynergy.com>>
    Co-authored-by: Keiichi Watanabe <keiichiw@chromium.org
    <mailto:keiichiw@chromium.org>>
    Co-authored-by: Alexandre Courbot <acourbot@chromium.org
    <mailto:acourbot@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?

    +\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@opensynergy.com

www.opensynergy.com

Handelsregister/Commercial Registry: Amtsgericht Charlottenburg, HRB 108616B
GeschÃftsfÃhrer/Managing Director: RÃgis Adjamah


[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]