<div dir="ltr"><div dir="ltr"><div><div dir="ltr" class="gmail_signature"><div dir="ltr"><br></div></div></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Mon, Jul 10, 2023 at 10:52 AM Alexander Gordeev <<a href="mailto:alexander.gordeev@opensynergy.com" target="_blank">alexander.gordeev@opensynergy.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Hi Albert,<br>
<br>
On 06.07.23 16:59, Albert Esteve wrote:<br>
> Hi Alexander,<br>
> <br>
> Thanks for the patch! It is a long document, so I skimmed a bit in the <br>
> first read. Find some comments/questions inlined.<br>
> I will give it a second deeper read soon, but overall I think is in <br>
> quite good shape. It feels really matured.<br>
<br>
Great! Thank you for taking the time to review it.<br>
<br>
> On Thu, Jun 29, 2023 at 4:49 PM Alexander Gordeev <br>
> <<a href="mailto:Alexander.Gordeev@opensynergy.com" target="_blank">Alexander.Gordeev@opensynergy.com</a> <br>
> <mailto:<a href="mailto:Alexander.Gordeev@opensynergy.com" target="_blank">Alexander.Gordeev@opensynergy.com</a>>> wrote:<br>
> <br>
> Add the specification of the video decoder and encoder devices, which<br>
> can be used to provide host-accelerated video operations to the guest.<br>
> <br>
> Signed-off-by: Alexander Gordeev <<a href="mailto:Alexander.Gordeev@opensynergy.com" target="_blank">Alexander.Gordeev@opensynergy.com</a><br>
> <mailto:<a href="mailto:Alexander.Gordeev@opensynergy.com" target="_blank">Alexander.Gordeev@opensynergy.com</a>>><br>
> Co-authored-by: Keiichi Watanabe <<a href="mailto:keiichiw@chromium.org" target="_blank">keiichiw@chromium.org</a><br>
> <mailto:<a href="mailto:keiichiw@chromium.org" target="_blank">keiichiw@chromium.org</a>>><br>
> Co-authored-by: Alexandre Courbot <<a href="mailto:acourbot@chromium.org" target="_blank">acourbot@chromium.org</a><br>
> <mailto:<a href="mailto:acourbot@chromium.org" target="_blank">acourbot@chromium.org</a>>><br>
> ---<br>
> conformance.tex | 4 +<br>
> content.tex | 1 +<br>
> device-types/video/description.tex | 2040 +++++++++++++++++++++<br>
> device-types/video/device-conformance.tex | 22 +<br>
> device-types/video/driver-conformance.tex | 16 +<br>
> introduction.tex | 21 +<br>
> 6 files changed, 2104 insertions(+)<br>
> create mode 100644 device-types/video/description.tex<br>
> create mode 100644 device-types/video/device-conformance.tex<br>
> create mode 100644 device-types/video/driver-conformance.tex<br>
> <br>
> diff --git a/conformance.tex b/conformance.tex<br>
> index 01ccd69..d719eda 100644<br>
> --- a/conformance.tex<br>
> +++ b/conformance.tex<br>
> @@ -34,6 +34,7 @@ \section{Conformance<br>
> Targets}\label{sec:Conformance / Conformance Targets}<br>
> \ref{sec:Conformance / Driver Conformance / SCMI Driver Conformance},<br>
> \ref{sec:Conformance / Driver Conformance / GPIO Driver<br>
> Conformance} or<br>
> \ref{sec:Conformance / Driver Conformance / PMEM Driver Conformance}.<br>
> +\ref{sec:Conformance / Driver Conformance / Video Driver Conformance},<br>
> <br>
> \item Clause \ref{sec:Conformance / Legacy Interface:<br>
> Transitional Device and Transitional Driver Conformance}.<br>
> \end{itemize}<br>
> @@ -61,6 +62,7 @@ \section{Conformance<br>
> Targets}\label{sec:Conformance / Conformance Targets}<br>
> \ref{sec:Conformance / Device Conformance / SCMI Device Conformance},<br>
> \ref{sec:Conformance / Device Conformance / GPIO Device<br>
> Conformance} or<br>
> \ref{sec:Conformance / Device Conformance / PMEM Device Conformance}.<br>
> +\ref{sec:Conformance / Device Conformance / Video Device Conformance},<br>
> <br>
> \item Clause \ref{sec:Conformance / Legacy Interface:<br>
> Transitional Device and Transitional Driver Conformance}.<br>
> \end{itemize}<br>
> @@ -152,6 +154,7 @@ \section{Conformance<br>
> Targets}\label{sec:Conformance / Conformance Targets}<br>
> \input{device-types/scmi/driver-conformance.tex}<br>
> \input{device-types/gpio/driver-conformance.tex}<br>
> \input{device-types/pmem/driver-conformance.tex}<br>
> +\input{device-types/video/driver-conformance.tex}<br>
> <br>
> \conformance{\section}{Device Conformance}\label{sec:Conformance /<br>
> Device Conformance}<br>
> <br>
> @@ -238,6 +241,7 @@ \section{Conformance<br>
> Targets}\label{sec:Conformance / Conformance Targets}<br>
> \input{device-types/scmi/device-conformance.tex}<br>
> \input{device-types/gpio/device-conformance.tex}<br>
> \input{device-types/pmem/device-conformance.tex}<br>
> +\input{device-types/video/device-conformance.tex}<br>
> <br>
> \conformance{\section}{Legacy Interface: Transitional Device and<br>
> Transitional Driver Conformance}\label{sec:Conformance / Legacy<br>
> Interface: Transitional Device and Transitional Driver Conformance}<br>
> A conformant implementation MUST be either transitional or<br>
> diff --git a/content.tex b/content.tex<br>
> index d2ab9eb..90708d7 100644<br>
> --- a/content.tex<br>
> +++ b/content.tex<br>
> @@ -765,6 +765,7 @@ \chapter{Device Types}\label{sec:Device Types}<br>
> \input{device-types/scmi/description.tex}<br>
> \input{device-types/gpio/description.tex}<br>
> \input{device-types/pmem/description.tex}<br>
> +\input{device-types/video/description.tex}<br>
> <br>
> \chapter{Reserved Feature Bits}\label{sec:Reserved Feature Bits}<br>
> <br>
> diff --git a/device-types/video/description.tex<br>
> b/device-types/video/description.tex<br>
> new file mode 100644<br>
> index 0000000..760df7f<br>
> --- /dev/null<br>
> +++ b/device-types/video/description.tex<br>
> @@ -0,0 +1,2040 @@<br>
> +\section{Video Device}<br>
> +\label{sec:Device Types / Video Device}<br>
> +<br>
> +The virtio video encoder and decoder devices provide support for<br>
> +host-accelerated video encoding and decoding. Despite being different<br>
> +device types, they use the same protocol and general flow.<br>
> +<br>
> +\subsection{Device ID}<br>
> +\label{sec:Device Types / Video Device / Device ID}<br>
> +<br>
> +\begin{description}<br>
> +\item[30]<br>
> + encoder device<br>
> +\item[31]<br>
> + decoder device<br>
> +\end{description}<br>
> +<br>
> +\subsection{Virtqueues}<br>
> +\label{sec:Device Types / Video Device / Virtqueues}<br>
> +<br>
> +\begin{description}<br>
> +\item[0]<br>
> + commandq - queue for driver commands and device responses to<br>
> these commands<br>
> +\item[1]<br>
> + eventq - queue for device delayed responses to commands and<br>
> standalone<br>
> + device events<br>
> +\end{description}<br>
> +<br>
> +\subsection{Feature bits}<br>
> +\label{sec:Device Types / Video Device / Feature bits}<br>
> +<br>
> +\begin{description}<br>
> +\item[VIRTIO_VIDEO_F_RESOURCE_GUEST_PAGES (0)]<br>
> + Guest pages can be used as the backing memory of resources.<br>
> +\item[VIRTIO_VIDEO_F_RESOURCE_NON_CONTIG (1)]<br>
> + The device can use non-contiguous guest memory as the backing<br>
> memory of<br>
> + resources. Only meaningful if VIRTIO_VIDEO_F_RESOURCE_GUEST_PAGES<br>
> is also<br>
> + set.<br>
> +\item[VIRTIO_VIDEO_F_RESOURCE_VIRTIO_OBJECT (2)]<br>
> + Objects exported by another virtio device can be used as the<br>
> backing memory<br>
> + of resources.<br>
> +\item[VIRTIO_VIDEO_F_RESOURCE_DYNAMIC (3)]<br>
> + The device supports re-attaching memory to resources while streaming.<br>
> +% TODO this flag is not used anywhere at the moment.<br>
> +% Might be necessary with Android.<br>
> +\end{description}<br>
> <br>
> <br>
> Maybe I am missing part of the previous dicussion, but is this <br>
> VIRTIO_VIDEO_F_RESOURCE_DYNAMIC flag new?<br>
> Not sure what re-attaching memory means and what the flag is supossed to do.<br>
> In the comment you mention specifically Android, so it got me curious. <br>
> What is the usecase for this feature flag?<br>
<br>
This flag first appeared in draft v6, but it is still not used anywhere <br>
in the text. I'm almost sure I know why it was added by Alexandre. Our <br>
experince with Android's Codec2 and v4l2_codec2 shows that something <br>
like this can happen:<br>
<br>
1. The V4L2 device is opened. For example, a decoder.<br>
2. All parameters and buffers are set up. Both queues are started.<br>
3. A buffer is queued and then dequeued later. From this moment it <br>
"belongs" to the guest user-space.<br>
4. This opportunity is used to detach the buffer's memory and attach <br>
some different chunks of memory.<br>
5. Then the buffer is queued again. And so on.<br>
<br>
The problem here is that according to V4L2 conventions the old memory is <br>
expected to not be modified after it is dequeued because it could be <br>
used as a reference in the decoding process. Unfortunately these <br>
conventions were rather informal until recently AFAIU. Here is the <br>
patch, that adds this and an interesting discussuin related to the <br>
issue: <br>
<a href="https://lore.kernel.org/all/20211018091427.88468-1-acourbot@chromium.org/" rel="noreferrer" target="_blank">https://lore.kernel.org/all/20211018091427.88468-1-acourbot@chromium.org/</a><br>
<br>
So the way it is done in Android could be problematic. So the driver <br>
should probably forbid the reattaching unless the queue is stopped. But <br>
if the virtio video device is, for example, always doing a pixel format <br>
conversion after the decoding, then it is OK to reattach the memory. In <br>
this case the device can advertise the VIRTIO_VIDEO_F_RESOURCE_DYNAMIC <br>
flag to notify the driver, that the memory could be safely reattached. <br>
Maybe this is not the best way to deal with this, see the discussion at <br>
the link above. At least it is not very invasive.<br>
<br>
> +<br>
> +\devicenormative{\subsubsection}{Feature bits}{Device Types / Video<br>
> Device / Feature bits}<br>
> +<br>
> +The device MUST set at least one of<br>
> VIRTIO_VIDEO_F_RESOURCE_GUEST_PAGES or<br>
> +VIRTIO_VIDEO_F_RESOURCE_VIRTIO_OBJECT, since the absence of both<br>
> bits would<br>
> +mean that no memory can be used at all for resources.<br>
> +<br>
> +The device MUST NOT set VIRTIO_VIDEO_F_RESOURCE_NON_CONTIG unless<br>
> it also sets<br>
> +VIRTIO_VIDEO_F_RESOURCE_GUEST_PAGES.<br>
> +<br>
> +\drivernormative{\subsubsection}{Feature bits}{Device Types / Video<br>
> Device / Feature bits}<br>
> +<br>
> +The driver MUST negotiate at least one of the<br>
> +VIRTIO_VIDEO_F_RESOURCE_GUEST_PAGES and<br>
> VIRTIO_VIDEO_F_RESOURCE_VIRTIO_OBJECT<br>
> +features.<br>
> +<br>
> +If VIRTIO_VIDEO_F_RESOURCE_GUEST_PAGES has been negotiated, but not<br>
> +VIRTIO_VIDEO_F_RESOURCE_NON_CONTIG, the driver MUST use physically<br>
> +contiguous memory for all the buffers it allocates.<br>
> +<br>
> +\subsection{Device configuration layout}<br>
> +\label{sec:Device Types / Video Device / Device configuration layout}<br>
> +<br>
> +The video device configuration space uses the following layout:<br>
> +<br>
> +\begin{lstlisting}<br>
> +struct virtio_video_config {<br>
> + le32 caps_length;<br>
> +};<br>
> +\end{lstlisting}<br>
> +<br>
> +\begin{description}<br>
> +\item[\field{caps_length}]<br>
> + is the minimum length in bytes that a device-writable buffer must<br>
> have<br>
> + in order to receive the response to<br>
> VIRTIO_VIDEO_CMD_DEVICE_QUERY_CAPS, see<br>
> + \ref{sec:Device Types / Video Device / Device Operation / Device<br>
> Operation: Device Commands / VIRTIO_VIDEO_CMD_DEVICE_QUERY_CAPS}.<br>
> +\end{description}<br>
> +<br>
> +\devicenormative{\subsubsection}{Device configuration<br>
> layout}{Device Types / Video Device / Device configuration layout}<br>
> +<br>
> +The device MUST set the \field{caps_length} field to a value equal to<br>
> +the response size of VIRTIO_VIDEO_CMD_DEVICE_QUERY_CAPS.<br>
> +<br>
> +\subsection{Supported parameters}<br>
> +\label{sec:Device Types / Video Device / Supported parameters}<br>
> +<br>
> +\subsubsection{Supported coded formats}<br>
> +\label{sec:Device Types / Video Device / Supported parameters /<br>
> Supported coded formats}<br>
> +<br>
> +The following coded formats are defined:<br>
> +\begin{lstlisting}<br>
> +#define VIRTIO_VIDEO_CODED_FORMAT_MPEG2 1 /* MPEG-2 Part 2 <br>
> (V4L2_PIX_FMT_MPEG2) */<br>
> +#define VIRTIO_VIDEO_CODED_FORMAT_MPEG4 2 /* MPEG-4 Part 2 <br>
> (V4L2_PIX_FMT_MPEG4) */<br>
> +#define VIRTIO_VIDEO_CODED_FORMAT_H264 3 /* H.264 <br>
> (V4L2_PIX_FMT_H264) */<br>
> +#define VIRTIO_VIDEO_CODED_FORMAT_HEVC 4 /* HEVC aka H.265<br>
> (V4L2_PIX_FMT_HEVC) */<br>
> +#define VIRTIO_VIDEO_CODED_FORMAT_VP8 5 /* VP8 <br>
> (V4L2_PIX_FMT_VP8) */<br>
> +#define VIRTIO_VIDEO_CODED_FORMAT_VP9 6 /* VP9 <br>
> (V4L2_PIX_FMT_VP9) */<br>
> +\end{lstlisting}<br>
> +<br>
> +The above constants have two usages:<br>
> +\begin{enumerate}<br>
> +\item As bit numbers, used to tell the driver which coded formats are<br>
> +supported by the device, see<br>
> +\ref{sec:Device Types / Video Device / Device Operation / Device<br>
> Operation: Device Commands / VIRTIO_VIDEO_CMD_DEVICE_QUERY_CAPS}.<br>
> +\item As values, used to designate the coded format when working with<br>
> +stream parameters, see<br>
> +\ref{sec:Device Types / Video Device / Device Operation / Device<br>
> Operation: Stream commands / VIRTIO_VIDEO_CMD_STREAM_SET_PARAMS}.<br>
> +\end{enumerate}<br>
> +<br>
> +The coded formats and the expected data units per buffer are<br>
> documented in<br>
> +\hyperref[intro:V4L2]{V4L2 header} and<br>
> +\hyperref[intro:V4L2 compressed]{V4L2 compressed formats<br>
> documentation}.<br>
> +<br>
> +\subsubsection{Supported raw formats}<br>
> +\label{sec:Device Types / Video Device / Supported parameters /<br>
> Supported raw formats}<br>
> +<br>
> +The following raw formats are defined:<br>
> +\begin{lstlisting}<br>
> +#define VIRTIO_VIDEO_RAW_FORMAT_ARGB8888 1 /* DRM_FORMAT_ARGB8888<br>
> / V4L2_PIX_FMT_ABGR32 */<br>
> +#define VIRTIO_VIDEO_RAW_FORMAT_BGRA8888 2 /* DRM_FORMAT_BGRA8888<br>
> / V4L2_PIX_FMT_ARGB32 */<br>
> +#define VIRTIO_VIDEO_RAW_FORMAT_RGBA8888 3 /* DRM_FORMAT_RGBA8888<br>
> / V4L2_PIX_FMT_BGRA32 */<br>
> +#define VIRTIO_VIDEO_RAW_FORMAT_NV12 4 /* DRM_FORMAT_NV12 <br>
> / V4L2_PIX_FMT_NV12 */<br>
> +#define VIRTIO_VIDEO_RAW_FORMAT_YUV420 5 /* DRM_FORMAT_YUV420 <br>
> / V4L2_PIX_FMT_YUV420 */<br>
> +#define VIRTIO_VIDEO_RAW_FORMAT_YVU420 6 /* DRM_FORMAT_YVU420 <br>
> / V4L2_PIX_FMT_YVU420 */<br>
> +#define VIRTIO_VIDEO_RAW_FORMAT_YUYV 7 /* DRM_FORMAT_YUYV <br>
> / V4L2_PIX_FMT_YUYV */<br>
> +\end{lstlisting}<br>
> +<br>
> +The above constants have two usages:<br>
> +\begin{enumerate}<br>
> +\item As bit numbers, used to tell the driver which raw formats are<br>
> +supported by the device, see<br>
> +\ref{sec:Device Types / Video Device / Device Operation / Device<br>
> Operation: Device Commands / VIRTIO_VIDEO_CMD_DEVICE_QUERY_CAPS}.<br>
> +\item As values, used to designate the raw format when working with<br>
> +stream parameters, see<br>
> +\ref{sec:Device Types / Video Device / Device Operation / Device<br>
> Operation: Stream commands / VIRTIO_VIDEO_CMD_STREAM_SET_PARAMS}.<br>
> +\end{enumerate}<br>
> +<br>
> +The layouts of raw formats are documented in \hyperref[intro:DRM<br>
> formats]{DRM}<br>
> +and \hyperref[intro:V4L2]{V4L2} headers, as well as in<br>
> +\hyperref[intro:V4L2 RGB]{V4L2 RGB} and<br>
> +\hyperref[intro:V4L2 YUV]{planar YUV} formats documentation.<br>
> +<br>
> +\subsubsection{Supported stream parameters}<br>
> +\label{sec:Device Types / Video Device / Supported parameters /<br>
> Supported stream parameters}<br>
> +<br>
> +The following stream parameters are defined:<br>
> +\begin{lstlisting}<br>
> +#define VIRTIO_VIDEO_PARAM_CODED_FORMAT 1<br>
> +#define VIRTIO_VIDEO_PARAM_RAW_FORMAT 2<br>
> +#define VIRTIO_VIDEO_PARAM_CODED_RESOURCES 3<br>
> +#define VIRTIO_VIDEO_PARAM_RAW_RESOURCES 4<br>
> +#define VIRTIO_VIDEO_PARAM_CROP 5<br>
> +#define VIRTIO_VIDEO_PARAM_BITRATE 6 /* Same as<br>
> V4L2_CID_MPEG_VIDEO_BITRATE */<br>
> +#define VIRTIO_VIDEO_PARAM_GROUP_CODEC_MPEG2 7<br>
> +#define VIRTIO_VIDEO_PARAM_GROUP_CODEC_MPEG4 8<br>
> +#define VIRTIO_VIDEO_PARAM_GROUP_CODEC_H264 9<br>
> +#define VIRTIO_VIDEO_PARAM_GROUP_CODEC_HEVC 10<br>
> +#define VIRTIO_VIDEO_PARAM_GROUP_CODEC_VP8 11<br>
> +#define VIRTIO_VIDEO_PARAM_GROUP_CODEC_VP9 12<br>
> +\end{lstlisting}<br>
> +% TODO acourbot: See b/241492607 (fractional frame rates??)<br>
> +<br>
> +The above constants have two usages:<br>
> +\begin{enumerate}<br>
> +\item As bit numbers, used to tell the driver which stream<br>
> parameters are<br>
> +supported by the device, see<br>
> +\ref{sec:Device Types / Video Device / Device Operation / Device<br>
> Operation: Device Commands / VIRTIO_VIDEO_CMD_DEVICE_QUERY_CAPS}.<br>
> +\item As values, used to designate the stream parameters when<br>
> working with<br>
> +them, see<br>
> +\ref{sec:Device Types / Video Device / Device Operation / Device<br>
> Operation: Stream commands / VIRTIO_VIDEO_CMD_STREAM_SET_PARAMS}.<br>
> +\end{enumerate}<br>
> +<br>
> +\subsection{Device Initialization}<br>
> +\label{sec:Device Types / Video Device / Device Initialization}<br>
> +<br>
> +\begin{enumerate}<br>
> +\def\labelenumi{\arabic{enumi}.}<br>
> +\item<br>
> + The driver reads the feature bits and negotiates the features it<br>
> needs.<br>
> +\item<br>
> + The driver sets up the commandq and the eventq.<br>
> +\item<br>
> + The driver reads the \field{caps_length} field of the configuration<br>
> + space and prepares a buffer of at least that size.<br>
> +\item<br>
> + The driver sends that buffer on the commandq with the<br>
> + VIRTIO_VIDEO_CMD_DEVICE_QUERY_CAPS command.<br>
> +\item<br>
> + The driver receives the response from the device, and parses its<br>
> capabilities.<br>
> +\end{enumerate}<br>
> +<br>
> +\subsection{Device Operation}<br>
> +\label{sec:Device Types / Video Device / Device Operation}<br>
> +<br>
> +The commandq is used by the driver to send commands to the device<br>
> and to<br>
> +receive the device's responses via used buffers. The eventq is used<br>
> by the<br>
> +device to send the device's delayed responses to commands and<br>
> standalone<br>
> +device events.<br>
> +<br>
> +The driver can create new streams using the<br>
> +VIRTIO_VIDEO_CMD_STREAM_CREATE command. Each stream has two resource<br>
> +queues (not to be confused with the virtio queues) called INPUT and<br>
> +OUTPUT, when the direction of the data flow matters. The INPUT<br>
> queue accepts<br>
> +driver-filled input data for the device (coded data for a decoder;<br>
> +input frames for an encoder), while the OUTPUT queue receives<br>
> resources to be<br>
> +filled by the device as a result of processing the INPUT queue<br>
> (decoded raw<br>
> +frames for a decoder; encoded data for an encoder).<br>
> +<br>
> +These same queues can be also called CODED and RAW, when their<br>
> content matters.<br>
> +The CODED queue is used to transfer compressed video data (INPUT<br>
> for a decoder;<br>
> +OUTPUT for an encoder), while the RAW queue is used to transfer raw<br>
> frames<br>
> +(OUTPUT for a decoder; INPUT for an encoder).<br>
> +<br>
> +The INPUT and OUTPUT queues are effectively independent, and the driver<br>
> +can fill them without caring about the other queue. In particular there<br>
> +is no need to queue input and output resources in pairs: one input<br>
> +resource can result in zero to many produced output resources.<br>
> +<br>
> +A resource is a set of memory buffers that contain a unit of data that<br>
> +the device can process or produce. Most resources will only have one<br>
> +buffer (like coded data and single-planar raw frames), but frames<br>
> using a<br>
> +multi-planar format can have several.<br>
> +<br>
> +Parameters allow the driver to configure the stream for the decoding or<br>
> +encoding operation. The parameters can be obtained and configured using<br>
> +VIRTIO_VIDEO_CMD_STREAM_SET_PARAMS. Available parameters depend on<br>
> +the device type and are detailed in section<br>
> +\ref{sec:Device Types / Video Device / Supported parameters /<br>
> Supported stream parameters}.<br>
> +<br>
> +Before resources can be submitted to a queue, backing memory must be<br>
> +attached to them using VIRTIO_VIDEO_CMD_RESOURCE_ATTACH_BACKING.<br>
> +The exact form of that memory is negotiated using the feature flags.<br>
> +<br>
> +In the case of a decoder device, the decoded frames are made available<br>
> +on the OUTPUT queue in presentation order.<br>
> +<br>
> +Resources are queued to the INPUT or OUTPUT queue using the<br>
> +VIRTIO_VIDEO_CMD_RESOURCE_QUEUE command. The device sends a delayed<br>
> response<br>
> +to this command when an input resource has been fully processed and<br>
> can be<br>
> +reused by the driver, or when an output resource has been filled by the<br>
> +device as a result of processing.<br>
> +<br>
> +The device can detect stream-related events that require intervention<br>
> +from the driver and signals them on the eventq, see<br>
> +\ref{sec:Device Types / Video Device / Device Operation / Device<br>
> Operation: Standalone Events}.<br>
> +One example is a dynamic parameters change while decoding a stream,<br>
> which<br>
> +may require the driver to reallocate the backing memory of its output<br>
> +resources to fit the new resolution.<br>
> +<br>
> +% RESET and DRAIN have essentially the same outcome: all the input<br>
> +% resources queued before the command are released, there are no<br>
> related<br>
> +% output resources in the decoder/encoder, the dequeued output<br>
> resources<br>
> +% can't be used as a reference by the device. So the other<br>
> requirements should<br>
> +% be reasonably similar.<br>
> +% Use-case: playback in a loop from second 1 till the end of file.<br>
> +<br>
> +% TODO put some examples in the comments<br>
> +<br>
> +\subsubsection{Device Operation: Command Virtqueue}<br>
> +\label{sec:Device Types / Video Device / Device Operation / Device<br>
> Operation: Command Virtqueue}<br>
> +<br>
> +This section details the commands that can be sent on the commandq by<br>
> +the driver, as well as the responses that the device will write.<br>
> +<br>
> +Different structures are used for each command and response. A command<br>
> +structure starts with the requested command code, defined as follows:<br>
> +<br>
> +\begin{lstlisting}<br>
> +/* Device */<br>
> +#define VIRTIO_VIDEO_CMD_DEVICE_QUERY_CAPS 0x100<br>
> +<br>
> +/* Stream */<br>
> +#define VIRTIO_VIDEO_CMD_STREAM_CREATE 0x200<br>
> +#define VIRTIO_VIDEO_CMD_STREAM_DESTROY 0x201<br>
> +#define VIRTIO_VIDEO_CMD_STREAM_SET_PARAMS 0x202<br>
> <br>
> <br>
> Why not having a GET_PARAMS counterpart?<br>
<br>
With the current approach to setting the parameters GET_PARAMS would be <br>
almost exactly the same as SET_PARAMS if params.stream_params_bitmask is <br>
set to zero. So I assumed that it is better to save the space and remove <br>
the GET_PARAMS counterpart since the length of the spec is already a <br>
sensitive topic.<br>
<br>
> +#define VIRTIO_VIDEO_CMD_STREAM_DRAIN 0x203<br>
> +#define VIRTIO_VIDEO_CMD_STREAM_RESET 0x204<br>
> +<br>
> +/* Resource */<br>
> +#define VIRTIO_VIDEO_CMD_RESOURCE_ATTACH_BACKING 0x300<br>
> +#define VIRTIO_VIDEO_CMD_RESOURCE_QUEUE 0x301<br>
> +\end{lstlisting}<br>
> +<br>
> +A response structure starts with the result of the requested command,<br>
> +defined as follows:<br>
> +<br>
> +\begin{lstlisting}<br>
> +/* Success */<br>
> +#define VIRTIO_VIDEO_RESULT_OK 0x000<br>
> +#define VIRTIO_VIDEO_RESULT_OK_DELAYED 0x001<br>
> +<br>
> +/* Error */<br>
> +#define VIRTIO_VIDEO_RESULT_ERR_INVALID_COMMAND 0x100<br>
> +#define VIRTIO_VIDEO_RESULT_ERR_INVALID_OPERATION 0x101<br>
> +#define VIRTIO_VIDEO_RESULT_ERR_INVALID_STREAM_ID 0x102<br>
> +#define VIRTIO_VIDEO_RESULT_ERR_INVALID_RESOURCE_ID 0x103<br>
> +#define VIRTIO_VIDEO_RESULT_ERR_INVALID_ARGUMENT 0x104<br>
> +#define VIRTIO_VIDEO_RESULT_ERR_OUT_OF_MEMORY 0x105<br>
> +\end{lstlisting}<br>
> +<br>
> +For response structures carrying an error code, the rest of the<br>
> +structure is considered invalid.<br>
> +<br>
> +For all commands beginning background operations and returning delayed<br>
> +responses over eventq, the command response is defined as follows:<br>
> +<br>
> +\begin{lstlisting}<br>
> +#define VIRTIO_VIDEO_INVALID_RESPONSE_ID 0xffffffff<br>
> +<br>
> +struct virtio_video_command_resp_delayed_common {<br>
> + le32 result; /* VIRTIO_VIDEO_RESULT_* */<br>
> + le32 delayed_response_id;<br>
> +};<br>
> +\end{lstlisting}<br>
> +<br>
> +\begin{description}<br>
> +\item[\field{result}]<br>
> + is<br>
> +<br>
> + \begin{description}<br>
> + \item[VIRTIO_VIDEO_RESULT_OK_DELAYED]<br>
> + if the command started the desired background operation<br>
> successfully,<br>
> + \item[VIRTIO_VIDEO_RESULT_ERR_INVALID_STREAM_ID]<br>
> + if the mentioned stream ID does not exist,<br>
> + \item[VIRTIO_VIDEO_RESULT_ERR_INVALID_RESOURCE_ID]<br>
> + if the mentioned resource ID does not exist,<br>
> + \item[VIRTIO_VIDEO_RESULT_ERR_INVALID_ARGUMENT]<br>
> + if other command parameters are not valid, e.g. not within the<br>
> device's<br>
> + capabilities,<br>
> + \item[VIRTIO_VIDEO_RESULT_ERR_INVALID_OPERATION]<br>
> + if the command is performed at a time when it is not valid.<br>
> + \end{description}<br>
> +\item[\field{delayed_response_id}]<br>
> + is an ID of the future delayed response provided by the device,<br>
> that allows<br>
> + to relate it to the command.<br>
> +\end{description}<br>
> +<br>
> +\devicenormative{\paragraph}{Device Operation: Command<br>
> Virtqueue}{Device Types / Video Device / Device Operation / Device<br>
> Operation: Command Virtqueue}<br>
> +<br>
> +Responses to a command MUST be written by the device in the first<br>
> +device-writable descriptor of the descriptor chain from which the<br>
> +command came.<br>
> +<br>
> +The device MUST return VIRTIO_VIDEO_RESULT_ERR_INVALID_COMMAND to<br>
> +any command code it does not recognize.<br>
> +<br>
> +\field{delayed_response_id} MUST be set to a stream-unique<br>
> identifier that<br>
> +remains valid as long as the background operation hasn't finished.<br>
> <br>
> +<br>
> +\drivernormative{\paragraph}{Device Operation: Command<br>
> Virtqueue}{Device Types / Video Device / Device Operation / Device<br>
> Operation: Command Virtqueue}<br>
> +<br>
> +Descriptor chains sent to the commandq by the driver MUST include at<br>
> +least one device-writable descriptor of a size sufficient to<br>
> receive the<br>
> +response to the queued command.<br>
> +<br>
> +The driver MUST NOT interpret the rest of a response whose result<br>
> is not<br>
> +VIRTIO_VIDEO_RESULT_OK or VIRTIO_VIDEO_RESULT_OK_DELAYED.<br>
> +<br>
> +\subsubsection{Device Operation: Event Virtqueue}<br>
> +\label{sec:Device Types / Video Device / Device Operation / Device<br>
> Operation: Event Virtqueue}<br>
> +<br>
> +The eventq is used by the device to send delayed responses to<br>
> commands queued<br>
> +by the driver on the commandq and standalone events. Stream errors<br>
> and dynamic<br>
> +parameters changes are caused by changes in the device's state, not by<br>
> +commands, still they are delivered as<br>
> VIRTIO_VIDEO_DELAYED_RESP_STREAM_DESTROY<br>
> +and VIRTIO_VIDEO_DELAYED_RESP_STREAM_SET_PARAMS, respectively.<br>
> <br>
> <br>
> So, ERROR and DRC events trigger a VIRTIO_VIDEO_DELAYED_RESP_STREAM_DESTROY<br>
> and VIRTIO_VIDEO_DELAYED_RESP_STREAM_SET_PARAMS events, respectively.<br>
> And VIRTIO_VIDEO_DELAYED_RESP_STREAM_SET_PARAMS will have<br>
> a valid `delayed_response_id` only if it comes from a SET_PARAMS command <br>
> from the<br>
> driver. Otherwise, if it is due to a dynamic parameter change, <br>
> `delayed_response_id`<br>
> will be set to `VIRTIO_VIDEO_INVALID_RESPONSE_ID`. Is this correct?<br>
<br>
Yes, this is correct. Looks like it is better to rename <br>
VIRTIO_VIDEO_INVALID_RESPONSE_ID to VIRTIO_VIDEO_STANDALONE_EVENT_ID <br>
maybe. Thanks!<br>
<br>
> +<br>
> +The supported events are defined as follows:<br>
> +<br>
> +\begin{lstlisting}<br>
> +#define VIRTIO_VIDEO_DELAYED_RESP_STREAM_DESTROY 1<br>
> +#define VIRTIO_VIDEO_DELAYED_RESP_STREAM_SET_PARAMS 2<br>
> +#define VIRTIO_VIDEO_DELAYED_RESP_STREAM_DRAIN 3<br>
> +#define VIRTIO_VIDEO_DELAYED_RESP_STREAM_RESET 4<br>
> +#define VIRTIO_VIDEO_DELAYED_RESP_RESOURCE_QUEUE 5<br>
> +<br>
> +#define VIRTIO_VIDEO_EVENT_FLAG_CANCELED (1 << 0)<br>
> +<br>
> +struct virtio_video_event {<br>
> + le32 event_type; /* VIRTIO_VIDEO_DELAYED_RESP_* */<br>
> + le32 stream_id;<br>
> + le32 delayed_response_id;<br>
> + le32 event_flags; /* Bitmask of VIRTIO_VIDEO_EVENT_FLAG_* */<br>
> + union {<br>
> + struct virtio_video_stream_set_params_delayed_resp<br>
> set_params;<br>
> + struct virtio_video_resource_queue_delayed_resp queue;<br>
> + };<br>
> +};<br>
> +\end{lstlisting}<br>
> +<br>
> +\begin{description}<br>
> +\item[\field{event_type}]<br>
> + is the type of the event.<br>
> +\item[\field{stream_id}]<br>
> + is the ID of a valid stream.<br>
> +\item[\field{delayed_response_id}]<br>
> + is an ID of the delayed response, that allows to relate it to a<br>
> previously<br>
> + submitted command. If it is set to<br>
> VIRTIO_VIDEO_INVALID_RESPONSE_ID, then<br>
> + this is a standalone event, see<br>
> + \ref{sec:Device Types / Video Device / Device Operation / Device<br>
> Operation: Standalone Events}.<br>
> +\item[\field{event_flags}]<br>
> + is a bitmask of VIRTIO_VIDEO_EVENT_FLAG_* flags<br>
> +<br>
> + \begin{description}<br>
> + \item[VIRTIO_VIDEO_EVENT_FLAG_CANCELED]<br>
> + is set if the command has been canceled by another command,<br>
> that has<br>
> + higher priority. Doesn't make sense for standalone events.<br>
> + \end{description}<br>
> +\end{description}<br>
> +<br>
> +The particular member of the union is selected according to the<br>
> +\field{event_type} for some of the types.<br>
> +<br>
> +\drivernormative{\paragraph}{Device Operation: Event<br>
> Virtqueue}{Device Types / Video Device / Device Operation / Device<br>
> Operation: Event Virtqueue}<br>
> +<br>
> +The driver MUST at any time have at least one descriptor with a used<br>
> +buffer large enough to contain a \field{struct virtio_video_event}<br>
> +queued on the eventq.<br>
> +<br>
> +The driver MUST NOT put device-readable descriptors into the eventq.<br>
> +<br>
> +\subsubsection{Device Operation: Device Commands}<br>
> +\label{sec:Device Types / Video Device / Device Operation / Device<br>
> Operation: Device Commands}<br>
> +<br>
> +This command allows retrieving the device capabilities.<br>
> +<br>
> +\paragraph{VIRTIO_VIDEO_CMD_DEVICE_QUERY_CAPS}<br>
> +\label{sec:Device Types / Video Device / Device Operation / Device<br>
> Operation: Device Commands / VIRTIO_VIDEO_CMD_DEVICE_QUERY_CAPS}<br>
> +<br>
> +Retrieve device capabilities for all available stream parameters.<br>
> +<br>
> +The driver sends this command with<br>
> +\field{struct virtio_video_device_query_caps}:<br>
> +<br>
> +\begin{lstlisting}<br>
> +struct virtio_video_device_query_caps {<br>
> + le32 cmd_type; /* VIRTIO_VIDEO_CMD_DEVICE_QUERY_CAPS */<br>
> +};<br>
> +\end{lstlisting}<br>
> +<br>
> +The device responds with<br>
> +\field{struct virtio_video_device_query_caps_resp}:<br>
> +<br>
> +\begin{lstlisting}<br>
> +#define MASK(x) (1 << (x))<br>
> +<br>
> +struct virtio_video_device_query_caps_resp {<br>
> + le32 result; /* VIRTIO_VIDEO_RESULT_* */<br>
> + le32 stream_params_bitmask; /* Bitmask of<br>
> MASK(VIRTIO_VIDEO_PARAM_*) */<br>
> + le32 coded_formats_bitmask; /* Bitmaks of<br>
> MASK(VIRTIO_VIDEO_CODED_FORMAT_*) */<br>
> + le32 raw_formats_bitmask; /* Bitmask of<br>
> MASK(VIRTIO_VIDEO_RAW_FORMAT_*) */<br>
> + le32 num_format_deps;<br>
> + le32 num_raw_format_caps;<br>
> + le32 num_coded_resources_caps;<br>
> + le32 num_raw_resources_caps;<br>
> + le32 num_bitrate_caps; /* If<br>
> MASK(VIRTIO_VIDEO_PARAM_BITRATE) is set. */<br>
> + u8 padding[4];<br>
> + /* If corresponding MASK(VIRTIO_VIDEO_PARAM_GROUP_CODEC_*)<br>
> is set. */<br>
> + struct virtio_video_mpeg2_caps mpeg2_caps;<br>
> + struct virtio_video_mpeg4_caps mpeg4_caps;<br>
> + struct virtio_video_h264_caps h264_caps;<br>
> + struct virtio_video_hevc_caps hevc_caps;<br>
> + struct virtio_video_vp8_caps vp8_caps;<br>
> + struct virtio_video_vp9_caps vp9_caps;<br>
> + /**<br>
> + * Followed by<br>
> + * struct virtio_video_format_dependency<br>
> format_deps[num_format_deps];<br>
> + */<br>
> + /**<br>
> + * Followed by<br>
> + * struct virtio_video_raw_format_caps<br>
> raw_format_caps[num_raw_format_caps];<br>
> + */<br>
> + /**<br>
> + * Followed by<br>
> + * struct virtio_video_coded_resources_caps<br>
> + * coded_resources_caps[num_coded_resources_caps];<br>
> + */<br>
> + /**<br>
> + * Followed by<br>
> + * struct virtio_video_raw_resources_caps<br>
> raw_resources_caps[num_raw_resources_caps];<br>
> + */<br>
> + /**<br>
> + * Followed by if MASK(VIRTIO_VIDEO_PARAM_BITRATE) is set<br>
> + * struct virtio_video_bitrate_caps<br>
> bitrate_caps[num_bitrate_caps];<br>
> + */<br>
> +};<br>
> <br>
> <br>
> Maybe nitpicking, but some of the member structs are inside a comment <br>
> and some are not.<br>
> Does not seem to correlate with them being conditional.<br>
> I think is nice to have conditional fields in comment blocks to <br>
> highlight it, but then the<br>
> VIRTIO_VIDEO_PARAM_GROUP_CODEC_* structs need to be in their own comment <br>
> block.<br>
<br>
Yeah, this style comes from draft v5, then I added the conditional <br>
statementson top, so now it is harder to understand. I also would like <br>
to do this in a different way. I was thinking recently about <br>
extendability of this construct, it doesn't look good. If a new codec or <br>
a new codec-specific parameters is added, it has to be guarded by a new <br>
feature flag, say VIRTIO_VIDEO_F_CODECS_2024. Then the device will have <br>
to provide different structures depending on the negotiated flags and <br>
the driver will have to parse it. This looks quite painful and <br>
error-prone. My current idea is to replace this with something like FDT <br>
to make it much more flexible. The resulting blob with all the <br>
capabilities can even be mapped directly to the guest memory. I'm still <br>
exploring this idea. WDYT?<br>
<br></blockquote><div><br></div><div>Yes, the struct looks a bit cumbersome and difficult to expand, as you mention.</div><div>But I am not sure what do you mean by FDT, or how you plan to map it to the guest</div><div>memory. Could you expand the idea?</div><div>But from what I see, structures for most formats have similar fields for capabilities.</div><div>Couldn't this be unified into a single capabilities struct and fill it with the raw data obtained</div><div>from the host device?</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
> +\end{lstlisting}<br>
> +<br>
> +\begin{description}<br>
> +\item[\field{result}]<br>
> + is<br>
> +<br>
> + \begin{description}<br>
> + \item[VIRTIO_VIDEO_RESULT_OK]<br>
> + if the operation succeeded,<br>
> + \item[VIRTIO_VIDEO_RESULT_ERR_OUT_OF_MEMORY]<br>
> + if the descriptor was smaller than the defined<br>
> \field{caps_length} in<br>
> + the video device configuration.<br>
> + \end{description}<br>
> +\item[\field{stream_params_bitmask}]<br>
> + is a bitmask of supported stream parameters.<br>
> +\item[\field{coded_formats_bitmask}]<br>
> + is a bitmask of supported coded formats.<br>
> +\item[\field{raw_formats_bitmask}]<br>
> + is a bitmask of supported raw formats.<br>
> +\item[\field{num_format_deps}]<br>
> + is the number of elements in the format_deps array.<br>
> +\item[\field{num_raw_format_caps}]<br>
> + is the number of elements in the raw_format_caps array.<br>
> +\item[\field{num_coded_resources_caps}]<br>
> + is the number of elements in the coded_resources_caps array.<br>
> +\item[\field{num_raw_resources_caps}]<br>
> + is the number of elements in the raw_resources_caps array.<br>
> +\item[\field{num_bitrate_caps}]<br>
> + is the number of elements in the bitrate_caps array.<br>
> +\item[\field{mpeg2_caps}]<br>
> + groups the capabilities of MPEG2 specific parameters.<br>
> +\item[\field{mpeg4_caps}]<br>
> + groups the capabilities of MPEG4 specific parameters.<br>
> +\item[\field{h264_caps}]<br>
> + groups the capabilities of H.264 specific parameters.<br>
> +\item[\field{hevc_caps}]<br>
> + groups the capabilities of HEVC specific parameters.<br>
> +\item[\field{vp8_caps}]<br>
> + groups the capabilities of VP8 specific parameters.<br>
> +\item[\field{vp9_caps}]<br>
> + groups the capabilities of VP9 specific parameters.<br>
> +\item[\field{format_deps}]<br>
> + is an array of size \field{num_format_deps} establishing dependencies<br>
> + between coded and raw formats.<br>
> +\item[\field{raw_format_caps}]<br>
> + is an array of size \field{num_raw_format_caps} containing the<br>
> supported<br>
> + raw formats capabilities.<br>
> +\item[\field{coded_resources_caps}]<br>
> + is an array of size \field{num_coded_resources_caps}, that sets<br>
> bounds for<br>
> + the number of resources in the CODED queue.<br>
> +\item[\field{raw_resources_caps}]<br>
> + is an array of size \field{num_raw_resources_caps}, that sets<br>
> bounds for<br>
> + the number of resources in the RAW queue.<br>
> +\item[\field{bitrate_caps}]<br>
> + is an array of size \field{num_bitrate_caps} containing the supported<br>
> + bitrates.<br>
> +\end{description}<br>
> +<br>
> +% TODO: V4L2 flow: 1. coded format without variants (maybe these flags<br>
> +% are relevant too: V4L2_FMT_FLAG_CONTINUOUS_BYTESTREAM?,<br>
> +% V4L2_FMT_FLAG_DYN_RESOLUTION?,<br>
> V4L2_FMT_FLAG_ENC_CAP_FRAME_INTERVAL???),<br>
> +% also include variants (see VIDIOC_QUERYCTRL), then raw formats,<br>
> +% then resolutions (discrete or stepwise, see VIDIOC_ENUM_FRAMESIZES),<br>
> +% intervals are optional (see VIDIOC_ENUM_FRAMEINTERVALS)<br>
> +<br>
> +\devicenormative{\subparagraph}{VIRTIO_VIDEO_CMD_DEVICE_QUERY_CAPS}{Device Types / Video Device / Device Operation / Device Operation: Device Commands / VIRTIO_VIDEO_CMD_DEVICE_QUERY_CAPS}<br>
> +<br>
> +The device MUST support at least these parameters:<br>
> +VIRTIO_VIDEO_PARAM_CODED_FORMAT, VIRTIO_VIDEO_PARAM_RAW_FORMAT,<br>
> +VIRTIO_VIDEO_PARAM_CODED_RESOURCES, VIRTIO_VIDEO_PARAM_RAW_RESOURCES.<br>
> +<br>
> +The device MUST NOT mark codec-specific parameters<br>
> +(VIRTIO_VIDEO_PARAM_GROUP_CODEC_*) as supported unless the<br>
> corresponding<br>
> +codecs are supported as well.<br>
> +<br>
> +The device MUST set to zero all fields with capabilities of unsupported<br>
> +parameters.<br>
> +<br>
> +The lengths \field{num_format_deps}, \field{num_raw_format_caps},<br>
> +\field{num_coded_resources_caps} and \field{num_raw_resources_caps}<br>
> MUST be<br>
> +positive.<br>
> +<br>
> +The device MUST write the five \field{format_deps},<br>
> +\field{raw_format_caps}, \field{coded_resources_caps},<br>
> +\field{raw_resources_caps} and \field{bitrate_caps} arrays, of length<br>
> +\field{num_format_deps}, \field{num_raw_format_caps},<br>
> +\field{num_coded_resources_caps}, \field{num_raw_resources_caps} and<br>
> +\field{num_bitrate_caps}, respectively.<br>
> +<br>
> +For each coded format in the \field{coded_formats_bitmask} there<br>
> MUST be<br>
> +at least one element of \field{format_deps} referencing it.<br>
> +<br>
> +For each raw format in the \field{raw_formats_bitmask} there MUST be<br>
> +at least one element of \field{format_deps} referencing it.<br>
> +<br>
> +For any coded and any raw format there MUST be at most one element of<br>
> +\field{format_deps} referencing both of them.<br>
> +<br>
> +Elements of \field{format_deps} SHOULD be ordered according to raw<br>
> format<br>
> +preferences of the device from preferred to not preferred ones.<br>
> +<br>
> +For each raw format in the \field{raw_formats_bitmask} there MUST be<br>
> +exactly one element of \field{raw_format_caps} referencing it.<br>
> +<br>
> +For each coded format in the \field{coded_formats_bitmask} there<br>
> MUST be<br>
> +exactly one element of \field{coded_resources_caps} referencing it.<br>
> +<br>
> +For each raw format in the \field{raw_formats_bitmask} there MUST be<br>
> +exactly one element of \field{raw_resources_caps} referencing it.<br>
> +<br>
> +If VIRTIO_VIDEO_PARAM_BITRATE is supported, then for each coded<br>
> format in<br>
> +the \field{coded_formats_bitmask} there MUST be exactly one element of<br>
> +\field{bitrate_caps} referencing it.<br>
> +<br>
> +The total size of the response MUST be equal to \field{caps_length}<br>
> +bytes, as reported by the device configuration.<br>
> +<br>
> +\subparagraph{VIRTIO_VIDEO_CMD_DEVICE_QUERY_CAPS: Format dependencies}<br>
> +<br>
> +The description of dependencies between coded and raw formats<br>
> +\field{virtio_video_format_dependency} is defined as follows:<br>
> +<br>
> +\begin{lstlisting}<br>
> +struct virtio_video_format_dependency {<br>
> + le32 coded_formats_bitmask; /* Bitmask of<br>
> MASK(VIRTIO_VIDEO_CODED_FORMAT_*) */<br>
> + le32 raw_format; /* VIRTIO_VIDEO_RAW_FORMAT_* */<br>
> +};<br>
> +\end{lstlisting}<br>
> +<br>
> +\begin{description}<br>
> +\item[\field{coded_formats_bitmask}]<br>
> + specifies coded formats, see<br>
> + \ref{sec:Device Types / Video Device / Supported parameters /<br>
> Supported coded formats}.<br>
> + If a bit for a specific coded format is set, then this coded<br>
> format can be<br>
> + decoded into the specified raw format or encoded from it.<br>
> +\item[\field{raw_format}]<br>
> + is a raw format, see<br>
> + \ref{sec:Device Types / Video Device / Supported parameters /<br>
> Supported raw formats}.<br>
> +\end{description}<br>
> +<br>
> +\devicenormative{\subparagraph}{VIRTIO_VIDEO_CMD_DEVICE_QUERY_CAPS:<br>
> Format dependencies}{Device Types / Video Device / Device Operation<br>
> / Device Operation: Device Commands /<br>
> VIRTIO_VIDEO_CMD_DEVICE_QUERY_CAPS /<br>
> VIRTIO_VIDEO_CMD_DEVICE_QUERY_CAPS: Format dependencies}<br>
> +<br>
> +\field{coded_formats_bitmask} MUST be a subset of<br>
> \field{coded_formats_bitmask}<br>
> +field of \field{struct virtio_video_device_query_caps_resp}.<br>
> +<br>
> +\field{coded_formats_bitmask} MUST specify at least one coded format.<br>
> +<br>
> +\field{raw_format} MUST be set to one of the supported raw formats<br>
> according to<br>
> +the \field{raw_formats_bitmask} field of<br>
> +\field{struct virtio_video_device_query_caps_resp}.<br>
> +<br>
> +\subparagraph{VIRTIO_VIDEO_CMD_DEVICE_QUERY_CAPS: Raw format<br>
> capabilities}<br>
> +\label{sec:Device Types / Video Device / Device Operation / Device<br>
> Operation: Device Commands / VIRTIO_VIDEO_CMD_DEVICE_QUERY_CAPS /<br>
> VIRTIO_VIDEO_CMD_DEVICE_QUERY_CAPS: Raw format capabilities}<br>
> +<br>
> +The raw format capability description<br>
> \field{virtio_video_raw_format_caps} is<br>
> +defined as follows:<br>
> +<br>
> +\begin{lstlisting}<br>
> +enum virtio_video_planes_layout {<br>
> + VIRTIO_VIDEO_PLANES_LAYOUT_SINGLE_BUFFER = 1,<br>
> + VIRTIO_VIDEO_PLANES_LAYOUT_MULTI_BUFFERS = 2,<br>
> +};<br>
> +<br>
> +struct virtio_video_range {<br>
> + le32 min;<br>
> + le32 max;<br>
> + le32 step;<br>
> + u8 padding[4];<br>
> +};<br>
> +<br>
> +struct virtio_video_raw_format_caps {<br>
> + le32 raw_formats_bitmask; /* Bitmask of<br>
> MASK(VIRTIO_VIDEO_RAW_FORMAT_*) */<br>
> + le32 planes_layouts; /* Bitmask of<br>
> VIRTIO_VIDEO_PLANES_LAYOUT_* */<br>
> + le32 plane_align;<br>
> + le32 stride_align_mask;<br>
> + struct virtio_video_range width_range;<br>
> + struct virtio_video_range height_range;<br>
> +};<br>
> +\end{lstlisting}<br>
> +<br>
> +\field{struct virtio_video_range} is used to represent a range of<br>
> values.<br>
> +An integer \(x\) is within the range \field{r} if<br>
> +\(\field{r.min} \le x \le \field{r.max}\) holds and \(x\) equals to<br>
> +\((\field{min} + \field{step} * n)\) for some integer \(n\).<br>
> +<br>
> +\begin{description}<br>
> +\item[\field{raw_formats_bitmask}]<br>
> + specifies raw formats, see<br>
> + \ref{sec:Device Types / Video Device / Supported parameters /<br>
> Supported raw formats},<br>
> + to which these capabilities apply.<br>
> +\item[\field{planes_layouts}]<br>
> + is a bitmask with the set of plane layout types from<br>
> + \field{enum virtio_video_planes_layout}.<br>
> +\item[\field{plane_align}]<br>
> + is the alignment of planes within a buffer in bytes. This field<br>
> is valid<br>
> + only if \field{planes_layouts} has the<br>
> + \field{VIRTIO_VIDEO_PLANES_LAYOUT_SINGLE_BUFFER} bit set.<br>
> +\item[\field{stride_align_mask}]<br>
> + is a mask of all supported power of two stride alignments.<br>
> +\item[\field{width_range}]<br>
> + is a range of widths in pixels.<br>
> +\item[\field{height_range}]<br>
> + is a range of heights in pixels.<br>
> +\end{description}<br>
> +<br>
> +\devicenormative{\subparagraph}{VIRTIO_VIDEO_CMD_DEVICE_QUERY_CAPS:<br>
> Raw format capabilities}{Device Types / Video Device / Device<br>
> Operation / Device Operation: Device Commands /<br>
> VIRTIO_VIDEO_CMD_DEVICE_QUERY_CAPS /<br>
> VIRTIO_VIDEO_CMD_DEVICE_QUERY_CAPS: Raw format capabilities}<br>
> +<br>
> +\field{raw_formats_bitmask} MUST be a subset of<br>
> \field{raw_formats_bitmask}<br>
> +field of \field{struct virtio_video_device_query_caps_resp}.<br>
> +<br>
> +\field{raw_formats_bitmask} MUST specify at least one raw format.<br>
> +<br>
> +The device MUST set<br>
> \field{VIRTIO_VIDEO_PLANES_LAYOUT_SINGLE_BUFFER} bit in<br>
> +\field{planes_layouts} if the plane layout with planes of a frame<br>
> laid out one<br>
> +after another in the same buffer is supported.<br>
> +<br>
> +The device MUST set<br>
> \field{VIRTIO_VIDEO_PLANES_LAYOUT_MULTI_BUFFERS} bit in<br>
> +\field{planes_layouts} if the plane layout with planes of a frame<br>
> laid out in<br>
> +separate buffers is supported.<br>
> +<br>
> +\field{plane_align} MUST be set to a power of two according to the<br>
> device<br>
> +plane alignment requirements if \field{planes_layouts} has the<br>
> +\field{VIRTIO_VIDEO_PLANES_LAYOUT_SINGLE_BUFFER} bit set or to zero<br>
> otherwise.<br>
> +<br>
> +\field{min}, \field{step} and \field{max} MUST be positive.<br>
> +<br>
> +\field{min} MUST be less then or equal to \field{max} within the<br>
> same range.<br>
> +<br>
> +\subparagraph{VIRTIO_VIDEO_CMD_DEVICE_QUERY_CAPS: Resource<br>
> capabilities}<br>
> +<br>
> +The CODED resource capabilities<br>
> \field{virtio_video_coded_resources_caps} is<br>
> +defined as follows:<br>
> +<br>
> +\begin{lstlisting}<br>
> +struct virtio_video_coded_resources_caps {<br>
> + le32 coded_formats_bitmask; /* Bitmask of<br>
> MASK(VIRTIO_VIDEO_CODED_FORMAT_*) */<br>
> + le32 min_resources;<br>
> + le32 max_resources;<br>
> + le32 buffer_size;<br>
> +};<br>
> +\end{lstlisting}<br>
> +<br>
> +\begin{description}<br>
> +\item[\field{coded_formats_bitmask}]<br>
> + specifies coded formats, see<br>
> + \ref{sec:Device Types / Video Device / Supported parameters /<br>
> Supported coded formats},<br>
> + to which these capabilities apply.<br>
> +\item[\field{min_resources}]<br>
> + is the minimum number of resources that the CODED queue supports<br>
> for all<br>
> + the specified coded formats.<br>
> +\item[\field{max_resources}]<br>
> + is the maximum number of resources that the CODED queue supports<br>
> for all<br>
> + the specified coded formats.<br>
> +\item[\field{buffer_size}]<br>
> + is the minimum size of the buffers that will back resources to be<br>
> queued.<br>
> +\end{description}<br>
> +<br>
> +The RAW resource capabilities<br>
> \field{virtio_video_raw_resources_caps} is<br>
> +defined as follows:<br>
> +<br>
> +\begin{lstlisting}<br>
> +struct virtio_video_raw_resources_caps {<br>
> + le32 raw_formats_bitmask; /* Bitmask of<br>
> MASK(VIRTIO_VIDEO_RAW_FORMAT_*) */<br>
> + le32 min_resources;<br>
> + le32 max_resources;<br>
> + u8 padding[4];<br>
> +};<br>
> +\end{lstlisting}<br>
> +<br>
> +\begin{description}<br>
> +\item[\field{raw_formats_bitmask}]<br>
> + specifies raw formats, see<br>
> + \ref{sec:Device Types / Video Device / Supported parameters /<br>
> Supported raw formats},<br>
> + to which these capabilities apply.<br>
> +\item[\field{min_resources}]<br>
> + is the minimum number of resources that the RAW queue supports<br>
> for all<br>
> + the specified raw formats.<br>
> +\item[\field{max_resources}]<br>
> + is the maximum number of resources that the RAW queue supports<br>
> for all<br>
> + the specified raw formats.<br>
> +\end{description}<br>
> +<br>
> +\devicenormative{\subparagraph}{VIRTIO_VIDEO_CMD_DEVICE_QUERY_CAPS:<br>
> Resource capabilities}{Device Types / Video Device / Device<br>
> Operation / Device Operation: Device Commands /<br>
> VIRTIO_VIDEO_CMD_DEVICE_QUERY_CAPS /<br>
> VIRTIO_VIDEO_CMD_DEVICE_QUERY_CAPS: Resource capabilities}<br>
> +<br>
> +\field{coded_formats_bitmask} MUST be a subset of<br>
> \field{coded_formats_bitmask}<br>
> +field of \field{struct virtio_video_device_query_caps_resp}.<br>
> +<br>
> +\field{coded_formats_bitmask} MUST specify at least one coded format.<br>
> +<br>
> +\field{raw_formats_bitmask} MUST be a subset of<br>
> \field{raw_formats_bitmask}<br>
> +field of \field{struct virtio_video_device_query_caps_resp}.<br>
> +<br>
> +\field{raw_formats_bitmask} MUST specify at least one raw format.<br>
> +<br>
> +\field{min_resources} MUST NOT be negative.<br>
> +<br>
> +\field{max_resources} MUST be greater then or equal to<br>
> \field{min_resources}<br>
> +within the same struct instance.<br>
> +<br>
> +\subparagraph{VIRTIO_VIDEO_CMD_DEVICE_QUERY_CAPS: Bitrates}<br>
> +<br>
> +The bitrate capabilities \field{virtio_video_bitrate_caps} is<br>
> +defined as follows:<br>
> +<br>
> +\begin{lstlisting}<br>
> +struct virtio_video_bitrate_caps {<br>
> + le32 coded_formats_bitmask; /* Bitmask of<br>
> MASK(VIRTIO_VIDEO_CODED_FORMAT_*) */<br>
> + le32 min_bitrate;<br>
> + le32 max_bitrate;<br>
> + u8 padding[4];<br>
> +};<br>
> +\end{lstlisting}<br>
> +<br>
> +\begin{description}<br>
> +\item[\field{coded_formats_bitmask}]<br>
> + specifies coded formats, see<br>
> + \ref{sec:Device Types / Video Device / Supported parameters /<br>
> Supported coded formats},<br>
> + to which these capabilities apply.<br>
> +\item[\field{min_bitrate}]<br>
> + is the minimum bitrate in bits per second supported by the<br>
> encoder for all the specified coded<br>
> + formats.<br>
> +\item[\field{max_bitrate}]<br>
> + is the maximum bitrate in bits per second supported by the<br>
> encoder for all the specified coded<br>
> + formats.<br>
> +\end{description}<br>
> +<br>
> +\devicenormative{\subparagraph}{VIRTIO_VIDEO_CMD_DEVICE_QUERY_CAPS:<br>
> Bitrates}{Device Types / Video Device / Device Operation / Device<br>
> Operation: Device Commands / VIRTIO_VIDEO_CMD_DEVICE_QUERY_CAPS /<br>
> VIRTIO_VIDEO_CMD_DEVICE_QUERY_CAPS: Bitrates}<br>
> +<br>
> +\field{coded_formats_bitmask} MUST be a subset of<br>
> \field{coded_formats_bitmask}<br>
> +field of \field{struct virtio_video_device_query_caps_resp}.<br>
> +<br>
> +\field{coded_formats_bitmask} MUST specify at least one coded format.<br>
> +<br>
> +\field{min_bitrate} MUST NOT be negative.<br>
> +<br>
> +\field{max_bitrate} MUST be greater then or equal to<br>
> \field{min_bitrate}<br>
> +within the same \field{struct virtio_video_bitrate_caps} instance.<br>
> +<br>
> +\subsubsection{Device Operation: Stream commands}<br>
> +\label{sec:Device Types / Video Device / Device Operation / Device<br>
> Operation: Stream commands}<br>
> +<br>
> +Stream commands allow the creation, destruction, and flow control of a<br>
> +stream.<br>
> +<br>
> +\paragraph{VIRTIO_VIDEO_CMD_STREAM_CREATE}<br>
> +\label{sec:Device Types / Video Device / Device Operation / Device<br>
> Operation: Stream commands / VIRTIO_VIDEO_CMD_STREAM_CREATE}<br>
> +<br>
> +Create a new stream using the device.<br>
> +<br>
> +The driver sends this command with<br>
> +\field{struct virtio_video_stream_create}:<br>
> +<br>
> +\begin{lstlisting}<br>
> +struct virtio_video_stream_create {<br>
> + le32 cmd_type; /* VIRTIO_VIDEO_CMD_STREAM_CREATE */<br>
> +};<br>
> +\end{lstlisting}<br>
> +<br>
> +The device responds with \field{struct<br>
> virtio_video_stream_create_resp}:<br>
> +<br>
> +\begin{lstlisting}<br>
> +struct virtio_video_stream_create_resp {<br>
> + le32 result; /* VIRTIO_VIDEO_RESULT_* */<br>
> + le32 stream_id;<br>
> +};<br>
> +\end{lstlisting}<br>
> +<br>
> +\begin{description}<br>
> +\item[\field{result}]<br>
> + is<br>
> +<br>
> + \begin{description}<br>
> + \item[VIRTIO_VIDEO_RESULT_OK]<br>
> + if the operation succeeded,<br>
> + \item[VIRTIO_VIDEO_RESULT_ERR_OUT_OF_MEMORY]<br>
> + if the limit of simultaneous streams has been reached by the<br>
> device and<br>
> + no more can be created.<br>
> + \item[VIRTIO_VIDEO_RESULT_ERR_INVALID_OPERATION]<br>
> + if the stream cannot be created due to an unexpected device issue.<br>
> + \end{description}<br>
> +\item[\field{stream_id}]<br>
> + is the ID of the created stream allocated by the device.<br>
> +\end{description}<br>
> +<br>
> +\devicenormative{\subparagraph}{VIRTIO_VIDEO_CMD_STREAM_CREATE}{Device Types / Video Device / Device Operation / Device Operation: Stream commands / VIRTIO_VIDEO_CMD_STREAM_CREATE}<br>
> +<br>
> +\field{stream_id} MUST be set to a device-unique identifier that<br>
> remains<br>
> +valid as long as the stream is alive.<br>
> +<br>
> +\paragraph{VIRTIO_VIDEO_CMD_STREAM_DESTROY}<br>
> +\label{sec:Device Types / Video Device / Device Operation / Device<br>
> Operation: Stream commands / VIRTIO_VIDEO_CMD_STREAM_DESTROY}<br>
> +<br>
> +% DESTROY has to be more like RESET, not DRAIN, because it is<br>
> called, for<br>
> +% example, when the guest user-space app closes a file descriptor.<br>
> So there<br>
> +% is no sense in continuing the processing.<br>
> +<br>
> +Destroy a video stream and all its resources. Any activity on the<br>
> stream<br>
> +is halted and all resources are released by the time the delayed<br>
> response is<br>
> +received by the driver.<br>
> +<br>
> +The driver sends this command with<br>
> +\field{struct virtio_video_stream_destroy}:<br>
> +<br>
> +\begin{lstlisting}<br>
> +struct virtio_video_stream_destroy {<br>
> + le32 cmd_type; /* VIRTIO_VIDEO_CMD_STREAM_DESTROY */<br>
> + le32 stream_id;<br>
> +};<br>
> +\end{lstlisting}<br>
> +<br>
> +\begin{description}<br>
> +\item[\field{stream_id}]<br>
> + is the ID of the stream to be destroyed, as previously returned by<br>
> + VIRTIO_VIDEO_CMD_STREAM_CREATE.<br>
> +\end{description}<br>
> +<br>
> +The device responds as described in<br>
> +\ref{sec:Device Types / Video Device / Device Operation / Device<br>
> Operation: Command Virtqueue}<br>
> +and begins the background DESTROY operation.<br>
> +<br>
> +When the command is completed the device sends the<br>
> +VIRTIO_VIDEO_DELAYED_RESP_STREAM_DESTROY delayed response, see<br>
> +\ref{sec:Device Types / Video Device / Device Operation / Device<br>
> Operation: Event Virtqueue}.<br>
> +The delayed response can also come in case of unrecoverable stream<br>
> error, see<br>
> +\ref{sec:Device Types / Video Device / Device Operation / Device<br>
> Operation: Standalone Events / Error Event}.<br>
> +<br>
> +\devicenormative{\subparagraph}{VIRTIO_VIDEO_CMD_STREAM_DESTROY}{Device Types / Video Device / Device Operation / Device Operation: Stream commands / VIRTIO_VIDEO_CMD_STREAM_DESTROY}<br>
> +<br>
> +Before the device sends a delayed response to<br>
> VIRTIO_VIDEO_CMD_STREAM_DESTROY,<br>
> +it MUST send all other pending delayed responses with<br>
> +VIRTIO_VIDEO_EVENT_FLAG_CANCELED flag set and detach all resources.<br>
> +<br>
> +After VIRTIO_VIDEO_CMD_STREAM_DESTROY is queued, the device MUST<br>
> reply with<br>
> +VIRTIO_VIDEO_RESULT_ERR_INVALID_STREAM_ID to any subsequently<br>
> queued command<br>
> +with this stream ID.<br>
> +<br>
> +The DESTROY operation MUST NOT be canceled.<br>
> +<br>
> +\drivernormative{\subparagraph}{VIRTIO_VIDEO_CMD_STREAM_DESTROY}{Device Types / Video Device / Device Operation / Device Operation: Stream commands / VIRTIO_VIDEO_CMD_STREAM_DESTROY}<br>
> +<br>
> +\field{stream_id} MUST be set to a valid stream ID previously returned<br>
> +by VIRTIO_VIDEO_CMD_STREAM_CREATE.<br>
> +<br>
> +The driver MUST stop using \field{stream_id} as a valid stream after it<br>
> +received the delayed response to this command.<br>
> +<br>
> +\paragraph{VIRTIO_VIDEO_CMD_STREAM_SET_PARAMS}<br>
> +\label{sec:Device Types / Video Device / Device Operation / Device<br>
> Operation: Stream commands / VIRTIO_VIDEO_CMD_STREAM_SET_PARAMS}<br>
> +<br>
> +Write values of selected parameters of a given stream, and receive<br>
> back the<br>
> +values for all the parameters supported by the device as reported by<br>
> +\ref{sec:Device Types / Video Device / Device Operation / Device<br>
> Operation: Device Commands / VIRTIO_VIDEO_CMD_DEVICE_QUERY_CAPS}.<br>
> +The operation can be either executed immediately, or queued into<br>
> the INPUT<br>
> +queue, i.e. after processing all the INPUT queue elements that are<br>
> queued<br>
> +before the command.<br>
> +<br>
> +The driver sends this command with<br>
> +\field{struct virtio_video_stream_set_params}:<br>
> +<br>
> +\begin{lstlisting}<br>
> +#define VIRTIO_VIDEO_SET_PARAMS_FLAG_IN_BAND (1 << 0)<br>
> +<br>
> +struct virtio_video_stream_set_params {<br>
> + le32 cmd_type; /* VIRTIO_VIDEO_CMD_STREAM_SET_PARAMS */<br>
> + le32 stream_id;<br>
> + le32 flags; /* Bitmask of VIRTIO_VIDEO_SET_PARAMS_FLAG_* */<br>
> + u8 padding[4];<br>
> + struct virtio_video_params params;<br>
> +};<br>
> +\end{lstlisting}<br>
> +<br>
> +\begin{description}<br>
> +\item[\field{stream_id}]<br>
> + is the ID of the stream we want to set a parameter for.<br>
> +\item[\field{flags}]<br>
> + is a bitmask of VIRTIO_VIDEO_SET_PARAMS_FLAG_* values.<br>
> +<br>
> + \begin{description}<br>
> + \item[\field{VIRTIO_VIDEO_SET_PARAMS_FLAG_IN_BAND}]<br>
> + The submitted parameters are to be set only after all of the<br>
> previously<br>
> + queued INPUT queue elements are processed. Without this flag the<br>
> + parameters are set Immediately.<br>
> + \end{description}<br>
> +\item[\field{params}]<br>
> + is a container for the selected stream parameters to be set.<br>
> +\end{description}<br>
> +<br>
> +The device responds as described in<br>
> +\ref{sec:Device Types / Video Device / Device Operation / Device<br>
> Operation: Command Virtqueue}<br>
> +and begins the background SET_PARAMS operation.<br>
> +<br>
> +When the background processing of the resource is completed the<br>
> device sends<br>
> +the VIRTIO_VIDEO_DELAYED_RESP_STREAM_SET_PARAMS delayed response, see<br>
> +\ref{sec:Device Types / Video Device / Device Operation / Device<br>
> Operation: Event Virtqueue}.<br>
> +The delayed response can also come in case of dynamic parameters<br>
> change, see<br>
> +\ref{sec:Device Types / Video Device / Device Operation / Device<br>
> Operation: Standalone Events / Dynamic Parameters Change Event}.<br>
> +<br>
> +The command-specific delayed response<br>
> +\field{struct virtio_video_stream_set_params_delayed_resp} is defined<br>
> +as follows:<br>
> +<br>
> +\begin{lstlisting}<br>
> +struct virtio_video_stream_set_params_delayed_resp {<br>
> + struct virtio_video_params params;<br>
> +};<br>
> +\end{lstlisting}<br>
> +<br>
> +\begin{description}<br>
> +\item[\field{params}]<br>
> + is a container for the actual values of all the parameters<br>
> supported by the<br>
> + device. The values set by the device may differ from the<br>
> requested values<br>
> + depending on the device's capabilities.<br>
> +\end{description}<br>
> +<br>
> +The \field{struct virtio_video_params} is defined as follows:<br>
> +<br>
> +\begin{lstlisting}<br>
> +struct virtio_video_raw_format {<br>
> + le32 format;<br>
> + le32 planes_layout; /* VIRTIO_VIDEO_PLANES_LAYOUT_* */<br>
> + le32 stride;<br>
> + le32 width;<br>
> + le32 height;<br>
> + u8 padding[4];<br>
> +};<br>
> +<br>
> +struct virtio_video_param_crop {<br>
> + le32 left;<br>
> + le32 top;<br>
> + le32 width;<br>
> + le32 height;<br>
> +};<br>
> +<br>
> +union virtio_video_codec_params {<br>
> + struct virtio_video_mpeg2_params mpeg2;<br>
> + struct virtio_video_mpeg4_params mpeg4;<br>
> + struct virtio_video_h264_params h264;<br>
> + struct virtio_video_hevc_params hevc;<br>
> + struct virtio_video_vp8_params vp8;<br>
> + struct virtio_video_vp9_params vp9;<br>
> +};<br>
> +<br>
> +struct virtio_video_params {<br>
> + le32 stream_params_bitmask; /* Bitmask of<br>
> MASK(VIRTIO_VIDEO_PARAM_*) */<br>
> + le32 coded_format; /* If<br>
> MASK(VIRTIO_VIDEO_PARAM_CODED_FORMAT) is set. */<br>
> + /* If MASK(VIRTIO_VIDEO_PARAM_RAW_FORMAT) is set. */<br>
> + struct virtio_video_raw_format raw_format;<br>
> + /* If MASK(VIRTIO_VIDEO_PARAM_CODED_RESOURCES) is set. */<br>
> + struct virtio_video_param_resources coded_resources;<br>
> + /* If MASK(VIRTIO_VIDEO_PARAM_RAW_RESOURCES) is set. */<br>
> + struct virtio_video_param_resources raw_resources;<br>
> + struct virtio_video_param_crop crop; /* If<br>
> MASK(VIRTIO_VIDEO_PARAM_CROP) is set. */<br>
> + le32 bitrate; /* If MASK(VIRTIO_VIDEO_PARAM_BITRATE) is set. */<br>
> + u8 padding[4];<br>
> + /* If the corresponding<br>
> MASK(VIRTIO_VIDEO_PARAM_GROUP_CODEC_*) is set<br>
> + * depending on the coded_format. */<br>
> + union virtio_video_codec_params codec;<br>
> +};<br>
> +\end{lstlisting}<br>
> <br>
> <br>
> This is a bit difficult to read for me, as some of the *if* comments are <br>
> written AFTER<br>
> the member definition, and some others BEFORE the member defintion.<br>
> Above, when introducing the VIRTIO_VIDEO_CMD_DEVICE_QUERY_CAPS command,<br>
> you use this format for the conditional fields:<br>
> /**<br>
> * Followed by if MASK(VIRTIO_VIDEO_PARAM_BITRATE) is set<br>
> * struct virtio_video_bitrate_caps bitrate_caps[num_bitrate_caps];<br>
> */<br>
> It spaces a bit more the members, and leaves them as part of the<br>
> comment block. I would suggest to keep the format consistent.<br>
<br>
Ack, I'll try to make it more readable.<br>
Also given the extendability concerns, that I explained above, I'd like <br>
to maybe finish combining all the parameters into groups and then to <br>
make the SET_PARAMS command operating on the parameter group level. <br>
Still thinking about this...<br>
<br>
-- <br>
Alexander Gordeev<br>
Senior Software Engineer<br>
<br>
OpenSynergy GmbH<br>
Rotherstr. 20, 10245 Berlin<br>
<br>
Phone: +49 30 60 98 54 0 - 88<br>
Fax: +49 (30) 60 98 54 0 - 99<br>
EMail: <a href="mailto:alexander.gordeev@opensynergy.com" target="_blank">alexander.gordeev@opensynergy.com</a><br>
<br>
<a href="http://www.opensynergy.com" rel="noreferrer" target="_blank">www.opensynergy.com</a><br>
<br>
Handelsregister/Commercial Registry: Amtsgericht Charlottenburg, HRB 108616B<br>
Geschäftsführer/Managing Director: Régis Adjamah<br>
<br>
</blockquote></div></div>