Uh oh!
There was an error while loading. Please reload this page.
Migrate audio pipeline to float from 16-bit integer - #2873
Conversation
cgutman
left a comment
There was a problem hiding this comment.
Successfully tested locally on macOS and Linux
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@## master #2873 +/- ##
=========================================
+ Coverage 9.19% 9.57% +0.37%
=========================================
Files 97 97 Lines 17523 17586 +63 Branches 8331 8336 +5 =========================================
+ Hits 1611 1683 +72 - Misses 13086 15099 +2013 + Partials 2826 804 -2022
Flags with carried forward coverage won't be shown. Click here to find out more.
|
Uh oh!
There was an error while loading. Please reload this page.
| // *2 --> needs to fit double | ||
| sample_buf = util::buffer_t<std::int16_t> { std::max(frames, frame_size) * 2 * channels_out }; | ||
| sample_buf = util::buffer_t<float> { std::max(frames, frame_size) * 2 * channels_out }; |
Check failure
Code scanning / CodeQL
Multiplication result converted to larger type
This seems to break audio on my system by default. Log is here; relevant lines: I can make audio work with your PR by disabling (or rather, disallowing audio playback via the Sound settings) the Steam Streaming Speaker and Microphone devices and reconnecting the session; simply setting a real (non-Steam) endpoint to the default doesn't fix the issue, as the Steam virtual devices are always set to default when a new session is started. |
ns6089
commented
Jul 20, 2024
Can reproduce, let's see what did I do wrong... |
ns6089
commented
Jul 20, 2024
Messed with the function picks format for steam virtual speakers device. @psyke83 Should work now, good catch |
ns6089
commented
Jul 20, 2024
Wasn't me by the way, github did it automatically. |
Reviews get dismissed when the code changes. That is a branch protection rule I have enabled. |
psyke83
commented
Jul 20, 2024
Confirmed working via Steam endpoints with latest changes on your PR branch - thanks. |
| set_wave_format(audio::wave_format_t &wave_format, const format_t &format) { | ||
| wave_format->nSamplesPerSec = SAMPLE_RATE; | ||
| wave_format->wBitsPerSample = 16; | ||
| wave_format->wBitsPerSample = 24; |
There was a problem hiding this comment.
I'm concerned we might need some additional logic for this. I don't know if it's guaranteed that all devices will support 24-bit PCM for all channel configurations. I think it would be best if we tried them in order: 32-bit -> 24-bit -> 16-bit
That would ensure we don't possibly regress any audio devices that are 16-bit only, and also provide the highest quality on devices with 32-bit support.
There was a problem hiding this comment.
It's used only for "virtual" sinks, which I thought only consisted of steam virtual speakers. Am I wrong in this assumption?
/** * If the requested sink is a virtual sink, meaning no speakers attached to * the host, then we can seamlessly set the format to stereo and surround sound. * * Any virtual sink detected will be prefixed by: * virtual-(format name) * If it doesn't contain that prefix, then the format will not be changed*/
std::optional<std::wstring>
set_format(const std::string &sink) {
auto sink_info = get_sink_info(sink);
// If the sink isn't a device name, we'll assume it's a device IDauto wstring_device_id = find_device_id_by_name(sink).value_or(from_utf8(sink_info.second.data()));
if (sink_info.first == format_t::none) {
// wstring_device_id does not contain virtual-(format name)// It's a simple deviceId, just pass it backreturnstd::make_optional(std::move(wstring_device_id));
}
wave_format_t wave_format;
auto status = policy->GetMixFormat(wstring_device_id.c_str(), &wave_format);
if (FAILED(status)) {
BOOST_LOG(error) << "Couldn't acquire Wave Format [0x"sv << util::hex(status).to_string_view() << ']';
return std::nullopt;
}
set_wave_format(wave_format, formats[(int) sink_info.first - 1]);
WAVEFORMATEXTENSIBLE p {};
status = policy->SetDeviceFormat(wstring_device_id.c_str(), wave_format.get(), (WAVEFORMATEX *) &p);
// Surround 5.1 might contain side-{left, right} instead of speaker in the back// Try again with different speaker mask.if (status == 0x88890008 && sink_info.first == format_t::surr51) {
set_wave_format(wave_format, surround_51_side_speakers);
status = policy->SetDeviceFormat(wstring_device_id.c_str(), wave_format.get(), (WAVEFORMATEX *) &p);
}
if (FAILED(status)) {
BOOST_LOG(error) << "Couldn't set Wave Format [0x"sv << util::hex(status).to_string_view() << ']';
return std::nullopt;
}
returnstd::make_optional(std::move(wstring_device_id));
}There was a problem hiding this comment.
It's used only for "virtual" sinks, which I thought only consisted of steam virtual speakers. Am I wrong in this assumption?
Other virtual speakers can be used, such as virtual audio cable https://vb-audio.com/Cable/
There was a problem hiding this comment.
Alright, so sunshine can have default or custom audio sink, and default or custom virtual audio sink.
Will check how it can be handled.
There was a problem hiding this comment.
I think it could just be as simple as a loop that tries SetDeviceFormat() until it succeeds.
There was a problem hiding this comment.
I thought it would be a simple loop too, but then ended up with a nasty memory corruption. The rest of the code had accumulated quite a bit of technical dept. Either way it should be ready soon.
There was a problem hiding this comment.
Pushed and tested what I could.
| auto adapter_name = no_null((LPWSTR) adapter_friendly_name.prop.pszVal); | ||
| auto device_name = no_null((LPWSTR) device_friendly_name.prop.pszVal); | ||
| auto device_description = no_null((LPWSTR) device_desc.prop.pszVal); | ||
| auto adapter_name = (LPWSTR) adapter_friendly_name.prop.pszVal; |
Check failure
Code scanning / CodeQL
Cast from char* to wchar_t*
There was a problem hiding this comment.
I'll commit a fix for these and merge
| auto device_name = no_null((LPWSTR) device_friendly_name.prop.pszVal); | ||
| auto device_description = no_null((LPWSTR) device_desc.prop.pszVal); | ||
| auto adapter_name = (LPWSTR) adapter_friendly_name.prop.pszVal; | ||
| auto device_name = (LPWSTR) device_friendly_name.prop.pszVal; |
Check failure
Code scanning / CodeQL
Cast from char* to wchar_t*
| auto device_description = no_null((LPWSTR) device_desc.prop.pszVal); | ||
| auto adapter_name = (LPWSTR) adapter_friendly_name.prop.pszVal; | ||
| auto device_name = (LPWSTR) device_friendly_name.prop.pszVal; | ||
| auto device_description = (LPWSTR) device_desc.prop.pszVal; |
Check failure
Code scanning / CodeQL
Cast from char* to wchar_t*
Float is the native format of opus codec and most if not all capture backends.
Co-authored-by: Cameron Gutman <aicommander@gmail.com>
Co-authored-by: Cameron Gutman <aicommander@gmail.com>
Description
Float is the native format of opus codec and most if not all capture backends.
Implementation progress:
Screenshot
Issues Fixed or Closed
Type of Change
.github/...)Checklist
Branch Updates
LizardByte requires that branches be up-to-date before merging. This means that after any PR is merged, this branch
must be updated before it can be merged. You must also
Allow edits from maintainers.