Add vhost-device-media backend for virtio-media#944
Conversation
c3a8789 to
4a6c56d
Compare
|
Thank you for this PR @aesteve-rh |
|
@dorindabassey virtio-video is still under development (see virtio-comment list for v10). It's not replaced by virtio-media. |
4a6c56d to
5ddd48f
Compare
6ffe82f to
0727f40
Compare
|
Some |
0727f40 to
42eaca8
Compare
@aesteve-rh yeah, we need to manually update |
|
Ah, yes. Makes sense. I think I can manage. Thanks for the pointer! |
|
CI update: rust-vmm/rust-vmm-ci#205 |
|
@aesteve-rh it seems there is still something missing in the CI container. I suggest running it locally following https://github.com/rust-vmm/rust-vmm-ci#running-the-tests-locally |
|
Another +1 for this one? rust-vmm/rust-vmm-ci#207 edit: |
e5a2e02 to
05a284d
Compare
|
Looks much cleaner now. One of the remaining issues (with cargo-audit) is related with Gnurou/v4l2r#60, so it is a matter of time we get rid of it, hopefully. The only other issue is with coverage. I will double check where's the gap and see if I can close it. Thanks @stefano-garzarella and @epilys for the reviews and the patience! |
7540e1a to
604b92c
Compare
937abba to
177bb16
Compare
|
The The only other job failing ( Either way, the branch is probably in good position to get some early reviews? I understand that it is a big change and it is not easy to go through in one sit. I myself get a hard time when revisting changes in each commit and self-reviewing. I am open to suggestions in how to organize this to make reviews easier to handle. |
Add the QEMU side of the vhost-user-media device, which connects to a
virtio-media vhost-user backend daemon implementing the V4L2 API over
the virtio-media protocol.
The virtio-media device is specified in the VirtIO specification v1.4,
section 5.22:
https://docs.oasis-open.org/virtio/virtio/v1.4/cs01/virtio-v1.4-cs01.html
Tested with the rust-vmm vhost-device-media backend [1]:
cargo run -- -s /path/to/media.sock -d /dev/video0 --backend v4l2-proxy
Example invocation:
qemu-system-x86_64 \
-chardev socket,path=/tmp/media.sock,id=media \
-device vhost-user-media-pci,chardev=media,id=media
[1] rust-vmm/vhost-device#944
Signed-off-by: Albert Esteve <aesteve@redhat.com>
There was a problem hiding this comment.
@aesteve-rh maybe we could squash all the commits into one? Not sure if it makes a difference in review, for me viewing all the changes at once looks very readable as well. WDYT?
@epilys Some of the commits have some explanation/reasoning for the changes which would be lost otherwise. Maybe for those that only touch the code would make sense to squash. On the one hand, reviewing commit by commit has its problems as there is some back and forth with the structure, so a reviewer can comment on something that changed later on. Squashing would solve that. On the other hand, reviewing only the final state is an option available for reviewers in github, just checking the So not sure, but it may make sense given that the changeset is so big, to just focus on the final state and add a single commit to the repository. |
|
My suggestion: squash everything together, except for maybe the unit tests and CI if you wanna keep the commits small. You can merge parts of the commit texts from each squashed commit. As I said, the code changes are easy to read (for me), but the changing between commits is making it unnecessarily confusing. Just my 2 cents |
|
Done the squash, left the test, CI and some other build deps update separated. Also, the new additions that came from comment on this branch I left separated so that chages are easier to spot and because they can stand on their own anyway (i.e., changing |
|
CI is only failing due to a required Btw, I already sent the frontend patch series to QEMU: https://lore.kernel.org/all/20260630112310.552606-1-aesteve@redhat.com/ |
|
OK went through everything again, LGTM overall. Let's wait for whatever CI/dependency todos we have and I think we're OK for merge, generally. |
|
@aesteve-rh anyhow was fixed, you can now rebase on top of |
The virtio-media device first appeared in virtio-spec v1.4 [1]. It standardizes how guest applications communicate with media devices on the host, such as encoders, decoders, and cameras, using the V4L2 (Video4Linux2) protocol for guest-host communication. The low-level implementation of the virtio-media protocol is provided by the device crate from the virtio-media repository [2]. The crate follows the same lib + binary structure as other vhost-device crates: core logic lives in lib.rs, while main.rs is a thin CLI entry point. Each backend is gated by an optional cargo feature to minimize binary size and the dependency footprint: - `simple-capture`: simple V4L2 capture device - `v4l2-proxy`: V4L2 proxy device (enabled by default) - `ffmpeg`: software video decoding via the virtio-media-ffmpeg-decoder crate [3], useful for testing without dedicated hardware - `null`: always-compiled no-op backend - `xen`: enables Xen platform support for rust-vmm dependencies A `V4l2DeviceType` helper resolves the device path given on the command line to the appropriate VFL_TYPE_* kernel constant. The `MediaAllocator`, which manages shared memory regions for the device, is adapted from `AddressAllocator` in the crosvm `resources` crate. [1] https://docs.oasis-open.org/virtio/virtio/v1.4/virtio-v1.4.html#x1-82200022 [2] https://github.com/chromeos/virtio-media [3] https://github.com/chromeos/virtio-media/tree/main/extras/ffmpeg-decoder Signed-off-by: Albert Esteve <aesteve@redhat.com>
Add unit tests to existing code for vhost-device-media covering: - descriptor_chain: DescriptorChainReader/Writer read/write paths and boundary behaviour. - vhu_media_thread: VhostUserMediaThread queue processing helpers. - vhu_media: VuMediaBackend construction, handle_event dispatch, and error-type conversions. - lib: CLI argument parsing and socket-binding smoke tests for each backend. Assisted-by: Claude <noreply@anthropic.com> Signed-off-by: Albert Esteve <aesteve@redhat.com>
vhost-device-media depends on v4l2r, which uses bindgen in its build script to generate Rust bindings from /usr/include/linux/videodev2.h. That header includes <sys/time.h>, which on Debian/Ubuntu aarch64 is placed under the multiarch path /usr/include/aarch64-linux-gnu/ rather than directly under /usr/include/. Clang, invoked by bindgen, does not add that directory to its default include search path, causing the build to fail with: /usr/include/linux/videodev2.h:60:10: fatal error: 'sys/time.h' file not found Set BINDGEN_EXTRA_CLANG_ARGS to pass the multiarch include directory to clang for all GNU build and test steps. The path is derived from $(uname -m) so it resolves correctly on x86_64 and aarch64 alike, and is a no-op on any platform where the directory does not exist. Additionally, the ffmpeg feature pulls in virtio-media-ffmpeg-decoder, whose build script uses pkg-config to locate libavcodec. pkg-config refuses to run for a musl cross-compilation target, and libavcodec is not available as a musl-linked library regardless. Exclude vhost-device-media from all musl build and test jobs, following the same pattern already used for vhost-device-gpu. Signed-off-by: Albert Esteve <aesteve@redhat.com>
Add a [patch.crates-io] override for v4l2r, redirecting every workspace member that pulls v4l2r to the git revision 12afd03. That revision replaces the unmaintained paste crate (RUSTSEC-2024-0436) with its maintained successor pastey, resolving the cargo audit denial. The direct dependency in vhost-device-media already pinned this revision. The patch unifies the transitive pull through virtio-media to the same source, eliminating duplicate crates-io entry from the lockfile. Signed-off-by: Albert Esteve <aesteve@redhat.com>
Move MediaArgs and BackendType into a dedicated args module so that it can be reused by the xtask crate to generate a manpage in a followup commit. The From<MediaArgs> for VuMediaConfig conversion is moved to lib.rs where both types are defined. Signed-off-by: Albert Esteve <aesteve@redhat.com>
Add vhost-device-media support for the mangen task in xtask binary. This generates a manpage in target/dist/man/vhost-device-media.1 Signed-off-by: Albert Esteve <aesteve@redhat.com>
This allows a service manager to start vhost-device-media with an already listening socket. With this, the service manager can create the socket in advance of starting any services, so there's no race in between vhost-device-media being started and being ready to accept connections. Signed-off-by: Albert Esteve <aesteve@redhat.com>
|
Passes the tests! Right now the only missing dependency is for I can handle it in a follow up or wait for the change to propagate here. For it to land it requires |
Summary of the PR
The virtio-media device first appeared in virtio-spec v1.4 [1].
It standardizes how guest applications communicate with media
devices on the host, such as encoders, decoders, and cameras,
using the V4L2 (Video4Linux2) protocol for guest-host
communication.
The low-level implementation of the virtio-media protocol is
provided by the device crate from the virtio-media repository [2].
The crate follows the same lib + binary structure as other
vhost-device crates: core logic lives in lib.rs, while main.rs
is a thin CLI entry point.
Each backend is gated by an optional cargo feature to minimize
binary size and the dependency footprint:
simple-capture: simple V4L2 capture devicev4l2-proxy: V4L2 proxy device (enabled by default)ffmpeg: software video decoding via the virtio-media-ffmpeg-decodercrate [3], useful for testing without dedicated hardware
null: always-compiled no-op backendxen: enables Xen platform support for rust-vmm dependenciesA
V4l2DeviceTypehelper resolves the device path given on thecommand line to the appropriate VFL_TYPE_* kernel constant.
The
MediaAllocator, which manages shared memory regions for thedevice, is adapted from
AddressAllocatorin the crosvmresourcescrate.
[1] https://docs.oasis-open.org/virtio/virtio/v1.4/virtio-v1.4.html#x1-82200022
[2] https://github.com/chromeos/virtio-media
[3] https://github.com/chromeos/virtio-media/tree/main/extras/ffmpeg-decoder
Requirements
Before submitting your PR, please make sure you addressed the following
requirements:
git commit -s), and the commit message has max 60 characters for thesummary and max 75 characters for each description line.
test.
Release" section of CHANGELOG.md (if no such section exists, please create one).
unsafecode is properly documented.