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 <kraxel@xxxxxxxxxx> 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 >
