Skip to content

vhost-device-sound: Add GStreamer audio backend support #876

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

vhost-device-sound: Add GStreamer audio backend support #876

wants to merge 4 commits into from
+1,676 −81

Conversation

nicholasdezai
Copy link

@nicholasdezai nicholasdezai commented Sep 16, 2025 •
edited
Loading

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:

  • All commits in this PR have Signed-Off-By trailers (with
    git commit -s), and the commit message has max 60 characters for the
    summary and max 75 characters for each description line.
  • All added/changed functionality has a corresponding unit/integration
    test.
  • All added/changed public-facing functionality has entries in the "Upcoming
    Release" section of CHANGELOG.md (if no such section exists, please create one).
  • Any newly added unsafe code is properly documented.

@dorindabassey
Copy link
Collaborator

the build is failing because gstreamer-base-1.0 is missing from the build environment, looks like we need to update the rust-vmm container with gstreamer.

  pkg-config exited with status code 1
  > PKG_CONFIG_ALLOW_SYSTEM_CFLAGS=1 pkg-config --libs --cflags gstreamer-base-1.0 'gstreamer-base-1.0 >= 1.20'
  The system library `gstreamer-base-1.0` required by crate `gstreamer-base-sys` was not found.
  The file `gstreamer-base-1.0.pc` needs to be installed and the PKG_CONFIG_PATH environment variable must contain its parent directory.
  The PKG_CONFIG_PATH environment variable is not set.

@MatiasVara
Copy link
Contributor

the build is failing because gstreamer-base-1.0 is missing from the build environment, looks like we need to update the rust-vmm container with gstreamer.

  pkg-config exited with status code 1
  > PKG_CONFIG_ALLOW_SYSTEM_CFLAGS=1 pkg-config --libs --cflags gstreamer-base-1.0 'gstreamer-base-1.0 >= 1.20'
  The system library `gstreamer-base-1.0` required by crate `gstreamer-base-sys` was not found.
  The file `gstreamer-base-1.0.pc` needs to be installed and the PKG_CONFIG_PATH environment variable must contain its parent directory.
  The PKG_CONFIG_PATH environment variable is not set.

right, I just did it at rust-vmm/rust-vmm-ci#190

@stefano-garzarella
Copy link
Member

stefano-garzarella commented Sep 22, 2025 •
edited
Loading

@nicholasdezai we just merged #881 that should bring @MatiasVara changes to add GStreamer deps to our CI container, please rebase this PR.

@nicholasdezai
Copy link
Author

I submitted a PR to rust-vmm-container to add the missing GStreamer dependencies
(gstreamer1.0-plugins-base and gstreamer1.0-plugins-good) required for running
the GStreamer backend tests in CI.

@stefano-garzarella
Copy link
Member

Please rebase this PR on main and keep dep updates (e.g. rust-vmm-ci submodule) in separate commits.

@nicholasdezai
build(deps): bump rust-vmm-ci from e8117d1 to c0f5d4c
db64228 
Bumps rust-vmm-ci from e8117d1 to c0f5d4c.

Signed-off-by: nicholasdezai <[email protected]>
@nicholasdezai nicholasdezai force-pushed the gst branch 2 times, most recently from d625287 to 121daab Compare September 25, 2025 17:51
@nicholasdezai
chore: bump gstreamer dependencies
84c7fe5 
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 <[email protected]>
stefano-garzarella
stefano-garzarella reviewed Sep 26, 2025
View reviewed changes
Copy link
Member

@stefano-garzarella stefano-garzarella left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

vhost-device-i2c/src/main.rs
socket_count: usize,

/// List of I2C bus and clients in format
/// `<bus-name>:<client_addr>[:<client_addr>][,<bus-name>:<client_addr>[:<client_addr>]]`.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unrelated change, I guess happened while rebasing.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't notice it here, it has been rolled back in the new version

Cargo.lock
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This commit 84c7fe5 can be squashed in the next one where you really need the dependencies.

vhost-device-sound/src/audio_backends/gstreamer.rs Outdated

pipeline
.add_many([&autoaudiosrc, appsink.upcast_ref()])
.expect("Failed to add elements to input pipeline");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not returning an Err ?

vhost-device-sound/src/audio_backends/gstreamer.rs Outdated
let mut stream_in = self.stream_in.write().unwrap();
let pipeline_in = stream_in
.get(&stream_id)
.expect("Can not find stream in `stream_in`.");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ditto.

@nicholasdezai
vhost-device-sound: Add GStreamer audio backend support
750f7d3 
Add gstreamer backend.

Signed-off-by: nicholasdezai <[email protected]>
@nicholasdezai nicholasdezai force-pushed the gst branch 2 times, most recently from 63c5f82 to 1e7e0fa Compare September 27, 2025 13:30
@nicholasdezai
vhost-device-sound: improve error handling in gst
859b44e 
- Replace some unwrap/expect with proper Result handling
- Add GstError to handle GstreamerBackend init fail

Signed-off-by: nicholasdezai <[email protected]>
@nicholasdezai
Copy link
Author

I have improved the error handling in the new commit. I hope the error handling makes more sense now

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@stefano-garzarella stefano-garzarella stefano-garzarella left review comments

@dorindabassey dorindabassey Awaiting requested review from dorindabassey dorindabassey is a code owner

@stsquad stsquad Awaiting requested review from stsquad stsquad is a code owner

@vireshk vireshk Awaiting requested review from vireshk vireshk is a code owner

@epilys epilys Awaiting requested review from epilys epilys is a code owner

At least 2 approving reviews are required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@nicholasdezai @dorindabassey @MatiasVara @stefano-garzarella