Skip to content

Send potential supported formats instead of few hardcoded ones.#23

Open
tegelane89 wants to merge 6 commits into
music-assistant:mainfrom
tegelane89:feature/detect-device-supported-formats
Open

Send potential supported formats instead of few hardcoded ones.#23
tegelane89 wants to merge 6 commits into
music-assistant:mainfrom
tegelane89:feature/detect-device-supported-formats

Conversation

@tegelane89

Copy link
Copy Markdown
Contributor

No description provided.

Comment thread src-tauri/src/sendspin/mod.rs Outdated
Comment thread src-tauri/src/sendspin/mod.rs Outdated
@teancom

teancom commented Mar 10, 2026

Copy link
Copy Markdown
Contributor

I'm so sorry, I didn't even notice this was still in Draft. 😬😅

@tegelane89

tegelane89 commented Mar 10, 2026

Copy link
Copy Markdown
Contributor Author

I'm so sorry, I didn't even notice this was still in Draft. 😬😅

As you can probably tell i dont really know rust. I Started learning it specifically to get this thing working in windows. So any help is appreciated. Thank you for your comments.

Its draft because I don't know if it is in generally approach. You may see that there is something that iterates over sample rates inside devices.rs. But im not sure what that is for. But i guess even if this pr is not going to be merged, it should help to find/understand root cause.

@tegelane89 tegelane89 marked this pull request as ready for review March 20, 2026 23:15
@tegelane89

Copy link
Copy Markdown
Contributor Author

I have pulled latest and fixed merge conflicts and addressed @teancom comments.
I guess the thing in devices.rs was meant to give list of devices to choose from.

I tried to play file source on windows 11, windows 10, fedora worksation VM (Gnome has no system tray - so it is impossible to select device. but plays on default device), Catchy Os KDE VM.

@tegelane89 tegelane89 requested a review from teancom March 21, 2026 00:31
@teancom

teancom commented Mar 22, 2026

Copy link
Copy Markdown
Contributor

Hi! Sorry for the slow response, I'm actually pretty new to this codebase myself. I'll add a couple of comments in-line in the diff.

@teancom teancom left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

There are a couple of other issues - no tests is a big one. But also, I feel like you had a proper bug report to go with this and now I can't find it. And the original comment box is empty. Please add (you can still edit) the top comment to describe what you're trying to achieve and why. That will definitely help with tracking down whatever the bug is.

Thank you!

use cpal::{traits::DeviceTrait, SampleFormat, SampleRate};
use sendspin::protocol::messages::AudioFormatSpec;

//pub fn get_device_formats(device_id: &str) -> Vec<AudioFormatSpec> {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

You can delete this commented out line.

buffer_size: cpal::BufferSize::Default,
};

// Try to open a stream to test if it's truly supported

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can I ask why we need to do this? Specifically the bit where we are trying a bunch of different combinations with 'live' data vs. trusting the ranges reported by supported_output_configs?

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants