Hi Dmitry, On Thu, Jul 2, 2020 at 4:32 PM Dmitry Sepp <dmitry.sepp@xxxxxxxxxxxxxxx> wrote: > > Hi Keiichi, > > Thank you very much for the hard work to update the spec and to summarize all > of the recent proposals! > > I want to again raise a topic that was discussed earlier and unfortunately the > latest proposal cannot resolve the problem. I hope together with upstream > people we'll be able to find a neat solution. > > Please consider the following case: > 1. Encoder case > 2. User app does reqbufs with DMABUF flag. > 3. User app submits frames to encode, each frame has a different fd, might be a > completely new buffer. > 4. Driver receives this buffer via queue() and does this check to verify > whether it is a known dmabuf: > https://elixir.bootlin.com/linux/v5.7.6/source/drivers/media/common/videobuf2/ > videobuf2-core.c#L1163 > 5. When the check fails, it does cleanup. > 6. BUG: As we got rid of the flexible resource detach/destroy calls, host side > has no way to know the resource has a new memory region. The new sgt is never > propagated to the host. > > The mentioned earlier CMD_RESOURE_REASSIGN_ENTRIES/CMD_RESOURE_REASSIGN_OBJECT > are not included in the spec. This shouldn't be a problem. Though we don't have a per-resource detach command, we can _replace_ attached sg-list or virtio-object by calling the attach command. In your scenario, if the driver notices a new dmabuf is given at 4 and 5, the driver should send RESOURCE_ATTACH command with the new dmabuf. Then, the old dmabuf was regarded as "detached". Please see the "Buffer life cycle" section. I renamed RESOURCE_REASSIGN_* commands to RESOURCE_ATTACH as it's also used at the first time to attach a buffer. Best regards, Keiichi > > Thanks in advance. > > Best regards, > Dmitry. > > On Dienstag, 23. Juni 2020 13:13:24 CEST Keiichi Watanabe wrote: > > > > This is the v4 RFC of virtio video device specification. > > PDF versions are available at [1, 2]. > > > > Note that this patch depends on a recent patchset "Cross-device resource > > sharing" [3]. > > > > Here is a list of major changes from v3: > > * Redesingned struct definitions for each command and request based on > > discussions at [4]. > > * Renamed commands/structs/enums to more descriptive names. > > * Had different structs and fields for image formats and bitstream formats. > > * Added more detailed specification for supported video codecs. > > * Made stream_id be allocated by the device. > > * Had a single parameter struct per-stream instead of per-queue parameters > > and controls. > > * Allowed the driver to specify the number of buffers to use via > > "cur_{image,bitstream}_buffers". > > * Renamed RESOURCE_CREATE command to RESOURCE_ATTACH command and allow the > > driver to use this command when replacing backing memories as well. > > > > [5] is the diff of the header file from v3. Note that it only contains > > changes in the header. We haven't updated the driver nor device > > implementation to focus on protocol design discussion first. > > > > While it may appear that many parts have been changed since the previous > > revision, these changes are to address the issues raised in previous > > discussions or/and to make the protocol simpler and easier to prevent > > misuse. > > I'd appreciate any types of feedback. > > > > Best regards, > > Keiichi > > > > [1] (full): > > https://drive.google.com/file/d/1DiOJZfUJ5wvFtnNFQicxt0zkp4Ob1o9C/view?usp= > > sharing [2] (only video section): > > https://drive.google.com/file/d/188uAkIWE0BsXETECez98y5fJKw8rslj3/view?usp= > > sharing [3] > > https://lists.oasis-open.org/archives/virtio-comment/202003/msg00035.html > > [4] https://markmail.org/thread/c6h3e3zn647qli3w > > [5] > > https://chromium-review.googlesource.com/c/chromiumos/third_party/kernel/+/ > > 2164411 > > > > Keiichi Watanabe (1): > > virtio-video: Add virtio video device specification > > > > .gitignore | 1 + > > content.tex | 1 + > > images/video-buffer-lifecycle.dot | 18 + > > make-setup-generated.sh | 8 + > > virtio-video.tex | 1163 +++++++++++++++++++++++++++++ > > 5 files changed, 1191 insertions(+) > > create mode 100644 .gitignore > > create mode 100644 images/video-buffer-lifecycle.dot > > create mode 100644 virtio-video.tex > > > > -- > > 2.27.0.111.gc72c7da667-goog > >
