vhost-device-sound: Add GStreamer audio backend support#876
vhost-device-sound: Add GStreamer audio backend support#876stefano-garzarella merged 1 commit intorust-vmm:mainfrom
Conversation
|
the build is failing because |
right, I just did it at rust-vmm/rust-vmm-ci#190 |
|
@nicholasdezai we just merged #881 that should bring @MatiasVara changes to add GStreamer deps to our CI container, please rebase this PR. |
|
I submitted a PR to rust-vmm-container to add the missing GStreamer dependencies |
|
Please rebase this PR on |
121daab to
b436032
Compare
stefano-garzarella
left a comment
There was a problem hiding this comment.
Great, thanks for working on this and preparing the container for the CI!
I left some comments, in general I'd avoid unwrap()/expect(). I mean t's fine for the lock or something that should not happen, but for example you have in some places .ok_or(Error::StreamWithIdNotFound(stream_id))? and in others .expect("Can not find stream in stream_in.");. Why? (I didn't look the entire code, so maybe there is a reason).
1e7e0fa to
859b44e
Compare
|
I have improved the error handling in the new commit. I hope the error handling makes more sense now |
|
I have tried both playback and recording. Both operations seem working. |
|
@dorindabassey @epilys PTAL |
PR LGTM last time I checked, but I’ll do a more thorough review later(tomorrow). |
|
@nicholasdezai please in the meantime rebase this on |
dorindabassey
left a comment
There was a problem hiding this comment.
Thank you @nicholasdezai for the work on this PR, I added just a few comments.
|
Hi @nicholasdezai , I tested this on Fedora42 with |
|
you also need to update the readme with the new backend |
|
I’m testing with Debian 13 (trixie), kernel 6.15.5. |
|
I just tried with qemu from master(29b77c1a2db2), Linux v6.16-rc1, and GStreamer 1.24.0, and it seems working. |
|
I used QEMU from master(517e9b4862cc), gstreamer 1.24.0, kernel 6.16.9, f42, also tried debian-13-nocloud-amd64.qcow2 image with gstreamer 1.24.0 i got no audio, but after updating to 1.24.13 it works with the debian image, so probably not an issue with the backend. |
Add gstreamer backend. Add gstreamer-related crates used in the vhost-device-sound module This affects only the vhost-device-sound module, and updates the following dependencies: - dependency-name: gstreamer dependency-version: 0.24.2 - dependency-name: gstreamer-app dependency-version: 0.24.2 - dependency-name: gstreamer-audio dependency-version: 0.24.2 Signed-off-by: nicholasdezai <nicholasdezai@gmail.com>
dorindabassey
left a comment
There was a problem hiding this comment.
PR LGTM, great work! @nicholasdezai
Summary of the PR
This change adds GStreamer audio backend support.
Since the Rust bindings for GStreamer rely on the underlying C libraries, this change introduces additional build dependencies. On Debian/Ubuntu systems, at least libgstreamer1.0-dev and libgstreamer-plugins-base1.0-dev are required.
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.