Re: [virtio-dev] [RFC RESEND] virtio-video: Add virtio video device specification
Hi Dmitry, On Fri, Dec 20, 2019 at 11:25 PM Dmitry Sepp wrote: > > Hi Keiichi, > > Could you please explain in a more detailed way what exactly CROP is used for? > Is it equivalent to G/S_CROP or G/S_SELECTION in v4l2? Do you only need to get > it, or to set as well? Can it also be useful for the encoder? It corresponds to struct v4l2_rect in V4L2, which is used in G/S_SELECTION. IIUC, G/S_SELECTION is a kind of super set of G/S_CROP. Both decoder and encoder can utilize both G_SELECTION and S_SELECTION. Their usages are documented in the specs of V4L2 stateful decoder/encoder API. stateful decoder spec: https://linuxtv.org/downloads/v4l-dvb-apis/uapi/v4l/dev-decoder.html stateful encoder spec (in maintainer's tree): https://hverkuil.home.xs4all.nl/codec-api/uapi/v4l/dev-encoder.html Best regards, Keiichi > > Regrds, > Dmitry. > > On Freitag, 13. Dezember 2019 17:31:24 CET Keiichi Watanabe wrote: > > On Fri, Dec 13, 2019 at 11:20 PM Keiichi Watanabe > wrote: > > > On Thu, Dec 12, 2019 at 7:34 PM Dmitry Sepp > wrote: > > > > Hi Keiichi, > > > > > > > > Thank you for your comment. > > > > What do you thing about selection/crop rectangles? Should this be > > > > defined as a must have? Or we just assume the device always sticks to > > > > the stream resolution. > > > > > > For decoding, the device needs to tell the driver crop rectangle when > > > a resolution change happens, as virtio-vdec has > > > virtio_vdec_host_req_stream_crop. > > > So, we should add a new field in virtio_video_param for it. > > > > > > For what it's worth, we have C-headers that exposes Chrome's video > > > codec functionalities. So, our virtio-video device will do > > > decoding/encoding by calling these functions. > > > Decoder: > > > https://chromium.git.corp.google.com/chromiumos/platform2/+/00ad71fa640c0 > > > 18c1d014f467c50b9a7dc962a10/arc/vm/libvda/libvda_decode.h Encoder: > > > https://chromium.git.corp.google.com/chromiumos/platform2/+/00ad71fa640c0 > > > 18c1d014f467c50b9a7dc962a10/arc/vm/libvda/libvda_encode.h > > Oops, the links were not accesible... > > The correct links are: > > Decoder: > > https://chromium.googlesource.com/chromiumos/platform2/+/00ad71fa640c018c1d > > 014f467c50b9a7dc962a10/arc/vm/libvda/libvda_decode.h Encoder: > > https://chromium.googlesource.com/chromiumos/platform2/+/00ad71fa640c018c1d > > 014f467c50b9a7dc962a10/arc/vm/libvda/libvda_encode.h > > > > Best, > > Keiichi > > > > > BTW, I'm preparing the next version of virtio-video spec by addressing > > > points we discussed in this thread. I'll send it here next week, > > > hopefully. > > > > > > Best, > > > Keiichi > > > > > > > Regards, > > > > Dmitry. > > > > > > > > On Donnerstag, 12. Dezember 2019 06:39:11 CET Keiichi Watanabe wrote: > > > > > On Tue, Dec 10, 2019 at 10:16 PM Dmitry Sepp > > > > > > > > > > wrote: > > > > > > Hi, > > > > > > > > > > > > Just to start, let's consider this v4l2 control: > > > > > > V4L2_CID_MPEG_VIDEO_FRAME_RC_ENABLE. > > > > > > As I can see, this control is referenced as a mandatory one in the > > > > > > Chromium > > > > > > sources [1]. > > > > > > > > > > > > So could someone from the Chromium team please explain why it is > > > > > > mandatory? > > > > > > (YouTube?) In fact, almost no encoders implement this control. Do we > > > > > > need > > > > > > it in virtio-video? > > > > > > > > > > That control is used to encode videos in constant bitrate (CBR) mode, > > > > > which is critical for > > > > > real-time use case like video conferencing. > > > > > > > > > > However, that Chromium source code just requires *host-side* drivers > > > > > to have the v4l2 > > > > > control. Also, I guess it's rare that a guest app wants to use CQP > > > > > instead of CBR from our > > > > > experience. > > > > > So, I suppose we can omit this control in virtio spec for now by > > > > > assuming that guests > > > > > always use CBR mode. > > > > > > > > > > Best regards, > > > > > Keiichi > > > > > > > > > > > [1] > > > > > > https://chromium.googlesource.com/chromium/src/media/+/refs/heads/ma > > > > > > ster/ > > > > > > gpu/v4l2/v4l2_video_encode_accelerator.cc#1500 > > > > > > > > > > > > Regards, > > > > > > Dmitry. > > > > > > > > > > > > On Montag, 9. Dezember 2019 22:12:28 CET Enrico Granata wrote: > > > > > > > +Changyeon Jo for his awareness > > > > > > > > > > > > > > Thanks, > > > > > > > - Enrico > > > > > > > > > > > > > > > > > > > > > On Mon, Dec 9, 2019 at 6:20 AM Dmitry Sepp > > > > > > > > > > > > > > > > > > > > > wrote: > > > > > > > > Hello, > > > > > > > > > > > > > > > > I'd like to invite everyone to share ideas regarding required > > > > > > > > encoder > > > > > > > > features > > > > > > > > in this separate sub-tree. > > > > > > > > > > > > > > > > In general, encoder devices are more complex compared to > > > > > > > > decoders. So > > > > > > > > the > > > > > > > > question I'd like to rise is in what way we define the minimal > > > > > > > > subset > > > > > > > > of > > > > >
Re: [virtio-dev] [RFC RESEND] virtio-video: Add virtio video device specification
Hi Keiichi, Could you please explain in a more detailed way what exactly CROP is used for? Is it equivalent to G/S_CROP or G/S_SELECTION in v4l2? Do you only need to get it, or to set as well? Can it also be useful for the encoder? Regrds, Dmitry. On Freitag, 13. Dezember 2019 17:31:24 CET Keiichi Watanabe wrote: > On Fri, Dec 13, 2019 at 11:20 PM Keiichi Watanabe wrote: > > On Thu, Dec 12, 2019 at 7:34 PM Dmitry Sepp wrote: > > > Hi Keiichi, > > > > > > Thank you for your comment. > > > What do you thing about selection/crop rectangles? Should this be > > > defined as a must have? Or we just assume the device always sticks to > > > the stream resolution. > > > > For decoding, the device needs to tell the driver crop rectangle when > > a resolution change happens, as virtio-vdec has > > virtio_vdec_host_req_stream_crop. > > So, we should add a new field in virtio_video_param for it. > > > > For what it's worth, we have C-headers that exposes Chrome's video > > codec functionalities. So, our virtio-video device will do > > decoding/encoding by calling these functions. > > Decoder: > > https://chromium.git.corp.google.com/chromiumos/platform2/+/00ad71fa640c0 > > 18c1d014f467c50b9a7dc962a10/arc/vm/libvda/libvda_decode.h Encoder: > > https://chromium.git.corp.google.com/chromiumos/platform2/+/00ad71fa640c0 > > 18c1d014f467c50b9a7dc962a10/arc/vm/libvda/libvda_encode.h > Oops, the links were not accesible... > The correct links are: > Decoder: > https://chromium.googlesource.com/chromiumos/platform2/+/00ad71fa640c018c1d > 014f467c50b9a7dc962a10/arc/vm/libvda/libvda_decode.h Encoder: > https://chromium.googlesource.com/chromiumos/platform2/+/00ad71fa640c018c1d > 014f467c50b9a7dc962a10/arc/vm/libvda/libvda_encode.h > > Best, > Keiichi > > > BTW, I'm preparing the next version of virtio-video spec by addressing > > points we discussed in this thread. I'll send it here next week, > > hopefully. > > > > Best, > > Keiichi > > > > > Regards, > > > Dmitry. > > > > > > On Donnerstag, 12. Dezember 2019 06:39:11 CET Keiichi Watanabe wrote: > > > > On Tue, Dec 10, 2019 at 10:16 PM Dmitry Sepp > > > > > > > > wrote: > > > > > Hi, > > > > > > > > > > Just to start, let's consider this v4l2 control: > > > > > V4L2_CID_MPEG_VIDEO_FRAME_RC_ENABLE. > > > > > As I can see, this control is referenced as a mandatory one in the > > > > > Chromium > > > > > sources [1]. > > > > > > > > > > So could someone from the Chromium team please explain why it is > > > > > mandatory? > > > > > (YouTube?) In fact, almost no encoders implement this control. Do we > > > > > need > > > > > it in virtio-video? > > > > > > > > That control is used to encode videos in constant bitrate (CBR) mode, > > > > which is critical for > > > > real-time use case like video conferencing. > > > > > > > > However, that Chromium source code just requires *host-side* drivers > > > > to have the v4l2 > > > > control. Also, I guess it's rare that a guest app wants to use CQP > > > > instead of CBR from our > > > > experience. > > > > So, I suppose we can omit this control in virtio spec for now by > > > > assuming that guests > > > > always use CBR mode. > > > > > > > > Best regards, > > > > Keiichi > > > > > > > > > [1] > > > > > https://chromium.googlesource.com/chromium/src/media/+/refs/heads/ma > > > > > ster/ > > > > > gpu/v4l2/v4l2_video_encode_accelerator.cc#1500 > > > > > > > > > > Regards, > > > > > Dmitry. > > > > > > > > > > On Montag, 9. Dezember 2019 22:12:28 CET Enrico Granata wrote: > > > > > > +Changyeon Jo for his awareness > > > > > > > > > > > > Thanks, > > > > > > - Enrico > > > > > > > > > > > > > > > > > > On Mon, Dec 9, 2019 at 6:20 AM Dmitry Sepp > > > > > > > > > > > > > > > > > > wrote: > > > > > > > Hello, > > > > > > > > > > > > > > I'd like to invite everyone to share ideas regarding required > > > > > > > encoder > > > > > > > features > > > > > > > in this separate sub-tree. > > > > > > > > > > > > > > In general, encoder devices are more complex compared to > > > > > > > decoders. So > > > > > > > the > > > > > > > question I'd like to rise is in what way we define the minimal > > > > > > > subset > > > > > > > of > > > > > > > features to be implemented by the virtio-video. > > > > > > > > > > > > > > We may look at the following to define the set of features: > > > > > > > 1. USB video, 2.3.6 Encoding Unit [1]. > > > > > > > 2. Android 10 Compatibility Definition [2]. > > > > > > > > > > > > > > Would be nice to hear about any specific requirements from the > > > > > > > Chromium > > > > > > > team as > > > > > > > well. > > > > > > > > > > > > > > [1] > > > > > > > https://www.usb.org/sites/default/files/USB_Video_Class_1_5.zip > > > > > > > [2] > > > > > > > https://source.android.com/compatibility/android-cdd#5_2_video_e > > > > > > > ncodin > > > > > > > g > > > > > > > > > > > > > > Thank you. > > > > > > > > > > > > > > Best regards, > > > > > > > Dmitry. > > > > > > > > > > > > > > On Mi
Re: [virtio-dev] [RFC RESEND] virtio-video: Add virtio video device specification
On Tue, Dec 17, 2019 at 05:13:59PM +0100, Dmitry Sepp wrote: > Hi, > > On Dienstag, 17. Dezember 2019 15:09:16 CET Keiichi Watanabe wrote: > > Hi, > > > > Thanks Tomasz and Gerd for the suggestions and information. > > > > On Tue, Dec 17, 2019 at 10:39 PM Gerd Hoffmann wrote: > > > Hi, > > > > > > > On the host side, the encode and decode APIs are different as well, so > > > > having separate implementation decoder and encoder, possibly just > > > > sharing some helper code, would make much more sense. > > > > > > When going down that route I'd suggest to use two device ids (even when > > > specifying both variants in one spec section and one header file due to > > > the overlaps) instead of feature flags. > > > > Sounds good. It makes sense to use different device IDs for different > > devices. > Does this mean one driver handles both? Or we have two separate drivers? That is the driver writers choice. You can have a single kernel module registering as driver for both IDs. Or you can have two kernel modules, each registering for one of the IDs, possibly with a third library module with helper code for common bits like buffer management. cheers, Gerd - To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
Re: [virtio-dev] [RFC RESEND] virtio-video: Add virtio video device specification
Hi, On Dienstag, 17. Dezember 2019 15:09:16 CET Keiichi Watanabe wrote: > Hi, > > Thanks Tomasz and Gerd for the suggestions and information. > > On Tue, Dec 17, 2019 at 10:39 PM Gerd Hoffmann wrote: > > Hi, > > > > > On the host side, the encode and decode APIs are different as well, so > > > having separate implementation decoder and encoder, possibly just > > > sharing some helper code, would make much more sense. > > > > When going down that route I'd suggest to use two device ids (even when > > specifying both variants in one spec section and one header file due to > > the overlaps) instead of feature flags. > > Sounds good. It makes sense to use different device IDs for different > devices. Does this mean one driver handles both? Or we have two separate drivers? > > > > I don't think using fourcc is a problem, and given that both drm and > > > > v4l2 use fourcc already this would be a good choice I think. > > > > > > Both DRM and V4L2 use two mutually incompatible sets of FourCCs, so > > > I'm not sure how it could be a good choice. At least unless we decide > > > to pick a specific set of FourCC. It doesn't help that Windows/DirectX > > > has its own set of FourCCs that's again slightly different than the > > > two mentioned before. > > > > Ouch, wasn't aware of that. That makes reusing fourcc codes much less > > useful. > > > > > > But the definition should be more specific than just "fourcc". Best > > > > would be to explicitly list and define each format supported by the > > > > spec. > > > > > > Why not be consistent with virtio-gpu and just define new formats as > > > we add support for them as sequential integers? > > > > Yes, lets do that. > > It makes sense. I seems to have overestimated FourCC. This is what was actually done in the driver code already (it is a bit ahead of the spec, but I guess no one has looked at it so far). Regards, Dmitry. > > Best, > Keiichi > > > cheers, > > > > Gerd - To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
Re: [virtio-dev] [RFC RESEND] virtio-video: Add virtio video device specification
Hi, Thanks Tomasz and Gerd for the suggestions and information. On Tue, Dec 17, 2019 at 10:39 PM Gerd Hoffmann wrote: > > Hi, > > > On the host side, the encode and decode APIs are different as well, so > > having separate implementation decoder and encoder, possibly just > > sharing some helper code, would make much more sense. > > When going down that route I'd suggest to use two device ids (even when > specifying both variants in one spec section and one header file due to > the overlaps) instead of feature flags. Sounds good. It makes sense to use different device IDs for different devices. > > > > I don't think using fourcc is a problem, and given that both drm and > > > v4l2 use fourcc already this would be a good choice I think. > > > > Both DRM and V4L2 use two mutually incompatible sets of FourCCs, so > > I'm not sure how it could be a good choice. At least unless we decide > > to pick a specific set of FourCC. It doesn't help that Windows/DirectX > > has its own set of FourCCs that's again slightly different than the > > two mentioned before. > > Ouch, wasn't aware of that. That makes reusing fourcc codes much less > useful. > > > > But the definition should be more specific than just "fourcc". Best > > > would be to explicitly list and define each format supported by the > > > spec. > > > > Why not be consistent with virtio-gpu and just define new formats as > > we add support for them as sequential integers? > > Yes, lets do that. > It makes sense. I seems to have overestimated FourCC. Best, Keiichi > cheers, > Gerd > - To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
Re: [virtio-dev] [RFC RESEND] virtio-video: Add virtio video device specification
Hi, > On the host side, the encode and decode APIs are different as well, so > having separate implementation decoder and encoder, possibly just > sharing some helper code, would make much more sense. When going down that route I'd suggest to use two device ids (even when specifying both variants in one spec section and one header file due to the overlaps) instead of feature flags. > > I don't think using fourcc is a problem, and given that both drm and > > v4l2 use fourcc already this would be a good choice I think. > > Both DRM and V4L2 use two mutually incompatible sets of FourCCs, so > I'm not sure how it could be a good choice. At least unless we decide > to pick a specific set of FourCC. It doesn't help that Windows/DirectX > has its own set of FourCCs that's again slightly different than the > two mentioned before. Ouch, wasn't aware of that. That makes reusing fourcc codes much less useful. > > But the definition should be more specific than just "fourcc". Best > > would be to explicitly list and define each format supported by the > > spec. > > Why not be consistent with virtio-gpu and just define new formats as > we add support for them as sequential integers? Yes, lets do that. cheers, Gerd - To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
Re: [virtio-dev] [RFC RESEND] virtio-video: Add virtio video device specification
On Mon, Dec 16, 2019 at 7:32 PM Gerd Hoffmann wrote: > > Hi, > > > > Hmm, modern GPUs support both encoding and decoding ... > > > > Many SoC architectures have completely separate IP blocks for encoding > > and decoding. Similarly, in GPUs those are usually completely separate > > parts of the pipeline. > > In the OS there is one driver per GPU handling both ... > That's usually only the case for the desktop PC-style GPUs. That said, it probably doesn't matter from the protocol point of view how it's handled in the host. At least it is definitely safe to assume that the host provides functionality for independently decoding and encoding things from different processes, which is enough to implement virtio decoder and encoder as separate devices. > > > I don't think we should bake that restriction into the specification. > > > It probably makes sense to use one virtqueue per function though, that > > > will simplify dispatching in both host and guest. > > > > > > > Wouldn't it make the handling easier if we had one virtio device per > > function? > > I don't think there is much of a difference between one device per > function and one virtqueue per function. You'll probably have a single > driver for both anyway, to share common code for buffer management etc. > The semantics of the encoder and decoder on the driver side, at least on Linux where V4L2 is used, are different enough for much of the code to be separated. That said, even with separate devices, the same driver can be instantiated multiple times, which is a common case in Linux, handled by the driver core. That basically gives us the ability to have as many instances of whatever function we want for free, without the need to explicitly handle multiple functions in one device in the driver. So that clearly equals simpler driver code. On the host side, the encode and decode APIs are different as well, so having separate implementation decoder and encoder, possibly just sharing some helper code, would make much more sense. > > > Use separate pixel_format (fourcc) and stream_format (H.264 etc.) enums? > > > > I'd specifically avoid FourCC here, as it's very loosely defined and > > could introduce confusion. > > I don't think using fourcc is a problem, and given that both drm and > v4l2 use fourcc already this would be a good choice I think. Both DRM and V4L2 use two mutually incompatible sets of FourCCs, so I'm not sure how it could be a good choice. At least unless we decide to pick a specific set of FourCC. It doesn't help that Windows/DirectX has its own set of FourCCs that's again slightly different than the two mentioned before. > > But the definition should be more specific than just "fourcc". Best > would be to explicitly list and define each format supported by the > spec. Why not be consistent with virtio-gpu and just define new formats as we add support for them as sequential integers? Best regards, Tomasz - To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
Re: [virtio-dev] [RFC RESEND] virtio-video: Add virtio video device specification
Hi, > > Hmm, modern GPUs support both encoding and decoding ... > > Many SoC architectures have completely separate IP blocks for encoding > and decoding. Similarly, in GPUs those are usually completely separate > parts of the pipeline. In the OS there is one driver per GPU handling both ... > > I don't think we should bake that restriction into the specification. > > It probably makes sense to use one virtqueue per function though, that > > will simplify dispatching in both host and guest. > > > > Wouldn't it make the handling easier if we had one virtio device per function? I don't think there is much of a difference between one device per function and one virtqueue per function. You'll probably have a single driver for both anyway, to share common code for buffer management etc. > > Use separate pixel_format (fourcc) and stream_format (H.264 etc.) enums? > > I'd specifically avoid FourCC here, as it's very loosely defined and > could introduce confusion. I don't think using fourcc is a problem, and given that both drm and v4l2 use fourcc already this would be a good choice I think. But the definition should be more specific than just "fourcc". Best would be to explicitly list and define each format supported by the spec. cheers, Gerd - To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
Re: [virtio-dev] [RFC RESEND] virtio-video: Add virtio video device specification
On Wed, Dec 4, 2019 at 6:16 PM Gerd Hoffmann wrote:
>
> Hi,
>
> > 1. Focus on only decoder/encoder functionalities first.
> >
> > As Tomasz said earlier in this thread, it'd be too complicated to support
> > camera
> > usage at the same time. So, I'd suggest to make it just a generic mem-to-mem
> > video processing device protocol for now.
> > If we finally decide to support camera in this protocol, we can add it
> > later.
>
> Agree.
>
> > 2. Only one feature bit can be specified for one device.
> >
> > I'd like to have a decoder device and encoder device separately.
> > It'd be natural to assume it because a decoder and an encoder are provided
> > as
> > different hardware.
>
> Hmm, modern GPUs support both encoding and decoding ...
>
Many SoC architectures have completely separate IP blocks for encoding
and decoding. Similarly, in GPUs those are usually completely separate
parts of the pipeline.
> I don't think we should bake that restriction into the specification.
> It probably makes sense to use one virtqueue per function though, that
> will simplify dispatching in both host and guest.
>
Wouldn't it make the handling easier if we had one virtio device per function?
[snip]
> > > +\begin{lstlisting}
> > > +enum virtio_video_pixel_format {
> > > + VIRTIO_VIDEO_PIX_FMT_UNDEFINED = 0,
> > > +
> > > + VIRTIO_VIDEO_PIX_FMT_H264 = 0x0100,
> > > + VIRTIO_VIDEO_PIX_FMT_NV12,
> > > + VIRTIO_VIDEO_PIX_FMT_NV21,
> > > + VIRTIO_VIDEO_PIX_FMT_I420,
> > > + VIRTIO_VIDEO_PIX_FMT_I422,
> > > + VIRTIO_VIDEO_PIX_FMT_XBGR,
> > > +};
> >
> > I'm wondering if we can use FOURCC instead. So, we can avoid reinventing a
> > mapping from formats to integers.
> > Also, I suppose the word "pixel formats" means only raw (decoded) formats.
> > But, it can be encoded format like H.264. So, I guess "image format" or
> > "fourcc" is a better word choice.
>
> Use separate pixel_format (fourcc) and stream_format (H.264 etc.) enums?
>
I'd specifically avoid FourCC here, as it's very loosely defined and
could introduce confusion. A separate enum for "image formats",
including both sounds good to me.
> > > +\begin{lstlisting}
> > > +struct virtio_video_function {
> > > + struct virtio_video_desc desc;
> > > + __le32 function_type; /* One of VIRTIO_VIDEO_FUNC_* types */
> > > + __le32 function_id;
> > > + struct virtio_video_params in_params;
> > > + struct virtio_video_params out_params;
> > > + __le32 num_caps;
> > > + __u8 padding[4];
> > > + /* Followed by struct virtio_video_capability video_caps[]; */
> > > +};
> > > +\end{lstlisting}
> >
> > If one device only has one functionality, virtio_video_function's fields
> > will be
> > no longer needed except in_params and out_params. So, we'd be able to remove
> > virtio_video_function and have in_params and out_params in
> > virtio_video_capability instead.
>
> Same goes for per-function virtqueues (used virtqueue implies function).
>
> > > +\begin{lstlisting}
> > > +struct virtio_video_resource_detach_backing {
> > > + struct virtio_video_ctrl_hdr hdr;
> > > + __le32 resource_id;
> > > + __u8 padding[4];
> > > +};
> > > +\end{lstlisting}
> > > +
> > > +\begin{description}
> > > +\item[\field{resource_id}] internal id of the resource.
> > > +\end{description}
> >
> > I suppose that it'd be better not to have the above series of T_RESOURCE
> > controls at least until we reach a conclusion in the thread of
> > buffer-sharing
> > device. If we end up concluding this type of controls is the best way,
> > we'll be
> > able to revisit here.
>
> Well. For buffer management there are a bunch of options.
>
> (1) Simply stick the buffers (well, pointers to the buffer pages) into
> the virtqueue. This is the standard virtio way.
>
> (2) Create resources, then put the resource ids into the virtqueue.
> virtio-gpu uses that model. First, because virtio-gpu needs an id
> to reference resources in the rendering command stream
> (virtio-video doesn't need this). Also because (some kinds of)
> resources are around for a long time and the guest-physical ->
> host-virtual mapping needs to be done only once that way (which
> I think would be the case for virtio-video too because v4l2
> re-uses buffers in robin-round fashion). Drawback is this
> assumes shared memory between host and guest (which is the case
> in typical use cases but it is not mandated by the virtio spec).
>
> (3) Import external resources (from virtio-gpu for example).
> Out of scope for now, will probably added as optional feature
> later.
>
> I guess long-term we want support either (1)+(3) or (2)+(3).
What this protocol has been proposing is a twist of (1), where there
is a "resource create" call that generates a local "resource ID" for
the given list of guest pages. I think that's a sane approach, given
that the number of pages to describe a buff
Re: [virtio-dev] [RFC RESEND] virtio-video: Add virtio video device specification
On Fri, Dec 13, 2019 at 11:20 PM Keiichi Watanabe wrote: > > On Thu, Dec 12, 2019 at 7:34 PM Dmitry Sepp > wrote: > > > > Hi Keiichi, > > > > Thank you for your comment. > > What do you thing about selection/crop rectangles? Should this be defined > > as a > > must have? Or we just assume the device always sticks to the stream > > resolution. > > > > For decoding, the device needs to tell the driver crop rectangle when > a resolution change happens, as virtio-vdec has > virtio_vdec_host_req_stream_crop. > So, we should add a new field in virtio_video_param for it. > > For what it's worth, we have C-headers that exposes Chrome's video > codec functionalities. So, our virtio-video device will do > decoding/encoding by calling these functions. > Decoder: > https://chromium.git.corp.google.com/chromiumos/platform2/+/00ad71fa640c018c1d014f467c50b9a7dc962a10/arc/vm/libvda/libvda_decode.h > Encoder: > https://chromium.git.corp.google.com/chromiumos/platform2/+/00ad71fa640c018c1d014f467c50b9a7dc962a10/arc/vm/libvda/libvda_encode.h > Oops, the links were not accesible... The correct links are: Decoder: https://chromium.googlesource.com/chromiumos/platform2/+/00ad71fa640c018c1d014f467c50b9a7dc962a10/arc/vm/libvda/libvda_decode.h Encoder: https://chromium.googlesource.com/chromiumos/platform2/+/00ad71fa640c018c1d014f467c50b9a7dc962a10/arc/vm/libvda/libvda_encode.h Best, Keiichi > BTW, I'm preparing the next version of virtio-video spec by addressing > points we discussed in this thread. I'll send it here next week, > hopefully. > > Best, > Keiichi > > > > > Regards, > > Dmitry. > > > > On Donnerstag, 12. Dezember 2019 06:39:11 CET Keiichi Watanabe wrote: > > > On Tue, Dec 10, 2019 at 10:16 PM Dmitry Sepp > > > > > > wrote: > > > > Hi, > > > > > > > > Just to start, let's consider this v4l2 control: > > > > V4L2_CID_MPEG_VIDEO_FRAME_RC_ENABLE. > > > > As I can see, this control is referenced as a mandatory one in the > > > > Chromium > > > > sources [1]. > > > > > > > > So could someone from the Chromium team please explain why it is > > > > mandatory? > > > > (YouTube?) In fact, almost no encoders implement this control. Do we > > > > need > > > > it in virtio-video? > > > > > > That control is used to encode videos in constant bitrate (CBR) mode, > > > which is critical for > > > real-time use case like video conferencing. > > > > > > However, that Chromium source code just requires *host-side* drivers > > > to have the v4l2 > > > control. Also, I guess it's rare that a guest app wants to use CQP > > > instead of CBR from our > > > experience. > > > So, I suppose we can omit this control in virtio spec for now by > > > assuming that guests > > > always use CBR mode. > > > > > > Best regards, > > > Keiichi > > > > > > > [1] > > > > https://chromium.googlesource.com/chromium/src/media/+/refs/heads/master/ > > > > gpu/v4l2/v4l2_video_encode_accelerator.cc#1500 > > > > > > > > Regards, > > > > Dmitry. > > > > > > > > On Montag, 9. Dezember 2019 22:12:28 CET Enrico Granata wrote: > > > > > +Changyeon Jo for his awareness > > > > > > > > > > Thanks, > > > > > - Enrico > > > > > > > > > > > > > > > On Mon, Dec 9, 2019 at 6:20 AM Dmitry Sepp > > > > > > > > > > > > > > > wrote: > > > > > > Hello, > > > > > > > > > > > > I'd like to invite everyone to share ideas regarding required > > > > > > encoder > > > > > > features > > > > > > in this separate sub-tree. > > > > > > > > > > > > In general, encoder devices are more complex compared to decoders. > > > > > > So > > > > > > the > > > > > > question I'd like to rise is in what way we define the minimal > > > > > > subset > > > > > > of > > > > > > features to be implemented by the virtio-video. > > > > > > > > > > > > We may look at the following to define the set of features: > > > > > > 1. USB video, 2.3.6 Encoding Unit [1]. > > > > > > 2. Android 10 Compatibility Definition [2]. > > > > > > > > > > > > Would be nice to hear about any specific requirements from the > > > > > > Chromium > > > > > > team as > > > > > > well. > > > > > > > > > > > > [1] https://www.usb.org/sites/default/files/USB_Video_Class_1_5.zip > > > > > > [2] > > > > > > https://source.android.com/compatibility/android-cdd#5_2_video_encodin > > > > > > g > > > > > > > > > > > > Thank you. > > > > > > > > > > > > Best regards, > > > > > > Dmitry. > > > > > > > > > > > > On Mittwoch, 4. Dezember 2019 10:16:20 CET Gerd Hoffmann wrote: > > > > > > > Hi, > > > > > > > > > > > > > > > 1. Focus on only decoder/encoder functionalities first. > > > > > > > > > > > > > > > > As Tomasz said earlier in this thread, it'd be too complicated > > > > > > > > to > > > > > > > > > > > > support > > > > > > > > > > > > > > camera usage at the same time. So, I'd suggest to make it just a > > > > > > > > > > > > generic > > > > > > > > > > > > > > mem-to-mem video processing device protocol for now. > > > > > > > > If we finally decide to support camera in this protocol, we can > > > > > > > > add it
Re: [virtio-dev] [RFC RESEND] virtio-video: Add virtio video device specification
+spice-devel for awareness
Context: there is a lot of work there on video streaming for SPICE, mostly
done ATM through proprietary APIs.
> On 9 Dec 2019, at 15:19, Dmitry Sepp wrote:
>
> Hello,
>
> I'd like to invite everyone to share ideas regarding required encoder
> features
> in this separate sub-tree.
>
> In general, encoder devices are more complex compared to decoders. So the
> question I'd like to rise is in what way we define the minimal subset of
> features to be implemented by the virtio-video.
>
> We may look at the following to define the set of features:
> 1. USB video, 2.3.6 Encoding Unit [1].
> 2. Android 10 Compatibility Definition [2].
>
> Would be nice to hear about any specific requirements from the Chromium team
> as
> well.
>
> [1] https://www.usb.org/sites/default/files/USB_Video_Class_1_5.zip
> [2] https://source.android.com/compatibility/android-cdd#5_2_video_encoding
>
> Thank you.
>
> Best regards,
> Dmitry.
>
> On Mittwoch, 4. Dezember 2019 10:16:20 CET Gerd Hoffmann wrote:
>> Hi,
>>
>>> 1. Focus on only decoder/encoder functionalities first.
>>>
>>> As Tomasz said earlier in this thread, it'd be too complicated to support
>>> camera usage at the same time. So, I'd suggest to make it just a generic
>>> mem-to-mem video processing device protocol for now.
>>> If we finally decide to support camera in this protocol, we can add it
>>> later.
>> Agree.
>>
>>> 2. Only one feature bit can be specified for one device.
>>>
>>> I'd like to have a decoder device and encoder device separately.
>>> It'd be natural to assume it because a decoder and an encoder are provided
>>> as different hardware.
>>
>> Hmm, modern GPUs support both encoding and decoding ...
>>
>> I don't think we should bake that restriction into the specification.
>> It probably makes sense to use one virtqueue per function though, that
>> will simplify dispatching in both host and guest.
>>
>>> 3. Separate buffer allocation functionalities from virtio-video protocol.
>>>
>>> To support various ways of guest/host buffer sharing, we might want to
>>> have a dedicated buffer sharing device as we're discussing in another
>>> thread. Until we reach consensus there, it'd be good not to have buffer
>>> allocation
>>> functionalities in virtio-video.
>>
>> I think virtio-video should be able to work as stand-alone device,
>> so we need some way to allocate buffers ...
>>
>> Buffer sharing with other devices can be added later.
>>
+The virtio video device is a virtual video streaming device that
supports the +following functions: encoder, decoder, capture, output.
+
+\subsection{Device ID}\label{sec:Device Types / Video Device / Device
ID}
+
+TBD.
>>>
>>> I'm wondering how and when we can determine and reserve this ID?
>>
>> Grab the next free, update the spec accordingly, submit the one-line
>> patch.
>>
+\begin{lstlisting}
+enum virtio_video_pixel_format {
+ VIRTIO_VIDEO_PIX_FMT_UNDEFINED = 0,
+
+ VIRTIO_VIDEO_PIX_FMT_H264 = 0x0100,
+ VIRTIO_VIDEO_PIX_FMT_NV12,
+ VIRTIO_VIDEO_PIX_FMT_NV21,
+ VIRTIO_VIDEO_PIX_FMT_I420,
+ VIRTIO_VIDEO_PIX_FMT_I422,
+ VIRTIO_VIDEO_PIX_FMT_XBGR,
+};
>>>
>>> I'm wondering if we can use FOURCC instead. So, we can avoid reinventing a
>>> mapping from formats to integers.
>>> Also, I suppose the word "pixel formats" means only raw (decoded) formats.
>>> But, it can be encoded format like H.264. So, I guess "image format" or
>>> "fourcc" is a better word choice.
>>
>> Use separate pixel_format (fourcc) and stream_format (H.264 etc.) enums?
>>
+\begin{lstlisting}
+struct virtio_video_function {
+ struct virtio_video_desc desc;
+ __le32 function_type; /* One of VIRTIO_VIDEO_FUNC_* types */
+ __le32 function_id;
+ struct virtio_video_params in_params;
+ struct virtio_video_params out_params;
+ __le32 num_caps;
+ __u8 padding[4];
+ /* Followed by struct virtio_video_capability video_caps[]; */
+};
+\end{lstlisting}
>>>
>>> If one device only has one functionality, virtio_video_function's fields
>>> will be no longer needed except in_params and out_params. So, we'd be
>>> able to remove virtio_video_function and have in_params and out_params in
>>> virtio_video_capability instead.
>>
>> Same goes for per-function virtqueues (used virtqueue implies function).
>>
+\begin{lstlisting}
+struct virtio_video_resource_detach_backing {
+ struct virtio_video_ctrl_hdr hdr;
+ __le32 resource_id;
+ __u8 padding[4];
+};
+\end{lstlisting}
+
+\begin{description}
+\item[\field{resource_id}] internal id of the resource.
+\end{description}
>>>
>>> I suppose that it'd be better not to have the above series of T_RESOURCE
>>> controls at least until we reach a conclusion in the thre
Re: [virtio-dev] [RFC RESEND] virtio-video: Add virtio video device specification
On Thu, Dec 12, 2019 at 7:34 PM Dmitry Sepp wrote: > > Hi Keiichi, > > Thank you for your comment. > What do you thing about selection/crop rectangles? Should this be defined as a > must have? Or we just assume the device always sticks to the stream > resolution. > For decoding, the device needs to tell the driver crop rectangle when a resolution change happens, as virtio-vdec has virtio_vdec_host_req_stream_crop. So, we should add a new field in virtio_video_param for it. For what it's worth, we have C-headers that exposes Chrome's video codec functionalities. So, our virtio-video device will do decoding/encoding by calling these functions. Decoder: https://chromium.git.corp.google.com/chromiumos/platform2/+/00ad71fa640c018c1d014f467c50b9a7dc962a10/arc/vm/libvda/libvda_decode.h Encoder: https://chromium.git.corp.google.com/chromiumos/platform2/+/00ad71fa640c018c1d014f467c50b9a7dc962a10/arc/vm/libvda/libvda_encode.h BTW, I'm preparing the next version of virtio-video spec by addressing points we discussed in this thread. I'll send it here next week, hopefully. Best, Keiichi > Regards, > Dmitry. > > On Donnerstag, 12. Dezember 2019 06:39:11 CET Keiichi Watanabe wrote: > > On Tue, Dec 10, 2019 at 10:16 PM Dmitry Sepp > > > > wrote: > > > Hi, > > > > > > Just to start, let's consider this v4l2 control: > > > V4L2_CID_MPEG_VIDEO_FRAME_RC_ENABLE. > > > As I can see, this control is referenced as a mandatory one in the > > > Chromium > > > sources [1]. > > > > > > So could someone from the Chromium team please explain why it is > > > mandatory? > > > (YouTube?) In fact, almost no encoders implement this control. Do we need > > > it in virtio-video? > > > > That control is used to encode videos in constant bitrate (CBR) mode, > > which is critical for > > real-time use case like video conferencing. > > > > However, that Chromium source code just requires *host-side* drivers > > to have the v4l2 > > control. Also, I guess it's rare that a guest app wants to use CQP > > instead of CBR from our > > experience. > > So, I suppose we can omit this control in virtio spec for now by > > assuming that guests > > always use CBR mode. > > > > Best regards, > > Keiichi > > > > > [1] > > > https://chromium.googlesource.com/chromium/src/media/+/refs/heads/master/ > > > gpu/v4l2/v4l2_video_encode_accelerator.cc#1500 > > > > > > Regards, > > > Dmitry. > > > > > > On Montag, 9. Dezember 2019 22:12:28 CET Enrico Granata wrote: > > > > +Changyeon Jo for his awareness > > > > > > > > Thanks, > > > > - Enrico > > > > > > > > > > > > On Mon, Dec 9, 2019 at 6:20 AM Dmitry Sepp > > > > > > > > wrote: > > > > > Hello, > > > > > > > > > > I'd like to invite everyone to share ideas regarding required encoder > > > > > features > > > > > in this separate sub-tree. > > > > > > > > > > In general, encoder devices are more complex compared to decoders. So > > > > > the > > > > > question I'd like to rise is in what way we define the minimal subset > > > > > of > > > > > features to be implemented by the virtio-video. > > > > > > > > > > We may look at the following to define the set of features: > > > > > 1. USB video, 2.3.6 Encoding Unit [1]. > > > > > 2. Android 10 Compatibility Definition [2]. > > > > > > > > > > Would be nice to hear about any specific requirements from the > > > > > Chromium > > > > > team as > > > > > well. > > > > > > > > > > [1] https://www.usb.org/sites/default/files/USB_Video_Class_1_5.zip > > > > > [2] > > > > > https://source.android.com/compatibility/android-cdd#5_2_video_encodin > > > > > g > > > > > > > > > > Thank you. > > > > > > > > > > Best regards, > > > > > Dmitry. > > > > > > > > > > On Mittwoch, 4. Dezember 2019 10:16:20 CET Gerd Hoffmann wrote: > > > > > > Hi, > > > > > > > > > > > > > 1. Focus on only decoder/encoder functionalities first. > > > > > > > > > > > > > > As Tomasz said earlier in this thread, it'd be too complicated to > > > > > > > > > > support > > > > > > > > > > > > camera usage at the same time. So, I'd suggest to make it just a > > > > > > > > > > generic > > > > > > > > > > > > mem-to-mem video processing device protocol for now. > > > > > > > If we finally decide to support camera in this protocol, we can > > > > > > > add it > > > > > > > later. > > > > > > > > > > > > Agree. > > > > > > > > > > > > > 2. Only one feature bit can be specified for one device. > > > > > > > > > > > > > > I'd like to have a decoder device and encoder device separately. > > > > > > > It'd be natural to assume it because a decoder and an encoder are > > > > > > > > > > provided > > > > > > > > > > > > as different hardware. > > > > > > > > > > > > Hmm, modern GPUs support both encoding and decoding ... > > > > > > > > > > > > I don't think we should bake that restriction into the > > > > > > specification. > > > > > > It probably makes sense to use one virtqueue per function though, > > > > > > that > > > > > > will simplify dispatching in both host and guest. > > > > > > > > > > > > > 3.
Re: [virtio-dev] [RFC RESEND] virtio-video: Add virtio video device specification
Hi Keiichi,
Thank you for your comment.
What do you thing about selection/crop rectangles? Should this be defined as a
must have? Or we just assume the device always sticks to the stream
resolution.
Regards,
Dmitry.
On Donnerstag, 12. Dezember 2019 06:39:11 CET Keiichi Watanabe wrote:
> On Tue, Dec 10, 2019 at 10:16 PM Dmitry Sepp
>
> wrote:
> > Hi,
> >
> > Just to start, let's consider this v4l2 control:
> > V4L2_CID_MPEG_VIDEO_FRAME_RC_ENABLE.
> > As I can see, this control is referenced as a mandatory one in the
> > Chromium
> > sources [1].
> >
> > So could someone from the Chromium team please explain why it is
> > mandatory?
> > (YouTube?) In fact, almost no encoders implement this control. Do we need
> > it in virtio-video?
>
> That control is used to encode videos in constant bitrate (CBR) mode,
> which is critical for
> real-time use case like video conferencing.
>
> However, that Chromium source code just requires *host-side* drivers
> to have the v4l2
> control. Also, I guess it's rare that a guest app wants to use CQP
> instead of CBR from our
> experience.
> So, I suppose we can omit this control in virtio spec for now by
> assuming that guests
> always use CBR mode.
>
> Best regards,
> Keiichi
>
> > [1]
> > https://chromium.googlesource.com/chromium/src/media/+/refs/heads/master/
> > gpu/v4l2/v4l2_video_encode_accelerator.cc#1500
> >
> > Regards,
> > Dmitry.
> >
> > On Montag, 9. Dezember 2019 22:12:28 CET Enrico Granata wrote:
> > > +Changyeon Jo for his awareness
> > >
> > > Thanks,
> > > - Enrico
> > >
> > >
> > > On Mon, Dec 9, 2019 at 6:20 AM Dmitry Sepp
> > >
> > > wrote:
> > > > Hello,
> > > >
> > > > I'd like to invite everyone to share ideas regarding required encoder
> > > > features
> > > > in this separate sub-tree.
> > > >
> > > > In general, encoder devices are more complex compared to decoders. So
> > > > the
> > > > question I'd like to rise is in what way we define the minimal subset
> > > > of
> > > > features to be implemented by the virtio-video.
> > > >
> > > > We may look at the following to define the set of features:
> > > > 1. USB video, 2.3.6 Encoding Unit [1].
> > > > 2. Android 10 Compatibility Definition [2].
> > > >
> > > > Would be nice to hear about any specific requirements from the
> > > > Chromium
> > > > team as
> > > > well.
> > > >
> > > > [1] https://www.usb.org/sites/default/files/USB_Video_Class_1_5.zip
> > > > [2]
> > > > https://source.android.com/compatibility/android-cdd#5_2_video_encodin
> > > > g
> > > >
> > > > Thank you.
> > > >
> > > > Best regards,
> > > > Dmitry.
> > > >
> > > > On Mittwoch, 4. Dezember 2019 10:16:20 CET Gerd Hoffmann wrote:
> > > > > Hi,
> > > > >
> > > > > > 1. Focus on only decoder/encoder functionalities first.
> > > > > >
> > > > > > As Tomasz said earlier in this thread, it'd be too complicated to
> > > >
> > > > support
> > > >
> > > > > > camera usage at the same time. So, I'd suggest to make it just a
> > > >
> > > > generic
> > > >
> > > > > > mem-to-mem video processing device protocol for now.
> > > > > > If we finally decide to support camera in this protocol, we can
> > > > > > add it
> > > > > > later.
> > > > >
> > > > > Agree.
> > > > >
> > > > > > 2. Only one feature bit can be specified for one device.
> > > > > >
> > > > > > I'd like to have a decoder device and encoder device separately.
> > > > > > It'd be natural to assume it because a decoder and an encoder are
> > > >
> > > > provided
> > > >
> > > > > > as different hardware.
> > > > >
> > > > > Hmm, modern GPUs support both encoding and decoding ...
> > > > >
> > > > > I don't think we should bake that restriction into the
> > > > > specification.
> > > > > It probably makes sense to use one virtqueue per function though,
> > > > > that
> > > > > will simplify dispatching in both host and guest.
> > > > >
> > > > > > 3. Separate buffer allocation functionalities from virtio-video
> > > >
> > > > protocol.
> > > >
> > > > > > To support various ways of guest/host buffer sharing, we might
> > > > > > want to
> > > > > > have a dedicated buffer sharing device as we're discussing in
> > > > > > another
> > > > > > thread. Until we reach consensus there, it'd be good not to have
> > > > > > buffer
> > > > > > allocation
> > > > > > functionalities in virtio-video.
> > > > >
> > > > > I think virtio-video should be able to work as stand-alone device,
> > > > > so we need some way to allocate buffers ...
> > > > >
> > > > > Buffer sharing with other devices can be added later.
> > > > >
> > > > > > > +The virtio video device is a virtual video streaming device
> > > > > > > that
> > > > > > > supports the +following functions: encoder, decoder, capture,
> > > > > > > output.
> > > > > > > +
> > > > > > > +\subsection{Device ID}\label{sec:Device Types / Video Device /
> > > >
> > > > Device
> > > >
> > > > > > > ID}
> > > > > > > +
> > > > > > > +TBD.
> > > > > >
> > > > > > I'm wondering how and when
Re: [virtio-dev] [RFC RESEND] virtio-video: Add virtio video device specification
On Tue, Dec 10, 2019 at 10:16 PM Dmitry Sepp
wrote:
>
> Hi,
>
> Just to start, let's consider this v4l2 control:
> V4L2_CID_MPEG_VIDEO_FRAME_RC_ENABLE.
> As I can see, this control is referenced as a mandatory one in the Chromium
> sources [1].
>
> So could someone from the Chromium team please explain why it is mandatory?
> (YouTube?) In fact, almost no encoders implement this control. Do we need it
> in virtio-video?
That control is used to encode videos in constant bitrate (CBR) mode,
which is critical for
real-time use case like video conferencing.
However, that Chromium source code just requires *host-side* drivers
to have the v4l2
control. Also, I guess it's rare that a guest app wants to use CQP
instead of CBR from our
experience.
So, I suppose we can omit this control in virtio spec for now by
assuming that guests
always use CBR mode.
Best regards,
Keiichi
>
> [1] https://chromium.googlesource.com/chromium/src/media/+/refs/heads/master/
> gpu/v4l2/v4l2_video_encode_accelerator.cc#1500
>
> Regards,
> Dmitry.
>
> On Montag, 9. Dezember 2019 22:12:28 CET Enrico Granata wrote:
> > +Changyeon Jo for his awareness
> >
> > Thanks,
> > - Enrico
> >
> >
> > On Mon, Dec 9, 2019 at 6:20 AM Dmitry Sepp
> >
> > wrote:
> > > Hello,
> > >
> > > I'd like to invite everyone to share ideas regarding required encoder
> > > features
> > > in this separate sub-tree.
> > >
> > > In general, encoder devices are more complex compared to decoders. So the
> > > question I'd like to rise is in what way we define the minimal subset of
> > > features to be implemented by the virtio-video.
> > >
> > > We may look at the following to define the set of features:
> > > 1. USB video, 2.3.6 Encoding Unit [1].
> > > 2. Android 10 Compatibility Definition [2].
> > >
> > > Would be nice to hear about any specific requirements from the Chromium
> > > team as
> > > well.
> > >
> > > [1] https://www.usb.org/sites/default/files/USB_Video_Class_1_5.zip
> > > [2]
> > > https://source.android.com/compatibility/android-cdd#5_2_video_encoding
> > >
> > > Thank you.
> > >
> > > Best regards,
> > > Dmitry.
> > >
> > > On Mittwoch, 4. Dezember 2019 10:16:20 CET Gerd Hoffmann wrote:
> > > > Hi,
> > > >
> > > > > 1. Focus on only decoder/encoder functionalities first.
> > > > >
> > > > > As Tomasz said earlier in this thread, it'd be too complicated to
> > >
> > > support
> > >
> > > > > camera usage at the same time. So, I'd suggest to make it just a
> > >
> > > generic
> > >
> > > > > mem-to-mem video processing device protocol for now.
> > > > > If we finally decide to support camera in this protocol, we can add it
> > > > > later.
> > > >
> > > > Agree.
> > > >
> > > > > 2. Only one feature bit can be specified for one device.
> > > > >
> > > > > I'd like to have a decoder device and encoder device separately.
> > > > > It'd be natural to assume it because a decoder and an encoder are
> > >
> > > provided
> > >
> > > > > as different hardware.
> > > >
> > > > Hmm, modern GPUs support both encoding and decoding ...
> > > >
> > > > I don't think we should bake that restriction into the specification.
> > > > It probably makes sense to use one virtqueue per function though, that
> > > > will simplify dispatching in both host and guest.
> > > >
> > > > > 3. Separate buffer allocation functionalities from virtio-video
> > >
> > > protocol.
> > >
> > > > > To support various ways of guest/host buffer sharing, we might want to
> > > > > have a dedicated buffer sharing device as we're discussing in another
> > > > > thread. Until we reach consensus there, it'd be good not to have
> > > > > buffer
> > > > > allocation
> > > > > functionalities in virtio-video.
> > > >
> > > > I think virtio-video should be able to work as stand-alone device,
> > > > so we need some way to allocate buffers ...
> > > >
> > > > Buffer sharing with other devices can be added later.
> > > >
> > > > > > +The virtio video device is a virtual video streaming device that
> > > > > > supports the +following functions: encoder, decoder, capture,
> > > > > > output.
> > > > > > +
> > > > > > +\subsection{Device ID}\label{sec:Device Types / Video Device /
> > >
> > > Device
> > >
> > > > > > ID}
> > > > > > +
> > > > > > +TBD.
> > > > >
> > > > > I'm wondering how and when we can determine and reserve this ID?
> > > >
> > > > Grab the next free, update the spec accordingly, submit the one-line
> > > > patch.
> > > >
> > > > > > +\begin{lstlisting}
> > > > > > +enum virtio_video_pixel_format {
> > > > > > + VIRTIO_VIDEO_PIX_FMT_UNDEFINED = 0,
> > > > > > +
> > > > > > + VIRTIO_VIDEO_PIX_FMT_H264 = 0x0100,
> > > > > > + VIRTIO_VIDEO_PIX_FMT_NV12,
> > > > > > + VIRTIO_VIDEO_PIX_FMT_NV21,
> > > > > > + VIRTIO_VIDEO_PIX_FMT_I420,
> > > > > > + VIRTIO_VIDEO_PIX_FMT_I422,
> > > > > > + VIRTIO_VIDEO_PIX_FMT_XBGR,
> > > > > > +};
> > > > >
> > > > > I'm wondering if we can use FOURCC instead. So, we can avoid
> > >
> > > reinventing a
> > >
>
Re: [virtio-dev] [RFC RESEND] virtio-video: Add virtio video device specification
Hi,
Just to start, let's consider this v4l2 control:
V4L2_CID_MPEG_VIDEO_FRAME_RC_ENABLE.
As I can see, this control is referenced as a mandatory one in the Chromium
sources [1].
So could someone from the Chromium team please explain why it is mandatory?
(YouTube?) In fact, almost no encoders implement this control. Do we need it
in virtio-video?
[1] https://chromium.googlesource.com/chromium/src/media/+/refs/heads/master/
gpu/v4l2/v4l2_video_encode_accelerator.cc#1500
Regards,
Dmitry.
On Montag, 9. Dezember 2019 22:12:28 CET Enrico Granata wrote:
> +Changyeon Jo for his awareness
>
> Thanks,
> - Enrico
>
>
> On Mon, Dec 9, 2019 at 6:20 AM Dmitry Sepp
>
> wrote:
> > Hello,
> >
> > I'd like to invite everyone to share ideas regarding required encoder
> > features
> > in this separate sub-tree.
> >
> > In general, encoder devices are more complex compared to decoders. So the
> > question I'd like to rise is in what way we define the minimal subset of
> > features to be implemented by the virtio-video.
> >
> > We may look at the following to define the set of features:
> > 1. USB video, 2.3.6 Encoding Unit [1].
> > 2. Android 10 Compatibility Definition [2].
> >
> > Would be nice to hear about any specific requirements from the Chromium
> > team as
> > well.
> >
> > [1] https://www.usb.org/sites/default/files/USB_Video_Class_1_5.zip
> > [2]
> > https://source.android.com/compatibility/android-cdd#5_2_video_encoding
> >
> > Thank you.
> >
> > Best regards,
> > Dmitry.
> >
> > On Mittwoch, 4. Dezember 2019 10:16:20 CET Gerd Hoffmann wrote:
> > > Hi,
> > >
> > > > 1. Focus on only decoder/encoder functionalities first.
> > > >
> > > > As Tomasz said earlier in this thread, it'd be too complicated to
> >
> > support
> >
> > > > camera usage at the same time. So, I'd suggest to make it just a
> >
> > generic
> >
> > > > mem-to-mem video processing device protocol for now.
> > > > If we finally decide to support camera in this protocol, we can add it
> > > > later.
> > >
> > > Agree.
> > >
> > > > 2. Only one feature bit can be specified for one device.
> > > >
> > > > I'd like to have a decoder device and encoder device separately.
> > > > It'd be natural to assume it because a decoder and an encoder are
> >
> > provided
> >
> > > > as different hardware.
> > >
> > > Hmm, modern GPUs support both encoding and decoding ...
> > >
> > > I don't think we should bake that restriction into the specification.
> > > It probably makes sense to use one virtqueue per function though, that
> > > will simplify dispatching in both host and guest.
> > >
> > > > 3. Separate buffer allocation functionalities from virtio-video
> >
> > protocol.
> >
> > > > To support various ways of guest/host buffer sharing, we might want to
> > > > have a dedicated buffer sharing device as we're discussing in another
> > > > thread. Until we reach consensus there, it'd be good not to have
> > > > buffer
> > > > allocation
> > > > functionalities in virtio-video.
> > >
> > > I think virtio-video should be able to work as stand-alone device,
> > > so we need some way to allocate buffers ...
> > >
> > > Buffer sharing with other devices can be added later.
> > >
> > > > > +The virtio video device is a virtual video streaming device that
> > > > > supports the +following functions: encoder, decoder, capture,
> > > > > output.
> > > > > +
> > > > > +\subsection{Device ID}\label{sec:Device Types / Video Device /
> >
> > Device
> >
> > > > > ID}
> > > > > +
> > > > > +TBD.
> > > >
> > > > I'm wondering how and when we can determine and reserve this ID?
> > >
> > > Grab the next free, update the spec accordingly, submit the one-line
> > > patch.
> > >
> > > > > +\begin{lstlisting}
> > > > > +enum virtio_video_pixel_format {
> > > > > + VIRTIO_VIDEO_PIX_FMT_UNDEFINED = 0,
> > > > > +
> > > > > + VIRTIO_VIDEO_PIX_FMT_H264 = 0x0100,
> > > > > + VIRTIO_VIDEO_PIX_FMT_NV12,
> > > > > + VIRTIO_VIDEO_PIX_FMT_NV21,
> > > > > + VIRTIO_VIDEO_PIX_FMT_I420,
> > > > > + VIRTIO_VIDEO_PIX_FMT_I422,
> > > > > + VIRTIO_VIDEO_PIX_FMT_XBGR,
> > > > > +};
> > > >
> > > > I'm wondering if we can use FOURCC instead. So, we can avoid
> >
> > reinventing a
> >
> > > > mapping from formats to integers.
> > > > Also, I suppose the word "pixel formats" means only raw (decoded)
> >
> > formats.
> >
> > > > But, it can be encoded format like H.264. So, I guess "image format"
> > > > or
> > > > "fourcc" is a better word choice.
> > >
> > > Use separate pixel_format (fourcc) and stream_format (H.264 etc.) enums?
> > >
> > > > > +\begin{lstlisting}
> > > > > +struct virtio_video_function {
> > > > > + struct virtio_video_desc desc;
> > > > > + __le32 function_type; /* One of VIRTIO_VIDEO_FUNC_* types */
> > > > > + __le32 function_id;
> > > > > + struct virtio_video_params in_params;
> > > > > + struct virtio_video_params out_params;
> > > > > + __le32
Re: [virtio-dev] [RFC RESEND] virtio-video: Add virtio video device specification
+Changyeon Jo for his awareness
Thanks,
- Enrico
On Mon, Dec 9, 2019 at 6:20 AM Dmitry Sepp
wrote:
> Hello,
>
> I'd like to invite everyone to share ideas regarding required encoder
> features
> in this separate sub-tree.
>
> In general, encoder devices are more complex compared to decoders. So the
> question I'd like to rise is in what way we define the minimal subset of
> features to be implemented by the virtio-video.
>
> We may look at the following to define the set of features:
> 1. USB video, 2.3.6 Encoding Unit [1].
> 2. Android 10 Compatibility Definition [2].
>
> Would be nice to hear about any specific requirements from the Chromium
> team as
> well.
>
> [1] https://www.usb.org/sites/default/files/USB_Video_Class_1_5.zip
> [2]
> https://source.android.com/compatibility/android-cdd#5_2_video_encoding
>
> Thank you.
>
> Best regards,
> Dmitry.
>
> On Mittwoch, 4. Dezember 2019 10:16:20 CET Gerd Hoffmann wrote:
> > Hi,
> >
> > > 1. Focus on only decoder/encoder functionalities first.
> > >
> > > As Tomasz said earlier in this thread, it'd be too complicated to
> support
> > > camera usage at the same time. So, I'd suggest to make it just a
> generic
> > > mem-to-mem video processing device protocol for now.
> > > If we finally decide to support camera in this protocol, we can add it
> > > later.
> > Agree.
> >
> > > 2. Only one feature bit can be specified for one device.
> > >
> > > I'd like to have a decoder device and encoder device separately.
> > > It'd be natural to assume it because a decoder and an encoder are
> provided
> > > as different hardware.
> >
> > Hmm, modern GPUs support both encoding and decoding ...
> >
> > I don't think we should bake that restriction into the specification.
> > It probably makes sense to use one virtqueue per function though, that
> > will simplify dispatching in both host and guest.
> >
> > > 3. Separate buffer allocation functionalities from virtio-video
> protocol.
> > >
> > > To support various ways of guest/host buffer sharing, we might want to
> > > have a dedicated buffer sharing device as we're discussing in another
> > > thread. Until we reach consensus there, it'd be good not to have buffer
> > > allocation
> > > functionalities in virtio-video.
> >
> > I think virtio-video should be able to work as stand-alone device,
> > so we need some way to allocate buffers ...
> >
> > Buffer sharing with other devices can be added later.
> >
> > > > +The virtio video device is a virtual video streaming device that
> > > > supports the +following functions: encoder, decoder, capture, output.
> > > > +
> > > > +\subsection{Device ID}\label{sec:Device Types / Video Device /
> Device
> > > > ID}
> > > > +
> > > > +TBD.
> > >
> > > I'm wondering how and when we can determine and reserve this ID?
> >
> > Grab the next free, update the spec accordingly, submit the one-line
> > patch.
> >
> > > > +\begin{lstlisting}
> > > > +enum virtio_video_pixel_format {
> > > > + VIRTIO_VIDEO_PIX_FMT_UNDEFINED = 0,
> > > > +
> > > > + VIRTIO_VIDEO_PIX_FMT_H264 = 0x0100,
> > > > + VIRTIO_VIDEO_PIX_FMT_NV12,
> > > > + VIRTIO_VIDEO_PIX_FMT_NV21,
> > > > + VIRTIO_VIDEO_PIX_FMT_I420,
> > > > + VIRTIO_VIDEO_PIX_FMT_I422,
> > > > + VIRTIO_VIDEO_PIX_FMT_XBGR,
> > > > +};
> > >
> > > I'm wondering if we can use FOURCC instead. So, we can avoid
> reinventing a
> > > mapping from formats to integers.
> > > Also, I suppose the word "pixel formats" means only raw (decoded)
> formats.
> > > But, it can be encoded format like H.264. So, I guess "image format" or
> > > "fourcc" is a better word choice.
> >
> > Use separate pixel_format (fourcc) and stream_format (H.264 etc.) enums?
> >
> > > > +\begin{lstlisting}
> > > > +struct virtio_video_function {
> > > > + struct virtio_video_desc desc;
> > > > + __le32 function_type; /* One of VIRTIO_VIDEO_FUNC_* types */
> > > > + __le32 function_id;
> > > > + struct virtio_video_params in_params;
> > > > + struct virtio_video_params out_params;
> > > > + __le32 num_caps;
> > > > + __u8 padding[4];
> > > > + /* Followed by struct virtio_video_capability video_caps[];
> */
> > > > +};
> > > > +\end{lstlisting}
> > >
> > > If one device only has one functionality, virtio_video_function's
> fields
> > > will be no longer needed except in_params and out_params. So, we'd be
> > > able to remove virtio_video_function and have in_params and out_params
> in
> > > virtio_video_capability instead.
> >
> > Same goes for per-function virtqueues (used virtqueue implies function).
> >
> > > > +\begin{lstlisting}
> > > > +struct virtio_video_resource_detach_backing {
> > > > + struct virtio_video_ctrl_hdr hdr;
> > > > + __le32 resource_id;
> > > > + __u8 padding[4];
> > > > +};
> > > > +\end{lstlisting}
> > > > +
> > > > +\begin{description}
> > > > +\item[\field{resource_id}] internal id of the resource.
> > > > +\end{description}
> > >
> > > I suppose th
Re: [virtio-dev] [RFC RESEND] virtio-video: Add virtio video device specification
Hello,
I'd like to invite everyone to share ideas regarding required encoder features
in this separate sub-tree.
In general, encoder devices are more complex compared to decoders. So the
question I'd like to rise is in what way we define the minimal subset of
features to be implemented by the virtio-video.
We may look at the following to define the set of features:
1. USB video, 2.3.6 Encoding Unit [1].
2. Android 10 Compatibility Definition [2].
Would be nice to hear about any specific requirements from the Chromium team as
well.
[1] https://www.usb.org/sites/default/files/USB_Video_Class_1_5.zip
[2] https://source.android.com/compatibility/android-cdd#5_2_video_encoding
Thank you.
Best regards,
Dmitry.
On Mittwoch, 4. Dezember 2019 10:16:20 CET Gerd Hoffmann wrote:
> Hi,
>
> > 1. Focus on only decoder/encoder functionalities first.
> >
> > As Tomasz said earlier in this thread, it'd be too complicated to support
> > camera usage at the same time. So, I'd suggest to make it just a generic
> > mem-to-mem video processing device protocol for now.
> > If we finally decide to support camera in this protocol, we can add it
> > later.
> Agree.
>
> > 2. Only one feature bit can be specified for one device.
> >
> > I'd like to have a decoder device and encoder device separately.
> > It'd be natural to assume it because a decoder and an encoder are provided
> > as different hardware.
>
> Hmm, modern GPUs support both encoding and decoding ...
>
> I don't think we should bake that restriction into the specification.
> It probably makes sense to use one virtqueue per function though, that
> will simplify dispatching in both host and guest.
>
> > 3. Separate buffer allocation functionalities from virtio-video protocol.
> >
> > To support various ways of guest/host buffer sharing, we might want to
> > have a dedicated buffer sharing device as we're discussing in another
> > thread. Until we reach consensus there, it'd be good not to have buffer
> > allocation
> > functionalities in virtio-video.
>
> I think virtio-video should be able to work as stand-alone device,
> so we need some way to allocate buffers ...
>
> Buffer sharing with other devices can be added later.
>
> > > +The virtio video device is a virtual video streaming device that
> > > supports the +following functions: encoder, decoder, capture, output.
> > > +
> > > +\subsection{Device ID}\label{sec:Device Types / Video Device / Device
> > > ID}
> > > +
> > > +TBD.
> >
> > I'm wondering how and when we can determine and reserve this ID?
>
> Grab the next free, update the spec accordingly, submit the one-line
> patch.
>
> > > +\begin{lstlisting}
> > > +enum virtio_video_pixel_format {
> > > + VIRTIO_VIDEO_PIX_FMT_UNDEFINED = 0,
> > > +
> > > + VIRTIO_VIDEO_PIX_FMT_H264 = 0x0100,
> > > + VIRTIO_VIDEO_PIX_FMT_NV12,
> > > + VIRTIO_VIDEO_PIX_FMT_NV21,
> > > + VIRTIO_VIDEO_PIX_FMT_I420,
> > > + VIRTIO_VIDEO_PIX_FMT_I422,
> > > + VIRTIO_VIDEO_PIX_FMT_XBGR,
> > > +};
> >
> > I'm wondering if we can use FOURCC instead. So, we can avoid reinventing a
> > mapping from formats to integers.
> > Also, I suppose the word "pixel formats" means only raw (decoded) formats.
> > But, it can be encoded format like H.264. So, I guess "image format" or
> > "fourcc" is a better word choice.
>
> Use separate pixel_format (fourcc) and stream_format (H.264 etc.) enums?
>
> > > +\begin{lstlisting}
> > > +struct virtio_video_function {
> > > + struct virtio_video_desc desc;
> > > + __le32 function_type; /* One of VIRTIO_VIDEO_FUNC_* types */
> > > + __le32 function_id;
> > > + struct virtio_video_params in_params;
> > > + struct virtio_video_params out_params;
> > > + __le32 num_caps;
> > > + __u8 padding[4];
> > > + /* Followed by struct virtio_video_capability video_caps[]; */
> > > +};
> > > +\end{lstlisting}
> >
> > If one device only has one functionality, virtio_video_function's fields
> > will be no longer needed except in_params and out_params. So, we'd be
> > able to remove virtio_video_function and have in_params and out_params in
> > virtio_video_capability instead.
>
> Same goes for per-function virtqueues (used virtqueue implies function).
>
> > > +\begin{lstlisting}
> > > +struct virtio_video_resource_detach_backing {
> > > + struct virtio_video_ctrl_hdr hdr;
> > > + __le32 resource_id;
> > > + __u8 padding[4];
> > > +};
> > > +\end{lstlisting}
> > > +
> > > +\begin{description}
> > > +\item[\field{resource_id}] internal id of the resource.
> > > +\end{description}
> >
> > I suppose that it'd be better not to have the above series of T_RESOURCE
> > controls at least until we reach a conclusion in the thread of
> > buffer-sharing device. If we end up concluding this type of controls is
> > the best way, we'll be able to revisit here.
>
> Well. For buffer management there are a bunch of options.
>
> (1) Simply stick the buffers
Re: [virtio-dev] [RFC RESEND] virtio-video: Add virtio video device specification
On Sat, Dec 7, 2019 at 12:50 AM Enrico Granata wrote:
>
> On Fri, Dec 6, 2019 at 4:30 AM Keiichi Watanabe wrote:
> >
> > Hi,
> >
> > On Fri, Dec 6, 2019 at 4:32 PM Gerd Hoffmann wrote:
> > >
> > > Hi,
> > >
> > > > > > > > +\subsection{Device ID}\label{sec:Device Types / Video Device /
> > > > > > > > Device ID}
> > > > > > > > +
> > > > > > > > +TBD.
> > > > > > >
> > > > > > > I'm wondering how and when we can determine and reserve this ID?
> > > > > >
> > > > > > Grab the next free, update the spec accordingly, submit the one-line
> > > > > > patch.
> > > >
> > > > Thanks. I will do so at the next version of the patch.
> > >
> > > No. Submit as separate one-liner patch which does nothing but grabbing
> > > an ID.
> >
> > Thanks. I sent https://markmail.org/message/jpjekiq7c4gkp6jt
> >
> > >
> > > > > > > I'm wondering if we can use FOURCC instead. So, we can avoid
> > > > > > > reinventing a
> > > > > > > mapping from formats to integers.
> > > > > > > Also, I suppose the word "pixel formats" means only raw (decoded)
> > > > > > > formats.
> > > > > > > But, it can be encoded format like H.264. So, I guess "image
> > > > > > > format" or
> > > > > > > "fourcc" is a better word choice.
> > > > > >
> > > > > > Use separate pixel_format (fourcc) and stream_format (H.264 etc.)
> > > > > > enums?
> > > > > >
> > > >
> > > > Fourcc is used for both raw and coded formats.
> > > > I'm not sure if it makes much sense to define different enums for raw
> > > > and coded formats, as
> > > > we reuse any other structs for both types of formats.
> > > >
> > > > What I'd suggest is like this:
> > > >
> > > > #define FOURCC(a,b,c,d) (a | (b << 8) | (c << 16) | (d << 24))
> > > >
> > > > enum virtio_video_fourcc {
> > > > VIRTIO_VIDEO_FOURCC_UNDEFINED = 0,
> > > >
> > > > /* Coded formats */
> > > > VIRTIO_VIDEO_FOURCC_H264 = FOURCC('H', '2', '6', '4'),
> > > > ...
> > > >
> > > > /* Raw formats */
> > > > VIRTIO_VIDEO_FOURCC_NV12 = FOURCC('N', 'V', '1', '2'),
> > > > ...
> > > > }
> > >
> > > Ok, that'll work.
> > >
> > > I've linked fourcc to drm fourcc codes in my head, and drm hasn't codes
> > > for the compressed formats.
> > >
> > > When defining things this way we should of course make sure that the raw
> > > format codes are identical to the ones drm uses.
> > >
> >
> > Yes. For Linux, though we have different constants for drm foucc and
> > video fourcc,
> > actual values are identical.
> > (e.g. DRM_FORMAT_NV12 and V4L2_PIX_FMT_NV12)
> > Then, we will have one more constant representing the same format for
> > virtio.
> >
> > > Is there a formal standard for these codes btw?
> >
> > RFC2361 defines FOURCCs for several formats, but it's not a formal
> > standard iiuc.
> > https://tools.ietf.org/html/rfc2361
> >
> > >
> > > > > As an interim solution, this form of "manual resource backing-store
> > > > > management" could be specified as a feature flag.
> > > > > Once there is an established solution for buffer sharing, we would
> > > > > then go ahead and introduce a new feature flag for "works with the
> > > > > buffer sharing mechanism", as an alternative to this manual mechanism.
> > > > >
> > > > > wdyt?
> > > >
> > > > It'd be a good idea to change buffer management method by a feature
> > > > flag.
> > >
> > > I don't think so. A device might want support multiple kinds of buffer
> > > management, most notably both their own buffers and imported buffers.
> > > Indicating which methods are available can be done with feature flags,
> > > but actually picking one not.
> > >
> >
> > Ah, you're right. Then, we might want to extend SET_PARAM or add a new
> > control
> > to specify a way of buffer management.
> >
>
> I think we need both. Feature flag(s) would give out the list of
> supported mechanism for a given device and driver.
> Out of the intersection, a new control can be used in order to pick
> the one actually being used for a given transaction.
Ah, I meant that we need to extend a control in addition to a feature
flag. Sorry for the
unclear reply.
On second thought, I wonder if we can set a type of buffer management methods in
STREAM_CREATE by adding fields like this:
enum virtio_video_buffer_type {
VIRTIO_VIDEO_BUF_TYPE_ALLOCATE,
/* VIRTIO_VIDEO_BUF_TYPE_IMPORT will be added */
};
struct virtio_video_stream_create {
struct virtio_video_ctrl_hdr hdr;
le32 in_buf_type; /* One of VIRTIO_VIDEO_BUF_TYPE_* */
le32 out_buf_type;
char debug_name[64];
};
wdyt?
-Keiichi
>
> > > > > > Well. For buffer management there are a bunch of options.
> > > > > >
> > > > > > (1) Simply stick the buffers (well, pointers to the buffer pages)
> > > > > > into
> > > > > > the virtqueue. This is the standard virtio way.
> > > > > >
> > > > > > (2) Create resources, then put the resource ids into the virtqueue.
> > > > > > virtio-gpu uses that model. First, because virtio-gpu needs
> > > > > > an id
> > > > > >
Re: [virtio-dev] [RFC RESEND] virtio-video: Add virtio video device specification
Hi, On Mon, Dec 9, 2019 at 8:38 PM Dmitry Sepp wrote: > > Hi, > > On Montag, 9. Dezember 2019 11:46:15 CET Gerd Hoffmann wrote: > > Hi, > > > > > > For (1) you'll simply do a QUEUE_BUFFER. The command carries references > > > > to the buffer pages. No resource management needed. > > > > > > > > For (2) you'll have RESOURCE_CREATE + RESOURCE_DESTROY + QUEUE_RESOURCE, > > > > where RESOURCE_CREATE passes the scatter list of buffer pages to the > > > > host and QUEUE_RESOURCE will carry just the resource id. > > > > > > > > For (3) you'll have RESOURCE_IMPORT + RESOURCE_DESTROY + QUEUE_RESOURCE. > > > > > > Thanks for the clarification. > > > On second thought, however, I'm wondering if we should keep all > > > RESOURCE controls. > > > This should be an option (4). > > > > Well, that's actually (2), aka "we use RESOURCE_* commands to manage > > resources". With the commands in the latest draft that would be: > > > > (1) RESOURCE_CREATE > > (2) RESOURCE_ATTACH_BACKING > > (3) RESOURCE_QUEUE (resource can be used multiple times here) > > (4) RESOURCE_DETACH_BACKING > > (5) RESOURCE_DESTROY > > > > I'm not sure we need the separate *_BACKING commands. I think we could > > also have RESOURCE_CREATE pass the buffer pages scatter list instead. > > > > Why it is done this way? Just because the design was copied from > > virtio-gpu? Or is there some specific reason? > > Yes, the design was just derived from virtio-gpu at early stages. > > I do agree we should merge the two steps. Thanks for the explanation. Merging them sounds good to me. -Keiichi > > > > > cheers, > > Gerd > > - To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
Re: [virtio-dev] [RFC RESEND] virtio-video: Add virtio video device specification
Hi, On Montag, 9. Dezember 2019 11:46:15 CET Gerd Hoffmann wrote: > Hi, > > > > For (1) you'll simply do a QUEUE_BUFFER. The command carries references > > > to the buffer pages. No resource management needed. > > > > > > For (2) you'll have RESOURCE_CREATE + RESOURCE_DESTROY + QUEUE_RESOURCE, > > > where RESOURCE_CREATE passes the scatter list of buffer pages to the > > > host and QUEUE_RESOURCE will carry just the resource id. > > > > > > For (3) you'll have RESOURCE_IMPORT + RESOURCE_DESTROY + QUEUE_RESOURCE. > > > > Thanks for the clarification. > > On second thought, however, I'm wondering if we should keep all > > RESOURCE controls. > > This should be an option (4). > > Well, that's actually (2), aka "we use RESOURCE_* commands to manage > resources". With the commands in the latest draft that would be: > > (1) RESOURCE_CREATE > (2) RESOURCE_ATTACH_BACKING > (3) RESOURCE_QUEUE (resource can be used multiple times here) > (4) RESOURCE_DETACH_BACKING > (5) RESOURCE_DESTROY > > I'm not sure we need the separate *_BACKING commands. I think we could > also have RESOURCE_CREATE pass the buffer pages scatter list instead. > > Why it is done this way? Just because the design was copied from > virtio-gpu? Or is there some specific reason? Yes, the design was just derived from virtio-gpu at early stages. I do agree we should merge the two steps. > > cheers, > Gerd - To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
Re: [virtio-dev] [RFC RESEND] virtio-video: Add virtio video device specification
Hi, > > For (1) you'll simply do a QUEUE_BUFFER. The command carries references > > to the buffer pages. No resource management needed. > > > > For (2) you'll have RESOURCE_CREATE + RESOURCE_DESTROY + QUEUE_RESOURCE, > > where RESOURCE_CREATE passes the scatter list of buffer pages to the > > host and QUEUE_RESOURCE will carry just the resource id. > > > > For (3) you'll have RESOURCE_IMPORT + RESOURCE_DESTROY + QUEUE_RESOURCE. > > > > Thanks for the clarification. > On second thought, however, I'm wondering if we should keep all > RESOURCE controls. > This should be an option (4). Well, that's actually (2), aka "we use RESOURCE_* commands to manage resources". With the commands in the latest draft that would be: (1) RESOURCE_CREATE (2) RESOURCE_ATTACH_BACKING (3) RESOURCE_QUEUE (resource can be used multiple times here) (4) RESOURCE_DETACH_BACKING (5) RESOURCE_DESTROY I'm not sure we need the separate *_BACKING commands. I think we could also have RESOURCE_CREATE pass the buffer pages scatter list instead. Why it is done this way? Just because the design was copied from virtio-gpu? Or is there some specific reason? cheers, Gerd - To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
Re: [virtio-dev] [RFC RESEND] virtio-video: Add virtio video device specification
On Fri, Dec 6, 2019 at 4:30 AM Keiichi Watanabe wrote:
>
> Hi,
>
> On Fri, Dec 6, 2019 at 4:32 PM Gerd Hoffmann wrote:
> >
> > Hi,
> >
> > > > > > > +\subsection{Device ID}\label{sec:Device Types / Video Device /
> > > > > > > Device ID}
> > > > > > > +
> > > > > > > +TBD.
> > > > > >
> > > > > > I'm wondering how and when we can determine and reserve this ID?
> > > > >
> > > > > Grab the next free, update the spec accordingly, submit the one-line
> > > > > patch.
> > >
> > > Thanks. I will do so at the next version of the patch.
> >
> > No. Submit as separate one-liner patch which does nothing but grabbing
> > an ID.
>
> Thanks. I sent https://markmail.org/message/jpjekiq7c4gkp6jt
>
> >
> > > > > > I'm wondering if we can use FOURCC instead. So, we can avoid
> > > > > > reinventing a
> > > > > > mapping from formats to integers.
> > > > > > Also, I suppose the word "pixel formats" means only raw (decoded)
> > > > > > formats.
> > > > > > But, it can be encoded format like H.264. So, I guess "image
> > > > > > format" or
> > > > > > "fourcc" is a better word choice.
> > > > >
> > > > > Use separate pixel_format (fourcc) and stream_format (H.264 etc.)
> > > > > enums?
> > > > >
> > >
> > > Fourcc is used for both raw and coded formats.
> > > I'm not sure if it makes much sense to define different enums for raw
> > > and coded formats, as
> > > we reuse any other structs for both types of formats.
> > >
> > > What I'd suggest is like this:
> > >
> > > #define FOURCC(a,b,c,d) (a | (b << 8) | (c << 16) | (d << 24))
> > >
> > > enum virtio_video_fourcc {
> > > VIRTIO_VIDEO_FOURCC_UNDEFINED = 0,
> > >
> > > /* Coded formats */
> > > VIRTIO_VIDEO_FOURCC_H264 = FOURCC('H', '2', '6', '4'),
> > > ...
> > >
> > > /* Raw formats */
> > > VIRTIO_VIDEO_FOURCC_NV12 = FOURCC('N', 'V', '1', '2'),
> > > ...
> > > }
> >
> > Ok, that'll work.
> >
> > I've linked fourcc to drm fourcc codes in my head, and drm hasn't codes
> > for the compressed formats.
> >
> > When defining things this way we should of course make sure that the raw
> > format codes are identical to the ones drm uses.
> >
>
> Yes. For Linux, though we have different constants for drm foucc and
> video fourcc,
> actual values are identical.
> (e.g. DRM_FORMAT_NV12 and V4L2_PIX_FMT_NV12)
> Then, we will have one more constant representing the same format for virtio.
>
> > Is there a formal standard for these codes btw?
>
> RFC2361 defines FOURCCs for several formats, but it's not a formal
> standard iiuc.
> https://tools.ietf.org/html/rfc2361
>
> >
> > > > As an interim solution, this form of "manual resource backing-store
> > > > management" could be specified as a feature flag.
> > > > Once there is an established solution for buffer sharing, we would
> > > > then go ahead and introduce a new feature flag for "works with the
> > > > buffer sharing mechanism", as an alternative to this manual mechanism.
> > > >
> > > > wdyt?
> > >
> > > It'd be a good idea to change buffer management method by a feature flag.
> >
> > I don't think so. A device might want support multiple kinds of buffer
> > management, most notably both their own buffers and imported buffers.
> > Indicating which methods are available can be done with feature flags,
> > but actually picking one not.
> >
>
> Ah, you're right. Then, we might want to extend SET_PARAM or add a new control
> to specify a way of buffer management.
>
I think we need both. Feature flag(s) would give out the list of
supported mechanism for a given device and driver.
Out of the intersection, a new control can be used in order to pick
the one actually being used for a given transaction.
> > > > > Well. For buffer management there are a bunch of options.
> > > > >
> > > > > (1) Simply stick the buffers (well, pointers to the buffer pages)
> > > > > into
> > > > > the virtqueue. This is the standard virtio way.
> > > > >
> > > > > (2) Create resources, then put the resource ids into the virtqueue.
> > > > > virtio-gpu uses that model. First, because virtio-gpu needs an
> > > > > id
> > > > > to reference resources in the rendering command stream
> > > > > (virtio-video doesn't need this). Also because (some kinds of)
> > > > > resources are around for a long time and the guest-physical ->
> > > > > host-virtual mapping needs to be done only once that way (which
> > > > > I think would be the case for virtio-video too because v4l2
> > > > > re-uses buffers in robin-round fashion). Drawback is this
> > > > > assumes shared memory between host and guest (which is the case
> > > > > in typical use cases but it is not mandated by the virtio spec).
> > > > >
> > > > > (3) Import external resources (from virtio-gpu for example).
> > > > > Out of scope for now, will probably added as optional feature
> > > > > later.
> > > > >
> > > > > I guess long-term we want support either (1)+(3) or (2)+(3).
> > > > >
> > >
> > > In t
Re: [virtio-dev] [RFC RESEND] virtio-video: Add virtio video device specification
Hi,
On Fri, Dec 6, 2019 at 4:32 PM Gerd Hoffmann wrote:
>
> Hi,
>
> > > > > > +\subsection{Device ID}\label{sec:Device Types / Video Device /
> > > > > > Device ID}
> > > > > > +
> > > > > > +TBD.
> > > > >
> > > > > I'm wondering how and when we can determine and reserve this ID?
> > > >
> > > > Grab the next free, update the spec accordingly, submit the one-line
> > > > patch.
> >
> > Thanks. I will do so at the next version of the patch.
>
> No. Submit as separate one-liner patch which does nothing but grabbing
> an ID.
Thanks. I sent https://markmail.org/message/jpjekiq7c4gkp6jt
>
> > > > > I'm wondering if we can use FOURCC instead. So, we can avoid
> > > > > reinventing a
> > > > > mapping from formats to integers.
> > > > > Also, I suppose the word "pixel formats" means only raw (decoded)
> > > > > formats.
> > > > > But, it can be encoded format like H.264. So, I guess "image format"
> > > > > or
> > > > > "fourcc" is a better word choice.
> > > >
> > > > Use separate pixel_format (fourcc) and stream_format (H.264 etc.) enums?
> > > >
> >
> > Fourcc is used for both raw and coded formats.
> > I'm not sure if it makes much sense to define different enums for raw
> > and coded formats, as
> > we reuse any other structs for both types of formats.
> >
> > What I'd suggest is like this:
> >
> > #define FOURCC(a,b,c,d) (a | (b << 8) | (c << 16) | (d << 24))
> >
> > enum virtio_video_fourcc {
> > VIRTIO_VIDEO_FOURCC_UNDEFINED = 0,
> >
> > /* Coded formats */
> > VIRTIO_VIDEO_FOURCC_H264 = FOURCC('H', '2', '6', '4'),
> > ...
> >
> > /* Raw formats */
> > VIRTIO_VIDEO_FOURCC_NV12 = FOURCC('N', 'V', '1', '2'),
> > ...
> > }
>
> Ok, that'll work.
>
> I've linked fourcc to drm fourcc codes in my head, and drm hasn't codes
> for the compressed formats.
>
> When defining things this way we should of course make sure that the raw
> format codes are identical to the ones drm uses.
>
Yes. For Linux, though we have different constants for drm foucc and
video fourcc,
actual values are identical.
(e.g. DRM_FORMAT_NV12 and V4L2_PIX_FMT_NV12)
Then, we will have one more constant representing the same format for virtio.
> Is there a formal standard for these codes btw?
RFC2361 defines FOURCCs for several formats, but it's not a formal
standard iiuc.
https://tools.ietf.org/html/rfc2361
>
> > > As an interim solution, this form of "manual resource backing-store
> > > management" could be specified as a feature flag.
> > > Once there is an established solution for buffer sharing, we would
> > > then go ahead and introduce a new feature flag for "works with the
> > > buffer sharing mechanism", as an alternative to this manual mechanism.
> > >
> > > wdyt?
> >
> > It'd be a good idea to change buffer management method by a feature flag.
>
> I don't think so. A device might want support multiple kinds of buffer
> management, most notably both their own buffers and imported buffers.
> Indicating which methods are available can be done with feature flags,
> but actually picking one not.
>
Ah, you're right. Then, we might want to extend SET_PARAM or add a new control
to specify a way of buffer management.
> > > > Well. For buffer management there are a bunch of options.
> > > >
> > > > (1) Simply stick the buffers (well, pointers to the buffer pages) into
> > > > the virtqueue. This is the standard virtio way.
> > > >
> > > > (2) Create resources, then put the resource ids into the virtqueue.
> > > > virtio-gpu uses that model. First, because virtio-gpu needs an id
> > > > to reference resources in the rendering command stream
> > > > (virtio-video doesn't need this). Also because (some kinds of)
> > > > resources are around for a long time and the guest-physical ->
> > > > host-virtual mapping needs to be done only once that way (which
> > > > I think would be the case for virtio-video too because v4l2
> > > > re-uses buffers in robin-round fashion). Drawback is this
> > > > assumes shared memory between host and guest (which is the case
> > > > in typical use cases but it is not mandated by the virtio spec).
> > > >
> > > > (3) Import external resources (from virtio-gpu for example).
> > > > Out of scope for now, will probably added as optional feature
> > > > later.
> > > >
> > > > I guess long-term we want support either (1)+(3) or (2)+(3).
> > > >
> >
> > In the first version of spec, we might want to support the minimal workable
> > set
> > of controls. Then, we'll be able to add additional controls with a new
> > feature
> > flag as Enrico suggested.
> >
> > So, the problem is what's the simplest scenario and which types of controls
> > are
> > required there. I guess it's enough for (1) and (2) if we have
> > T_RESOURCE_CREATE
> > and T_RESOURCE_DESTROY.
>
> For (1) you'll simply do a QUEUE_BUFFER. The command carries references
> to the buffer pages. No resource management needed.
>
> For (2) you'll have
Re: [virtio-dev] [RFC RESEND] virtio-video: Add virtio video device specification
Hi,
> > > > > +\subsection{Device ID}\label{sec:Device Types / Video Device /
> > > > > Device ID}
> > > > > +
> > > > > +TBD.
> > > >
> > > > I'm wondering how and when we can determine and reserve this ID?
> > >
> > > Grab the next free, update the spec accordingly, submit the one-line
> > > patch.
>
> Thanks. I will do so at the next version of the patch.
No. Submit as separate one-liner patch which does nothing but grabbing
an ID.
> > > > I'm wondering if we can use FOURCC instead. So, we can avoid
> > > > reinventing a
> > > > mapping from formats to integers.
> > > > Also, I suppose the word "pixel formats" means only raw (decoded)
> > > > formats.
> > > > But, it can be encoded format like H.264. So, I guess "image format" or
> > > > "fourcc" is a better word choice.
> > >
> > > Use separate pixel_format (fourcc) and stream_format (H.264 etc.) enums?
> > >
>
> Fourcc is used for both raw and coded formats.
> I'm not sure if it makes much sense to define different enums for raw
> and coded formats, as
> we reuse any other structs for both types of formats.
>
> What I'd suggest is like this:
>
> #define FOURCC(a,b,c,d) (a | (b << 8) | (c << 16) | (d << 24))
>
> enum virtio_video_fourcc {
> VIRTIO_VIDEO_FOURCC_UNDEFINED = 0,
>
> /* Coded formats */
> VIRTIO_VIDEO_FOURCC_H264 = FOURCC('H', '2', '6', '4'),
> ...
>
> /* Raw formats */
> VIRTIO_VIDEO_FOURCC_NV12 = FOURCC('N', 'V', '1', '2'),
> ...
> }
Ok, that'll work.
I've linked fourcc to drm fourcc codes in my head, and drm hasn't codes
for the compressed formats.
When defining things this way we should of course make sure that the raw
format codes are identical to the ones drm uses.
Is there a formal standard for these codes btw?
> > As an interim solution, this form of "manual resource backing-store
> > management" could be specified as a feature flag.
> > Once there is an established solution for buffer sharing, we would
> > then go ahead and introduce a new feature flag for "works with the
> > buffer sharing mechanism", as an alternative to this manual mechanism.
> >
> > wdyt?
>
> It'd be a good idea to change buffer management method by a feature flag.
I don't think so. A device might want support multiple kinds of buffer
management, most notably both their own buffers and imported buffers.
Indicating which methods are available can be done with feature flags,
but actually picking one not.
> > > Well. For buffer management there are a bunch of options.
> > >
> > > (1) Simply stick the buffers (well, pointers to the buffer pages) into
> > > the virtqueue. This is the standard virtio way.
> > >
> > > (2) Create resources, then put the resource ids into the virtqueue.
> > > virtio-gpu uses that model. First, because virtio-gpu needs an id
> > > to reference resources in the rendering command stream
> > > (virtio-video doesn't need this). Also because (some kinds of)
> > > resources are around for a long time and the guest-physical ->
> > > host-virtual mapping needs to be done only once that way (which
> > > I think would be the case for virtio-video too because v4l2
> > > re-uses buffers in robin-round fashion). Drawback is this
> > > assumes shared memory between host and guest (which is the case
> > > in typical use cases but it is not mandated by the virtio spec).
> > >
> > > (3) Import external resources (from virtio-gpu for example).
> > > Out of scope for now, will probably added as optional feature
> > > later.
> > >
> > > I guess long-term we want support either (1)+(3) or (2)+(3).
> > >
>
> In the first version of spec, we might want to support the minimal workable
> set
> of controls. Then, we'll be able to add additional controls with a new feature
> flag as Enrico suggested.
>
> So, the problem is what's the simplest scenario and which types of controls
> are
> required there. I guess it's enough for (1) and (2) if we have
> T_RESOURCE_CREATE
> and T_RESOURCE_DESTROY.
For (1) you'll simply do a QUEUE_BUFFER. The command carries references
to the buffer pages. No resource management needed.
For (2) you'll have RESOURCE_CREATE + RESOURCE_DESTROY + QUEUE_RESOURCE,
where RESOURCE_CREATE passes the scatter list of buffer pages to the
host and QUEUE_RESOURCE will carry just the resource id.
For (3) you'll have RESOURCE_IMPORT + RESOURCE_DESTROY + QUEUE_RESOURCE.
cheers,
Gerd
-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
Re: [virtio-dev] [RFC RESEND] virtio-video: Add virtio video device specification
Hi,
On Thu, Dec 5, 2019 at 4:12 AM Enrico Granata wrote:
>
> On Wed, Dec 4, 2019 at 1:16 AM Gerd Hoffmann wrote:
> >
> > Hi,
> >
> > > 1. Focus on only decoder/encoder functionalities first.
> > >
> > > As Tomasz said earlier in this thread, it'd be too complicated to support
> > > camera
> > > usage at the same time. So, I'd suggest to make it just a generic
> > > mem-to-mem
> > > video processing device protocol for now.
> > > If we finally decide to support camera in this protocol, we can add it
> > > later.
> >
> > Agree.
> >
> > > 2. Only one feature bit can be specified for one device.
> > >
> > > I'd like to have a decoder device and encoder device separately.
> > > It'd be natural to assume it because a decoder and an encoder are
> > > provided as
> > > different hardware.
> >
> > Hmm, modern GPUs support both encoding and decoding ...
> >
> > I don't think we should bake that restriction into the specification.
> > It probably makes sense to use one virtqueue per function though, that
> > will simplify dispatching in both host and guest.
> >
It'd be a better idea to have a set of virtqueues for each function.
So, we will have 2 * (# of features provided) virtqueues, as one
function requires controlq
and eventq.
> > > 3. Separate buffer allocation functionalities from virtio-video protocol.
> > >
> > > To support various ways of guest/host buffer sharing, we might want to
> > > have a
> > > dedicated buffer sharing device as we're discussing in another thread.
> > > Until we
> > > reach consensus there, it'd be good not to have buffer allocation
> > > functionalities in virtio-video.
> >
> > I think virtio-video should be able to work as stand-alone device,
> > so we need some way to allocate buffers ...
> >
> > Buffer sharing with other devices can be added later.
Good point. Then, we should have T_RESOURCE_CREATE and T_RESOURCE_DESTROY
at least.
> >
> > > > +The virtio video device is a virtual video streaming device that
> > > > supports the
> > > > +following functions: encoder, decoder, capture, output.
> > > > +
> > > > +\subsection{Device ID}\label{sec:Device Types / Video Device / Device
> > > > ID}
> > > > +
> > > > +TBD.
> > >
> > > I'm wondering how and when we can determine and reserve this ID?
> >
> > Grab the next free, update the spec accordingly, submit the one-line
> > patch.
Thanks. I will do so at the next version of the patch.
> >
> > > > +\begin{lstlisting}
> > > > +enum virtio_video_pixel_format {
> > > > + VIRTIO_VIDEO_PIX_FMT_UNDEFINED = 0,
> > > > +
> > > > + VIRTIO_VIDEO_PIX_FMT_H264 = 0x0100,
> > > > + VIRTIO_VIDEO_PIX_FMT_NV12,
> > > > + VIRTIO_VIDEO_PIX_FMT_NV21,
> > > > + VIRTIO_VIDEO_PIX_FMT_I420,
> > > > + VIRTIO_VIDEO_PIX_FMT_I422,
> > > > + VIRTIO_VIDEO_PIX_FMT_XBGR,
> > > > +};
> > >
> > > I'm wondering if we can use FOURCC instead. So, we can avoid reinventing a
> > > mapping from formats to integers.
> > > Also, I suppose the word "pixel formats" means only raw (decoded) formats.
> > > But, it can be encoded format like H.264. So, I guess "image format" or
> > > "fourcc" is a better word choice.
> >
> > Use separate pixel_format (fourcc) and stream_format (H.264 etc.) enums?
> >
Fourcc is used for both raw and coded formats.
I'm not sure if it makes much sense to define different enums for raw
and coded formats, as
we reuse any other structs for both types of formats.
What I'd suggest is like this:
#define FOURCC(a,b,c,d) (a | (b << 8) | (c << 16) | (d << 24))
enum virtio_video_fourcc {
VIRTIO_VIDEO_FOURCC_UNDEFINED = 0,
/* Coded formats */
VIRTIO_VIDEO_FOURCC_H264 = FOURCC('H', '2', '6', '4'),
...
/* Raw formats */
VIRTIO_VIDEO_FOURCC_NV12 = FOURCC('N', 'V', '1', '2'),
...
}
> > > > +\begin{lstlisting}
> > > > +struct virtio_video_function {
> > > > + struct virtio_video_desc desc;
> > > > + __le32 function_type; /* One of VIRTIO_VIDEO_FUNC_* types */
> > > > + __le32 function_id;
> > > > + struct virtio_video_params in_params;
> > > > + struct virtio_video_params out_params;
> > > > + __le32 num_caps;
> > > > + __u8 padding[4];
> > > > + /* Followed by struct virtio_video_capability video_caps[]; */
> > > > +};
> > > > +\end{lstlisting}
> > >
> > > If one device only has one functionality, virtio_video_function's fields
> > > will be
> > > no longer needed except in_params and out_params. So, we'd be able to
> > > remove
> > > virtio_video_function and have in_params and out_params in
> > > virtio_video_capability instead.
> >
> > Same goes for per-function virtqueues (used virtqueue implies function).
> >
> > > > +\begin{lstlisting}
> > > > +struct virtio_video_resource_detach_backing {
> > > > + struct virtio_video_ctrl_hdr hdr;
> > > > + __le32 resource_id;
> > > > + __u8 padding[4];
> > > > +};
> > > > +\end{lstlisting}
> > > > +
> > > > +\begin{description}
> > > > +\item[\field{resource_id}] i
Re: [virtio-dev] [RFC RESEND] virtio-video: Add virtio video device specification
On Wed, Dec 4, 2019 at 1:16 AM Gerd Hoffmann wrote:
>
> Hi,
>
> > 1. Focus on only decoder/encoder functionalities first.
> >
> > As Tomasz said earlier in this thread, it'd be too complicated to support
> > camera
> > usage at the same time. So, I'd suggest to make it just a generic mem-to-mem
> > video processing device protocol for now.
> > If we finally decide to support camera in this protocol, we can add it
> > later.
>
> Agree.
>
> > 2. Only one feature bit can be specified for one device.
> >
> > I'd like to have a decoder device and encoder device separately.
> > It'd be natural to assume it because a decoder and an encoder are provided
> > as
> > different hardware.
>
> Hmm, modern GPUs support both encoding and decoding ...
>
> I don't think we should bake that restriction into the specification.
> It probably makes sense to use one virtqueue per function though, that
> will simplify dispatching in both host and guest.
>
> > 3. Separate buffer allocation functionalities from virtio-video protocol.
> >
> > To support various ways of guest/host buffer sharing, we might want to have
> > a
> > dedicated buffer sharing device as we're discussing in another thread.
> > Until we
> > reach consensus there, it'd be good not to have buffer allocation
> > functionalities in virtio-video.
>
> I think virtio-video should be able to work as stand-alone device,
> so we need some way to allocate buffers ...
>
> Buffer sharing with other devices can be added later.
>
> > > +The virtio video device is a virtual video streaming device that
> > > supports the
> > > +following functions: encoder, decoder, capture, output.
> > > +
> > > +\subsection{Device ID}\label{sec:Device Types / Video Device / Device ID}
> > > +
> > > +TBD.
> >
> > I'm wondering how and when we can determine and reserve this ID?
>
> Grab the next free, update the spec accordingly, submit the one-line
> patch.
>
> > > +\begin{lstlisting}
> > > +enum virtio_video_pixel_format {
> > > + VIRTIO_VIDEO_PIX_FMT_UNDEFINED = 0,
> > > +
> > > + VIRTIO_VIDEO_PIX_FMT_H264 = 0x0100,
> > > + VIRTIO_VIDEO_PIX_FMT_NV12,
> > > + VIRTIO_VIDEO_PIX_FMT_NV21,
> > > + VIRTIO_VIDEO_PIX_FMT_I420,
> > > + VIRTIO_VIDEO_PIX_FMT_I422,
> > > + VIRTIO_VIDEO_PIX_FMT_XBGR,
> > > +};
> >
> > I'm wondering if we can use FOURCC instead. So, we can avoid reinventing a
> > mapping from formats to integers.
> > Also, I suppose the word "pixel formats" means only raw (decoded) formats.
> > But, it can be encoded format like H.264. So, I guess "image format" or
> > "fourcc" is a better word choice.
>
> Use separate pixel_format (fourcc) and stream_format (H.264 etc.) enums?
>
> > > +\begin{lstlisting}
> > > +struct virtio_video_function {
> > > + struct virtio_video_desc desc;
> > > + __le32 function_type; /* One of VIRTIO_VIDEO_FUNC_* types */
> > > + __le32 function_id;
> > > + struct virtio_video_params in_params;
> > > + struct virtio_video_params out_params;
> > > + __le32 num_caps;
> > > + __u8 padding[4];
> > > + /* Followed by struct virtio_video_capability video_caps[]; */
> > > +};
> > > +\end{lstlisting}
> >
> > If one device only has one functionality, virtio_video_function's fields
> > will be
> > no longer needed except in_params and out_params. So, we'd be able to remove
> > virtio_video_function and have in_params and out_params in
> > virtio_video_capability instead.
>
> Same goes for per-function virtqueues (used virtqueue implies function).
>
> > > +\begin{lstlisting}
> > > +struct virtio_video_resource_detach_backing {
> > > + struct virtio_video_ctrl_hdr hdr;
> > > + __le32 resource_id;
> > > + __u8 padding[4];
> > > +};
> > > +\end{lstlisting}
> > > +
> > > +\begin{description}
> > > +\item[\field{resource_id}] internal id of the resource.
> > > +\end{description}
> >
> > I suppose that it'd be better not to have the above series of T_RESOURCE
> > controls at least until we reach a conclusion in the thread of
> > buffer-sharing
> > device. If we end up concluding this type of controls is the best way,
> > we'll be
> > able to revisit here.
>
As an interim solution, this form of "manual resource backing-store
management" could be specified as a feature flag.
Once there is an established solution for buffer sharing, we would
then go ahead and introduce a new feature flag for "works with the
buffer sharing mechanism", as an alternative to this manual mechanism.
wdyt?
> Well. For buffer management there are a bunch of options.
>
> (1) Simply stick the buffers (well, pointers to the buffer pages) into
> the virtqueue. This is the standard virtio way.
>
> (2) Create resources, then put the resource ids into the virtqueue.
> virtio-gpu uses that model. First, because virtio-gpu needs an id
> to reference resources in the rendering command stream
> (virtio-video doesn't need this). Also because (some kinds of)
> resour
Re: [virtio-dev] [RFC RESEND] virtio-video: Add virtio video device specification
Hi,
> 1. Focus on only decoder/encoder functionalities first.
>
> As Tomasz said earlier in this thread, it'd be too complicated to support
> camera
> usage at the same time. So, I'd suggest to make it just a generic mem-to-mem
> video processing device protocol for now.
> If we finally decide to support camera in this protocol, we can add it later.
Agree.
> 2. Only one feature bit can be specified for one device.
>
> I'd like to have a decoder device and encoder device separately.
> It'd be natural to assume it because a decoder and an encoder are provided as
> different hardware.
Hmm, modern GPUs support both encoding and decoding ...
I don't think we should bake that restriction into the specification.
It probably makes sense to use one virtqueue per function though, that
will simplify dispatching in both host and guest.
> 3. Separate buffer allocation functionalities from virtio-video protocol.
>
> To support various ways of guest/host buffer sharing, we might want to have a
> dedicated buffer sharing device as we're discussing in another thread. Until
> we
> reach consensus there, it'd be good not to have buffer allocation
> functionalities in virtio-video.
I think virtio-video should be able to work as stand-alone device,
so we need some way to allocate buffers ...
Buffer sharing with other devices can be added later.
> > +The virtio video device is a virtual video streaming device that supports
> > the
> > +following functions: encoder, decoder, capture, output.
> > +
> > +\subsection{Device ID}\label{sec:Device Types / Video Device / Device ID}
> > +
> > +TBD.
>
> I'm wondering how and when we can determine and reserve this ID?
Grab the next free, update the spec accordingly, submit the one-line
patch.
> > +\begin{lstlisting}
> > +enum virtio_video_pixel_format {
> > + VIRTIO_VIDEO_PIX_FMT_UNDEFINED = 0,
> > +
> > + VIRTIO_VIDEO_PIX_FMT_H264 = 0x0100,
> > + VIRTIO_VIDEO_PIX_FMT_NV12,
> > + VIRTIO_VIDEO_PIX_FMT_NV21,
> > + VIRTIO_VIDEO_PIX_FMT_I420,
> > + VIRTIO_VIDEO_PIX_FMT_I422,
> > + VIRTIO_VIDEO_PIX_FMT_XBGR,
> > +};
>
> I'm wondering if we can use FOURCC instead. So, we can avoid reinventing a
> mapping from formats to integers.
> Also, I suppose the word "pixel formats" means only raw (decoded) formats.
> But, it can be encoded format like H.264. So, I guess "image format" or
> "fourcc" is a better word choice.
Use separate pixel_format (fourcc) and stream_format (H.264 etc.) enums?
> > +\begin{lstlisting}
> > +struct virtio_video_function {
> > + struct virtio_video_desc desc;
> > + __le32 function_type; /* One of VIRTIO_VIDEO_FUNC_* types */
> > + __le32 function_id;
> > + struct virtio_video_params in_params;
> > + struct virtio_video_params out_params;
> > + __le32 num_caps;
> > + __u8 padding[4];
> > + /* Followed by struct virtio_video_capability video_caps[]; */
> > +};
> > +\end{lstlisting}
>
> If one device only has one functionality, virtio_video_function's fields will
> be
> no longer needed except in_params and out_params. So, we'd be able to remove
> virtio_video_function and have in_params and out_params in
> virtio_video_capability instead.
Same goes for per-function virtqueues (used virtqueue implies function).
> > +\begin{lstlisting}
> > +struct virtio_video_resource_detach_backing {
> > + struct virtio_video_ctrl_hdr hdr;
> > + __le32 resource_id;
> > + __u8 padding[4];
> > +};
> > +\end{lstlisting}
> > +
> > +\begin{description}
> > +\item[\field{resource_id}] internal id of the resource.
> > +\end{description}
>
> I suppose that it'd be better not to have the above series of T_RESOURCE
> controls at least until we reach a conclusion in the thread of buffer-sharing
> device. If we end up concluding this type of controls is the best way, we'll
> be
> able to revisit here.
Well. For buffer management there are a bunch of options.
(1) Simply stick the buffers (well, pointers to the buffer pages) into
the virtqueue. This is the standard virtio way.
(2) Create resources, then put the resource ids into the virtqueue.
virtio-gpu uses that model. First, because virtio-gpu needs an id
to reference resources in the rendering command stream
(virtio-video doesn't need this). Also because (some kinds of)
resources are around for a long time and the guest-physical ->
host-virtual mapping needs to be done only once that way (which
I think would be the case for virtio-video too because v4l2
re-uses buffers in robin-round fashion). Drawback is this
assumes shared memory between host and guest (which is the case
in typical use cases but it is not mandated by the virtio spec).
(3) Import external resources (from virtio-gpu for example).
Out of scope for now, will probably added as optional feature
later.
I guess long-term we want support either (1)+(3) or (2)+(3).
cheers,
Gerd
Re: [virtio-dev] [RFC RESEND] virtio-video: Add virtio video device specification
Hi everyone,
After comparing this virtio-video and virtio-vdec we proposed in a different
thread, I suppose that it's good to use the virtio-video protocol as a basis for
discussion. Then, we can improve this protocol together to merge both
requirements and establish one protocol.
Here, let me share my ideas to improve virtio-video. If my direction looks
reasonable to you, I'm going to update this virtio-video RFC patch as v2 by
adding "Co-authored-by" line in its commit message.
(Or, if there is a better way for this kind of collaboration, please
let me know.)
--
First, I'd like to suggest the following three changes for the general design:
1. Focus on only decoder/encoder functionalities first.
As Tomasz said earlier in this thread, it'd be too complicated to support camera
usage at the same time. So, I'd suggest to make it just a generic mem-to-mem
video processing device protocol for now.
If we finally decide to support camera in this protocol, we can add it later.
2. Only one feature bit can be specified for one device.
I'd like to have a decoder device and encoder device separately.
It'd be natural to assume it because a decoder and an encoder are provided as
different hardware. Also, this assumption will make the protocol and
implementation simpler.
e.g. my inline comments about VIRTIO_VIDEO_T_GET_FUNCS and
virtio_video_function below
3. Separate buffer allocation functionalities from virtio-video protocol.
To support various ways of guest/host buffer sharing, we might want to have a
dedicated buffer sharing device as we're discussing in another thread. Until we
reach consensus there, it'd be good not to have buffer allocation
functionalities in virtio-video.
--
For each control, please see my inline comments for the protocol below.
(I pasted the original RFC content below, as it was lost during the
previous conversation.)
On Wed, Nov 6, 2019 at 1:23 AM Dmitry Sepp wrote:
>
> This patch proposes a virtio specification for a new virtio video
> device.
>
> The virtio video device is an abstract video streaming device that
> operates input and/or output data buffers to share video devices with
> several guests. The device uses descriptor structures to advertise and
> negotiate stream formats and controls. This allows the driver to modify
> the processing logic of the device on a per stream basis. The generic
> nature of the device makes it possible to support the following video
> functions: encoder, decoder, processor, capture, output.
>
> Thank you in advance for any feedback.
>
> Signed-off-by: Dmitry Sepp
> ---
> content.tex | 1 +
> virtio-video.tex | 776 +++
> 2 files changed, 777 insertions(+)
> create mode 100644 virtio-video.tex
>
> diff --git a/content.tex b/content.tex
> index b1ea9b9..6990b5d 100644
> --- a/content.tex
> +++ b/content.tex
> @@ -5698,6 +5698,7 @@ \subsubsection{Legacy Interface: Framing
> Requirements}\label{sec:Device
> \input{virtio-crypto.tex}
> \input{virtio-vsock.tex}
> \input{virtio-fs.tex}
> +\input{virtio-video.tex}
>
> \chapter{Reserved Feature Bits}\label{sec:Reserved Feature Bits}
>
> diff --git a/virtio-video.tex b/virtio-video.tex
> new file mode 100644
> index 000..84aade8
> --- /dev/null
> +++ b/virtio-video.tex
> @@ -0,0 +1,776 @@
> +\section{Video Device}\label{sec:Device Types / Video Device}
> +
> +The virtio video device is a virtual video streaming device that supports the
> +following functions: encoder, decoder, capture, output.
> +
> +\subsection{Device ID}\label{sec:Device Types / Video Device / Device ID}
> +
> +TBD.
I'm wondering how and when we can determine and reserve this ID?
> +
> +\subsection{Virtqueues}\label{sec:Device Types / Video Device / Virtqueues}
> +
> +\begin{description}
> +\item[0] controlq
> +\item[1] eventq
> +\end{description}
> +
> +\subsection{Feature bits}\label{sec:Device Types / Video Device / Feature
> bits}
> +
> +\begin{description}
> +\item[VIRTIO_VIDEO_F_ENCODER (0)] Video encoder function supported.
> +\item[VIRTIO_VIDEO_F_DECODER (1)] Video decoder function supported.
> +\item[VIRTIO_VIDEO_F_PROCESSOR (2)] Video processor function supported.
> +\item[VIRTIO_VIDEO_F_CAPTURE (3)] Video capture function supported.
> +\item[VIRTIO_VIDEO_F_OUTPUT (4)] Video output function supported.
> +\end{description}
As I suggested above, it'd be good to have only VIRTIO_VIDEO_F_DECODER and
VIRTIO_VIDEO_F_ENCODER for now and prohibit specifying both at the same time.
> +
> +\subsection{Supported functions}\label{sec:Device Types / Video Device /
> Supported video functions}
> +
> +The following video functions are defined:
> +
> +\begin{lstlisting}
> +enum virtio_video_func_type {
> + VIRTIO_VIDEO_FUNC_UNDEFINED = 0,
> +
> + VIRTIO_VIDEO_FUNC_ENCODER = 0x0100,
> + VIRTIO_VIDEO_FUNC_DECODER,
> + VIRTIO_VIDEO_FUNC_PROCESSOR,
> + VIRTIO_VIDEO_FUNC_CAPTURE,
> + VIRTIO_VIDEO_FUNC_OUTPUT,
> +};
> +\end{lstlisting}
> +
> +
Re: [virtio-dev] [RFC RESEND] virtio-video: Add virtio video device specification
Hi Gerd, On Mittwoch, 20. November 2019 12:29:29 CET Gerd Hoffmann wrote: > Hi, > > > > > > 3. No support for getting plane requirements from the device (sg vs > > > > > contig, > > > > > size, stride alignment, plane count). > > > > > > > > There is actually a bigger difference that results in that. Vdec > > > > assumes host-allocated buffers coming from a different device, e.g. > > > > virtio-gpu and the host having the right knowledge to allocate the > > > > buffers correctly. This is related to the fact that it's generally > > > > difficult to convey all the allocation constraints in a generic > > > > manner. > > > > > > Yep, buffer handling is tricky, especially when it comes to decoding > > > directly to gpu buffers and also when supporting playback of > > > drm-protected streams where the guest might not be allowed to access > > > the stream data. > > > > Also, if we decide to have a buffer sharing device as Gerd suggested > > in a different thread, > > we'll get less overlaps between video codec feature and camera feature. > > e.g. VIRTIO_VIDEO_T_RESOURCE_* would be simplified. (or removed?) > > Disclaimer: Havn't found the time yet to go over both virtio-video and > virtio-vdec in detail. > > > As Tomasz said, I think virtio-vdec can be modified to support > > encoding as well without big changes. I'm happy to update our > > protocol and driver implementation to support encoding if needed. > > I think it makes sense to have a rough plan first ;) > Is there a virtio-video implementation too? Yes, of course. We will be ready to share the code very soon. Regards, Dmitry. > > When it comes to buffer handling: I don't like that virtio-vdec depends > on virtio-gpu for buffer handling. Allowing sharing buffers between > virtio-vdec and virtio-gpu (and possibly other devices) makes sense as > an optional extension. But IMO the encoder/decoder device should be > able to operate as stand-alone device. > > cheers, > Gerd > > > - > To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org > For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org - To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
Re: [virtio-dev] [RFC RESEND] virtio-video: Add virtio video device specification
Hi, > > > > 3. No support for getting plane requirements from the device (sg vs > > > > contig, > > > > size, stride alignment, plane count). > > > > > > There is actually a bigger difference that results in that. Vdec > > > assumes host-allocated buffers coming from a different device, e.g. > > > virtio-gpu and the host having the right knowledge to allocate the > > > buffers correctly. This is related to the fact that it's generally > > > difficult to convey all the allocation constraints in a generic > > > manner. > > > > Yep, buffer handling is tricky, especially when it comes to decoding > > directly to gpu buffers and also when supporting playback of > > drm-protected streams where the guest might not be allowed to access > > the stream data. > Also, if we decide to have a buffer sharing device as Gerd suggested > in a different thread, > we'll get less overlaps between video codec feature and camera feature. > e.g. VIRTIO_VIDEO_T_RESOURCE_* would be simplified. (or removed?) Disclaimer: Havn't found the time yet to go over both virtio-video and virtio-vdec in detail. > As Tomasz said, I think virtio-vdec can be modified to support > encoding as well without big changes. I'm happy to update our > protocol and driver implementation to support encoding if needed. I think it makes sense to have a rough plan first ;) Is there a virtio-video implementation too? When it comes to buffer handling: I don't like that virtio-vdec depends on virtio-gpu for buffer handling. Allowing sharing buffers between virtio-vdec and virtio-gpu (and possibly other devices) makes sense as an optional extension. But IMO the encoder/decoder device should be able to operate as stand-alone device. cheers, Gerd - To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
Re: [virtio-dev] [RFC RESEND] virtio-video: Add virtio video device specification
Hello Gerd, Please see the answer below. On Freitag, 8. November 2019 08:49:55 CET Gerd Hoffmann wrote: > On Thu, Nov 07, 2019 at 02:09:34PM +0100, Dmitry Sepp wrote: > > Hello Gerd, > > > > Thank you for your feedback. > > > > There is no relationship between those. As I mentioned earlier. we have > > also been working on a virtio video device at the same time. And there is > > no relationship between the two specs. > > Keiichi, have you looked at the spec? > > I think it is useful to have a single device specification for all video > functions given that there is a bunch of common stuff. Both encoder and > decoder must negotiate video frame and video stream parameters for > example. Also the virtio-video spec looks like a superset of > virtio-vdec. > > Is there any important feature in video-vdec which is missing in > virtio-video? > > > virtio-video: > > 1. Uses the 'driver requests - device responses' model. > > 2. Does not have the logical split of bitstreams and framebuffers, has > > only a generic buffer object. > > Can you give a quick summary on buffer object management? Buffer objects are implemented in a similar way to how it was done for virtio- gpu. The driver allocates a resource (descriptor) on the device. Than the driver allocates/imports backing pages and attaches those to the resource. When a buffer is ready for processing from the driver's point of view, the driver queues it to the device (please see struct virtio_video_resource_queue). Within this structure the driver provides required metadata (number of data bytes in the buffer, timestamp/counter and so on). The nice thing about this is that there is no real decode or encode requests. The device just fetches the buffer from the input queue, applies some transformation according to its function and settings, fetches an output buffer from the respective queue, and stores the result in the output buffer (or -s, if needed). The device keeps buffer requests until the respective buffer has been processed (i.e. input is consumed or output is written). When the buffer is not needed anymore, the virtio_video_resource_queue request completes asynchronously. This also makes it possible to easily handle different frame types that are decoded out-of-order. Regards, Dmitry. > > thanks, > Gerd - To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
Re: [virtio-dev] [RFC RESEND] virtio-video: Add virtio video device specification
First of all, thanks Dmitry for sharing your protocol here. I hope we can have a productive discussion to establish a nice protocol by comparing with virtio-vdec and virtio-video. On Fri, Nov 8, 2019 at 6:05 PM Gerd Hoffmann wrote: > > > > 1. Both the device and the driver submit requests to each other. For each > > > request the response is sent as a separate request. > > > > To be more precise, in vdec there are no responses. The guest sends > > commands to the host using one virtqueue. The host signals > > asynchronous events, which might not have the exact earlier guest > > request associated to them. > > How do you report errors? Is there an error event for that? We use a field |result| in virtio_vdec_host_req struct. That's to say an error is associated with a host request, not a guest request. Also, we have VIRTIO_VDEC_HOST_REQ_NOTIFY_GLOBAL_ERROR event to report an error associated with no host request. > > > An example of such special case could be > > H.264 framebuffer reordering, where one might end up with a few decode > > requests not resulting in any frames being output and then one decode > > request that would trigger multiple accumulated frames to be returned. > > Note: this can be done with a request/response model too, by simply > completing the decode request when there is frame data, so in that case > multiple decode requests simply complete at the same time. Yes, you can > have multiple outstanding requests in virtio. Out-of-order completion > is also allowed btw. > > > > 2. No support for getting/setting video stream parameters. For example > > > (decoder): output format (NV12, I420), so the driver cannot really select > > > the > > > output format after headers have been parsed. > > > > Getting video stream parameters is there, but they are currently left > > fully in control of the host video decoder. Ability to select between > > multiple possible formats could be worth adding, though. > > Nice to see you agree on that one ;) > > > > 3. No support for getting plane requirements from the device (sg vs > > > contig, > > > size, stride alignment, plane count). > > > > There is actually a bigger difference that results in that. Vdec > > assumes host-allocated buffers coming from a different device, e.g. > > virtio-gpu and the host having the right knowledge to allocate the > > buffers correctly. This is related to the fact that it's generally > > difficult to convey all the allocation constraints in a generic > > manner. > > Yep, buffer handling is tricky, especially when it comes to decoding > directly to gpu buffers and also when supporting playback of > drm-protected streams where the guest might not be allowed to access > the stream data. > > > > 5. Decoder only: new devices will be needed to support encoder, processor > > > or > > > capture. Currently input is always a bitstream, output is always a video > > > frame. No way set input format (needed for encoder, see 2). > > > > The rationale for this was that this is a point of contact with the > > host and a possible attack surface, so having a protocol as specific > > as possible makes the attack surface smaller and is easier to validate > > in the device implementation. > > I think it certainly makes sense to support both video encoding and > video decoding with a single device spec. > > For capture (especially camera) and processor things are less clear, > although there is at least some overlap too. IIRC most of the spec is > "TBD" for those anyway, so I'd suggest to drop them from the spec for > now and focus on the video parts. > I agree that having video codec feature and camera feature in one protocol sounds too complex. Also, if we decide to have a buffer sharing device as Gerd suggested in a different thread, we'll get less overlaps between video codec feature and camera feature. e.g. VIRTIO_VIDEO_T_RESOURCE_* would be simplified. (or removed?) As Tomasz said, I think virtio-vdec can be modified to support encoding as well without big changes. I'm happy to update our protocol and driver implementation to support encoding if needed. Best regards, Keiichi > cheers, > Gerd > - To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
Re: [virtio-dev] [RFC RESEND] virtio-video: Add virtio video device specification
> > 1. Both the device and the driver submit requests to each other. For each > > request the response is sent as a separate request. > > To be more precise, in vdec there are no responses. The guest sends > commands to the host using one virtqueue. The host signals > asynchronous events, which might not have the exact earlier guest > request associated to them. How do you report errors? Is there an error event for that? > An example of such special case could be > H.264 framebuffer reordering, where one might end up with a few decode > requests not resulting in any frames being output and then one decode > request that would trigger multiple accumulated frames to be returned. Note: this can be done with a request/response model too, by simply completing the decode request when there is frame data, so in that case multiple decode requests simply complete at the same time. Yes, you can have multiple outstanding requests in virtio. Out-of-order completion is also allowed btw. > > 2. No support for getting/setting video stream parameters. For example > > (decoder): output format (NV12, I420), so the driver cannot really select > > the > > output format after headers have been parsed. > > Getting video stream parameters is there, but they are currently left > fully in control of the host video decoder. Ability to select between > multiple possible formats could be worth adding, though. Nice to see you agree on that one ;) > > 3. No support for getting plane requirements from the device (sg vs contig, > > size, stride alignment, plane count). > > There is actually a bigger difference that results in that. Vdec > assumes host-allocated buffers coming from a different device, e.g. > virtio-gpu and the host having the right knowledge to allocate the > buffers correctly. This is related to the fact that it's generally > difficult to convey all the allocation constraints in a generic > manner. Yep, buffer handling is tricky, especially when it comes to decoding directly to gpu buffers and also when supporting playback of drm-protected streams where the guest might not be allowed to access the stream data. > > 5. Decoder only: new devices will be needed to support encoder, processor or > > capture. Currently input is always a bitstream, output is always a video > > frame. No way set input format (needed for encoder, see 2). > > The rationale for this was that this is a point of contact with the > host and a possible attack surface, so having a protocol as specific > as possible makes the attack surface smaller and is easier to validate > in the device implementation. I think it certainly makes sense to support both video encoding and video decoding with a single device spec. For capture (especially camera) and processor things are less clear, although there is at least some overlap too. IIRC most of the spec is "TBD" for those anyway, so I'd suggest to drop them from the spec for now and focus on the video parts. cheers, Gerd - To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
Re: [virtio-dev] [RFC RESEND] virtio-video: Add virtio video device specification
On Fri, Nov 8, 2019 at 4:50 PM Gerd Hoffmann wrote: > > On Thu, Nov 07, 2019 at 02:09:34PM +0100, Dmitry Sepp wrote: > > Hello Gerd, > > > > Thank you for your feedback. > > > > There is no relationship between those. As I mentioned earlier. we have also > > been working on a virtio video device at the same time. And there is no > > relationship between the two specs. > > Keiichi, have you looked at the spec? > > I think it is useful to have a single device specification for all video > functions given that there is a bunch of common stuff. Both encoder and > decoder must negotiate video frame and video stream parameters for > example. Also the virtio-video spec looks like a superset of > virtio-vdec. > > Is there any important feature in video-vdec which is missing in > virtio-video? > I just replied to Dmitry's email with further clarification on some vdec aspects and rationale behind some of the design decisions. Please take a look. I think it should be possible to build one protocol for both decoding and encoding. Actually virtio-vdec shouldn't need too much modification to handle encoding. The ability to set operating mode (decoder vs encoder) and set frame buffer format should be enough. However I believe that making it as generic as virtio-video adds too much complexity, increasing the possible attack surface and making it difficult to validate. Best regards, Tomasz - To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
Re: [virtio-dev] [RFC RESEND] virtio-video: Add virtio video device specification
On Thu, Nov 7, 2019 at 10:09 PM Dmitry Sepp wrote: > > Hello Gerd, > > Thank you for your feedback. > > There is no relationship between those. As I mentioned earlier. we have also > been working on a virtio video device at the same time. And there is no > relationship between the two specs. > > I can point you to the differences I see: > > virtio-vdec: > 1. Both the device and the driver submit requests to each other. For each > request the response is sent as a separate request. To be more precise, in vdec there are no responses. The guest sends commands to the host using one virtqueue. The host signals asynchronous events, which might not have the exact earlier guest request associated to them. An example of such special case could be H.264 framebuffer reordering, where one might end up with a few decode requests not resulting in any frames being output and then one decode request that would trigger multiple accumulated frames to be returned. > 2. No support for getting/setting video stream parameters. For example > (decoder): output format (NV12, I420), so the driver cannot really select the > output format after headers have been parsed. Getting video stream parameters is there, but they are currently left fully in control of the host video decoder. Ability to select between multiple possible formats could be worth adding, though. > 3. No support for getting plane requirements from the device (sg vs contig, > size, stride alignment, plane count). There is actually a bigger difference that results in that. Vdec assumes host-allocated buffers coming from a different device, e.g. virtio-gpu and the host having the right knowledge to allocate the buffers correctly. This is related to the fact that it's generally difficult to convey all the allocation constraints in a generic manner. > 4. In the vdec device Drain and Flush are not separate for each buffer queue. > So seek and dynamic resolution change (adaptive playback) cannot be > implemented because 'flush' can have different meaning for a resolution change > and a seek. That's not true. Drain and flush can be defined very precisely for a stateful video decoder. For example, V4L2 defines drain as follows: https://www.kernel.org/doc/html/latest/media/uapi/v4l/dev-decoder.html#drain and it's modeled after that in vdec. There is no flush explicitly defined in V4L2, but that corresponds to the behavior of STREAMOFF, which drops all the buffers on given queue. There is no practical use for flushing just one queue in case of a video decoder, so we decided to simplify it down to a single flush that fully resets the decoder, which is useful for instantaneous seek. There is also already infrastructure existing for dynamic resolution change too, using the stream information host request and drain flow, which is very similar to how this is done in V4L2: https://www.kernel.org/doc/html/latest/media/uapi/v4l/dev-decoder.html#dynamic-resolution-change > 5. Decoder only: new devices will be needed to support encoder, processor or > capture. Currently input is always a bitstream, output is always a video > frame. No way set input format (needed for encoder, see 2). The rationale for this was that this is a point of contact with the host and a possible attack surface, so having a protocol as specific as possible makes the attack surface smaller and is easier to validate in the device implementation. > > virtio-video: > 1. Uses the 'driver requests - device responses' model. > 2. Does not have the logical split of bitstreams and framebuffers, has only a > generic buffer object. > 3. Generic: can support any type of video device right away due to flexibility > of stream configuration. Both input and output buffer queues can accept > bitstream and frame buffers and run independently. (more controls need to be > defined for e.g. camera) To fully support real cameras, not just simple UVC webcams, some mechanism to have multiple output and capture streams synchronized together would be needed, because Android Camera HALv3 heavily relies on multiple stream support. For example, a simple camera application with ZSL (zero shutter lag) could setup following streams: 1) YUV preview 2) RAW capture 3) RAW output 4) JPEG capture 1) and 2) would operate at camera frame rate, while 3) and 4) would be given on demand whenever the user presses the shutter button. Presence of 3) and 4) must not affect 1) and 2), i.e. the preview and raw capture must continue at camera frame rate. > 4. Supports seek, drain, dynamic resolution change using an API agnostic > command set (no v4l2/vaapi or so on remoting). > 5. Complex configuration (most likely cannot be simplified for such a complex > device type). > > Best regards, > Dmitry. > > On Donnerstag, 7. November 2019 10:56:57 CET Gerd Hoffmann wrote: > > On Tue, Nov 05, 2019 at 08:19:19PM +0100, Dmitry Sepp wrote: > > > [Resend after fixing an issue with the virtio-dev mailing list] > > > > > > This patch proposes a virtio speci
Re: [virtio-dev] [RFC RESEND] virtio-video: Add virtio video device specification
On Thu, Nov 07, 2019 at 02:09:34PM +0100, Dmitry Sepp wrote: > Hello Gerd, > > Thank you for your feedback. > > There is no relationship between those. As I mentioned earlier. we have also > been working on a virtio video device at the same time. And there is no > relationship between the two specs. Keiichi, have you looked at the spec? I think it is useful to have a single device specification for all video functions given that there is a bunch of common stuff. Both encoder and decoder must negotiate video frame and video stream parameters for example. Also the virtio-video spec looks like a superset of virtio-vdec. Is there any important feature in video-vdec which is missing in virtio-video? > virtio-video: > 1. Uses the 'driver requests - device responses' model. > 2. Does not have the logical split of bitstreams and framebuffers, has only > a > generic buffer object. Can you give a quick summary on buffer object management? thanks, Gerd - To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
Re: [virtio-dev] [RFC RESEND] virtio-video: Add virtio video device specification
Hello Gerd, Thank you for your feedback. There is no relationship between those. As I mentioned earlier. we have also been working on a virtio video device at the same time. And there is no relationship between the two specs. I can point you to the differences I see: virtio-vdec: 1. Both the device and the driver submit requests to each other. For each request the response is sent as a separate request. 2. No support for getting/setting video stream parameters. For example (decoder): output format (NV12, I420), so the driver cannot really select the output format after headers have been parsed. 3. No support for getting plane requirements from the device (sg vs contig, size, stride alignment, plane count). 4. In the vdec device Drain and Flush are not separate for each buffer queue. So seek and dynamic resolution change (adaptive playback) cannot be implemented because 'flush' can have different meaning for a resolution change and a seek. 5. Decoder only: new devices will be needed to support encoder, processor or capture. Currently input is always a bitstream, output is always a video frame. No way set input format (needed for encoder, see 2). virtio-video: 1. Uses the 'driver requests - device responses' model. 2. Does not have the logical split of bitstreams and framebuffers, has only a generic buffer object. 3. Generic: can support any type of video device right away due to flexibility of stream configuration. Both input and output buffer queues can accept bitstream and frame buffers and run independently. (more controls need to be defined for e.g. camera) 4. Supports seek, drain, dynamic resolution change using an API agnostic command set (no v4l2/vaapi or so on remoting). 5. Complex configuration (most likely cannot be simplified for such a complex device type). Best regards, Dmitry. On Donnerstag, 7. November 2019 10:56:57 CET Gerd Hoffmann wrote: > On Tue, Nov 05, 2019 at 08:19:19PM +0100, Dmitry Sepp wrote: > > [Resend after fixing an issue with the virtio-dev mailing list] > > > > This patch proposes a virtio specification for a new virtio video > > device. > > Hmm, quickly looking over this, it looks simliar to the vdec draft > posted a few weeks ago, with other device variants added but not > fully specified yet. > > So, can you clarify the relationship between the two? > > thanks, > Gerd - To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
Re: [virtio-dev] [RFC RESEND] virtio-video: Add virtio video device specification
On Tue, Nov 05, 2019 at 08:19:19PM +0100, Dmitry Sepp wrote: > [Resend after fixing an issue with the virtio-dev mailing list] > > This patch proposes a virtio specification for a new virtio video > device. Hmm, quickly looking over this, it looks simliar to the vdec draft posted a few weeks ago, with other device variants added but not fully specified yet. So, can you clarify the relationship between the two? thanks, Gerd - To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] [RFC RESEND] virtio-video: Add virtio video device specification
[Resend after fixing an issue with the virtio-dev mailing list]
This patch proposes a virtio specification for a new virtio video
device.
The virtio video device is an abstract video streaming device that
operates input and/or output data buffers to share video devices with
several guests. The device uses descriptor structures to advertise and
negotiate stream formats and controls. This allows the driver to modify
the processing logic of the device on a per stream basis. The generic
nature of the device makes it possible to support the following video
functions: encoder, decoder, processor, capture, output.
Thank you in advance for any feedback.
Signed-off-by: Dmitry Sepp
---
content.tex | 1 +
virtio-video.tex | 776 +++
2 files changed, 777 insertions(+)
create mode 100644 virtio-video.tex
diff --git a/content.tex b/content.tex
index b1ea9b9..6990b5d 100644
--- a/content.tex
+++ b/content.tex
@@ -5698,6 +5698,7 @@ \subsubsection{Legacy Interface: Framing
Requirements}\label{sec:Device
\input{virtio-crypto.tex}
\input{virtio-vsock.tex}
\input{virtio-fs.tex}
+\input{virtio-video.tex}
\chapter{Reserved Feature Bits}\label{sec:Reserved Feature Bits}
diff --git a/virtio-video.tex b/virtio-video.tex
new file mode 100644
index 000..84aade8
--- /dev/null
+++ b/virtio-video.tex
@@ -0,0 +1,776 @@
+\section{Video Device}\label{sec:Device Types / Video Device}
+
+The virtio video device is a virtual video streaming device that supports the
+following functions: encoder, decoder, capture, output.
+
+\subsection{Device ID}\label{sec:Device Types / Video Device / Device ID}
+
+TBD.
+
+\subsection{Virtqueues}\label{sec:Device Types / Video Device / Virtqueues}
+
+\begin{description}
+\item[0] controlq
+\item[1] eventq
+\end{description}
+
+\subsection{Feature bits}\label{sec:Device Types / Video Device / Feature bits}
+
+\begin{description}
+\item[VIRTIO_VIDEO_F_ENCODER (0)] Video encoder function supported.
+\item[VIRTIO_VIDEO_F_DECODER (1)] Video decoder function supported.
+\item[VIRTIO_VIDEO_F_PROCESSOR (2)] Video processor function supported.
+\item[VIRTIO_VIDEO_F_CAPTURE (3)] Video capture function supported.
+\item[VIRTIO_VIDEO_F_OUTPUT (4)] Video output function supported.
+\end{description}
+
+\subsection{Supported functions}\label{sec:Device Types / Video Device /
Supported video functions}
+
+The following video functions are defined:
+
+\begin{lstlisting}
+enum virtio_video_func_type {
+ VIRTIO_VIDEO_FUNC_UNDEFINED = 0,
+
+ VIRTIO_VIDEO_FUNC_ENCODER = 0x0100,
+ VIRTIO_VIDEO_FUNC_DECODER,
+ VIRTIO_VIDEO_FUNC_PROCESSOR,
+ VIRTIO_VIDEO_FUNC_CAPTURE,
+ VIRTIO_VIDEO_FUNC_OUTPUT,
+};
+\end{lstlisting}
+
+\subsubsection{Function description}\label{sec:Device Types / Video Device /
Supported functions / Function description}
+
+The device reports its configuration using descriptors. A descriptor is a data
+structure with a defined format. Each descriptor begins with a generic virtio
+video descriptor header that contains information about the descriptor type and
+its length.
+
+\begin{lstlisting}
+enum virtio_video_desc_type {
+ VIRTIO_VIDEO_DESC_UNDEFINED = 0,
+
+ VIRTIO_VIDEO_DESC_FRAME_RATE = 0x0100,
+ VIRTIO_VIDEO_DESC_FRAME_SIZE,
+ VIRTIO_VIDEO_DESC_PIX_FORMAT,
+ VIRTIO_VIDEO_DESC_PLANE_FORMAT,
+ VIRTIO_VIDEO_DESC_EXTRAS,
+ VIRTIO_VIDEO_DESC_CAP,
+ VIRTIO_VIDEO_DESC_FUNC,
+ VIRTIO_VIDEO_DESC_PARAMS,
+};
+
+struct virtio_video_desc {
+ __le32 type; /* One of VIRTIO_VIDEO_DESC_* types */
+ __le16 length;
+ __u8 padding[2];
+};
+\end{lstlisting}
+
+Functions describe a set of services that the device offers. In general, the
+device can transform data from one format to another
+(encoder/decoder/processor) or produce/consume data (capture/output).
+
+Functions have the descriptor type VIRTIO_VIDEO_DESC_FUNC. Each
+encoder/decoder/processor function has 2 pins, input and output. Capture
+function has only one input pin, output has only an output pin. Each pin has a
+capability that describes formats this pin can handle. Also, a function can
+have a set of control capabilities that control the process of data
+transformation/production/consumption.
+
+The capability that describes the data format consists of:
+
+\begin{itemize*}
+\item a set of pixel formats that contains one or more:
+\item supported frame sizes that contains one or more:
+\item supported frame rates.
+\end{itemize*}
+
+The description of each function looks as follows:
+
+\begin{itemize*}
+ \item capability (input pin)
+ \begin{itemize*}
+\item pixel format
+\begin{itemize*}
+ \item frame size
+\begin{itemize*}
+ \item frame rate
+ \item ... (other possible frame rates)
+\end{itemize*}
+ \item ... (other possible frame sizes)
+ \end{itemize*}
+\item ... (other possible pixel formats)
+\end{itemize*}
+
