From 54ae9a64ace39c170da9ba1ea0ecf7ae6f51ff91 Mon Sep 17 00:00:00 2001 From: Robin Davies Date: Sat, 2 Nov 2024 08:59:24 -0400 Subject: [PATCH] Deal with legacay alsa drivers misreporting available channels. --- .vscode/launch.json | 2 +- CMakeLists.txt | 4 +- README.md | 4 +- docs/Installing.md | 6 +- docs/download.md | 4 +- docs/index.md | 4 +- src/AlsaDriver.cpp | 166 +++++++++++++++++++++++++++------------ src/AlsaDriverTest.cpp | 4 +- src/AvahiServiceTest.cpp | 22 +++--- src/PiPedalAlsaTest.cpp | 19 ++++- src/alsaCheck.cpp | 123 ++++++++++++++++++++++++++--- 11 files changed, 267 insertions(+), 91 deletions(-) diff --git a/.vscode/launch.json b/.vscode/launch.json index 4064db1..3a3272a 100644 --- a/.vscode/launch.json +++ b/.vscode/launch.json @@ -112,7 +112,7 @@ //"[json_variants]" // subtest of your choice, or none to run all of the tests. //"[inverting_mutex_test]" // "[utf8_to_utf32]" - "[wifi_channels_test]" + "[pipedal_alsa_test]" ], "stopAtEntry": false, diff --git a/CMakeLists.txt b/CMakeLists.txt index 8e9ed55..3e0509e 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -1,10 +1,10 @@ cmake_minimum_required(VERSION 3.16.0) project(pipedal - VERSION 1.3.64 + VERSION 1.3.65 DESCRIPTION "PiPedal Guitar Effect Pedal For Raspberry Pi" HOMEPAGE_URL "https://rerdavies.github.io/pipedal" ) -set (DISPLAY_VERSION "PiPedal v1.3.64-Experimental") +set (DISPLAY_VERSION "PiPedal v1.3.65-Experimental") set (PACKAGE_ARCHITECTURE "arm64") set (CMAKE_INSTALL_PREFIX "/usr/") diff --git a/README.md b/README.md index 5420e39..b9f2e9e 100644 --- a/README.md +++ b/README.md @@ -6,13 +6,13 @@ -Download: v1.3.64 +Download: v1.3.65 Website: [https://rerdavies.github.io/pipedal](https://rerdavies.github.io/pipedal). Documentation: [https://rerdavies.github.io/pipedal/Documentation.html](https://rerdavies.github.io/pipedal/Documentation.html).   -#### NEW version 1.3.64 Release, providing [snapshots](https://rerdavies.github.io/pipedal/Snaphots.html), and a new Performance View. See the [release notes](https://rerdavies.github.io/pipedal/ReleaseNotes) for details. +#### NEW version 1.3.65 Release, providing [snapshots](https://rerdavies.github.io/pipedal/Snaphots.html), and a new Performance View. See the [release notes](https://rerdavies.github.io/pipedal/ReleaseNotes) for details.   diff --git a/docs/Installing.md b/docs/Installing.md index 55478e2..0557401 100644 --- a/docs/Installing.md +++ b/docs/Installing.md @@ -13,17 +13,17 @@ page_icon: img/Install4.jpg Download the most recent Debian (.deb) package for your platform: -- [Raspberry Pi OS bookworm (64-bit) v1.3.64](https://github.com/rerdavies/pipedal/releases/download/) +- [Raspberry Pi OS bookworm (64-bit) v1.3.65](https://github.com/rerdavies/pipedal/releases/download/) - [Ubuntu/Raspberry Pi OS bullseyeye (64-bit) v1.2.31](https://github.com/rerdavies/pipedal/releases/download/v1.1.31/pipedal_1.1.31_arm64.deb) -Version 1.3.64 has not yet been tested on Ubuntu or Raspberry Pi OS bullseye. On these platforms, we recommend that you use version 1.1.31. +Version 1.3.65 has not yet been tested on Ubuntu or Raspberry Pi OS bullseye. On these platforms, we recommend that you use version 1.1.31. Install the package by running ``` sudo apt update cd ~/Downloads - sudo apt-get install pipedal_1.3.64_arm64.deb + sudo apt-get install pipedal_1.3.65_arm64.deb ``` Adjust accordingly if you have downloaded v1.1.31. diff --git a/docs/download.md b/docs/download.md index a3e16cb..7a873eb 100644 --- a/docs/download.md +++ b/docs/download.md @@ -4,7 +4,7 @@ Download the most recent Debian (.deb) package for your platform: -- Raspberry Pi OS Bookworm (64-bit) v1.3.64 +- Raspberry Pi OS Bookworm (64-bit) v1.3.65 Install the package by running @@ -12,7 +12,7 @@ Install the package by running ``` sudo apt update cd ~/Downloads - sudo apt-get install ./pipedal_1.3.64_arm64.deb + sudo apt-get install ./pipedal_1.3.65_arm64.deb ``` Follow the instructions in [_Configuring PiPedal After Installation_](https://rerdavies.github.io/pipedal/Configuring.html) to complete the installation. diff --git a/docs/index.md b/docs/index.md index 78ede05..1a2b054 100644 --- a/docs/index.md +++ b/docs/index.md @@ -1,7 +1,7 @@ -v1.3.64 +v1.3.65   @@ -9,7 +9,7 @@ To download PiPedal, click [here](download.md). To view PiPedal documentation, click [here](Documentation.md). -#### NEW version 1.3.64 Release. See the [release notes](https://rerdavies.github.io/pipedal/ReleaseNotes) for details. +#### NEW version 1.3.65 Release. See the [release notes](https://rerdavies.github.io/pipedal/ReleaseNotes) for details.   diff --git a/src/AlsaDriver.cpp b/src/AlsaDriver.cpp index 10932b5..03c6fa9 100644 --- a/src/AlsaDriver.cpp +++ b/src/AlsaDriver.cpp @@ -24,6 +24,7 @@ #include "pch.h" #include "util.hpp" +#include "Finally.hpp" #include #include #include "ss.hpp" @@ -66,7 +67,7 @@ namespace pipedal AudioFormat *formats, size_t nItems) { - snd_pcm_hw_params_t* test_params; + snd_pcm_hw_params_t *test_params; snd_pcm_hw_params_alloca(&test_params); for (size_t i = 0; i < nItems; ++i) @@ -197,15 +198,16 @@ namespace pipedal AudioDriverHost *driverHost = nullptr; - - void validate_capture_handle() { // leftover debugging for a buffer overrun :-/ - #ifdef DEBUG + void validate_capture_handle() + { // leftover debugging for a buffer overrun :-/ +#ifdef DEBUG if (snd_pcm_type(captureHandle) != SND_PCM_TYPE_HW) { throw std::runtime_error("Capture handle has been overwritten"); } - #endif +#endif } + public: AlsaDriverImpl(AudioDriverHost *driverHost) : driverHost(driverHost) @@ -377,35 +379,26 @@ namespace pipedal unsigned int channels_min = 0; err = snd_pcm_hw_params_get_channels_max(hwParams, &channels_max); - if (err < 0) { + if (err < 0) + { AlsaError(SS("Can't get channels_max.")); } err = snd_pcm_hw_params_get_channels_min(hwParams, &channels_min); - if (err < 0) { + if (err < 0) + { AlsaError(SS("Can't get channels_min.")); } - *channels = channels_max; - if (channels_max > 2 && channels_min <= 2 && channels_min > 0) { - unsigned int bestChannelConfig = 2; - - snd_pcm_hw_params_t* test_params; - snd_pcm_hw_params_alloca(&test_params); - snd_pcm_hw_params_copy(test_params, hwParams); - - if ((err = snd_pcm_hw_params_set_channels(handle, test_params, - bestChannelConfig)) >= 0) - { - *channels = bestChannelConfig; - } - + if (ShouldForceStereoChannels(handle, hwParams, channels_min, channels_max)) + { + *channels = 2; } - if (*channels > 1024) + if (*channels >= 1024) { // The default PCM device has unlimited channels. // report 2 channels @@ -1312,7 +1305,6 @@ namespace pipedal } } validate_capture_handle(); - } void recover_from_output_underrun(snd_pcm_t *capture_handle, snd_pcm_t *playback_handle, int err) { @@ -1398,7 +1390,6 @@ namespace pipedal } audioRunning = true; validate_capture_handle(); - } else { @@ -1427,7 +1418,6 @@ namespace pipedal // buffer boundaries (but discard them) snd_pcm_sframes_t framesRead; - auto state = snd_pcm_state(handle); auto frame_bytes = this->captureFrameSize; do @@ -1971,12 +1961,14 @@ namespace pipedal for (size_t i = 0; i < devices.size(); ++i) { - try { + try + { const auto &device = devices[i]; auto midiDevice = std::make_unique(); midiDevice->Open(device); midiDevices.push_back(std::move(midiDevice)); - } catch (const std::exception &e) + } + catch (const std::exception &e) { Lv2Log::error(e.what()); } @@ -2051,6 +2043,95 @@ namespace pipedal return new AlsaDriverImpl(driverHost); } + bool ShouldForceStereoChannels(snd_pcm_t *pcmHandle, snd_pcm_hw_params_t *hwParams, unsigned int channelsMin, unsigned int channelsMax) + { + // The problem: old IC2 drivers seem to return 1-8 channels, but 8 channels is non-functinal. The assumption is that legacy drivers + // (I2C drivers, particularl, but also the Rpi headphones device, as an interesting example) that don't support channel maps do this. + // Hypothetically, devices could allow slection of hardware-downmixed surround channels. So deal with this case defensively. + // The approach: check the channel map and do our best to interpret what we find. + // No channel map, or any part of the channel map is unknown? Probably the legalcy case we're interested in. Return TRUE + // If the channel map is a surround format, return true in that case as well. + // If the channel map is pairwise, return false! (legitimately multi-channel devices should not be forced to stereo). + // If the channel map is all FL/FR/MONO return false (a hypothetical configuration for a multi-channel device) + // If the channel map is not all FL/FR/MONO, assume that it's an upmixed/downmixed surround format, and return TRUE. + // This is high-risk code, because it attempts to anticipate hypothetical device configurations with no actual testing. + + if (channelsMax <= 2) + return false; + if (channelsMin == channelsMax) + return false; + if (channelsMin > 2) + return false; // can't imagine what sort of device this is. + + snd_pcm_hw_params_t *test_params; + + snd_pcm_hw_params_alloca(&test_params); + snd_pcm_hw_params_copy(test_params, hwParams); + + // can we select 2 channels? + if (snd_pcm_hw_params_set_channels(pcmHandle, test_params, (unsigned int)2) >= 0) + { + snd_pcm_chmap_query_t **chmaps = snd_pcm_query_chmaps(pcmHandle); + + if (chmaps == nullptr) + { + return true; // probably an old driver. Do it. + } + Finally ff([chmaps]() + { snd_pcm_free_chmaps(chmaps); }); + for (size_t i = 0; chmaps[i] != nullptr; ++i) + { + snd_pcm_chmap_query_t *chmap = chmaps[i]; + if (chmap->map.channels == channelsMax) + { + switch (chmap->type) + { + case SND_CHMAP_TYPE_NONE: + default: + return true; // weird legacy case? Do it. + case SND_CHMAP_TYPE_PAIRED: + return false; // A legitimate multi-channel device. definitely don't do it. + + case SND_CHMAP_TYPE_VAR: + case SND_CHMAP_TYPE_FIXED: + { + // we should do it for surround formats. guard against other hypothetical mappings for legitimately multi-channel devices. + + snd_pcm_chmap_position pos0 = (snd_pcm_chmap_position)(chmap->map.pos[0]); + if (pos0 == snd_pcm_chmap_position::SND_CHMAP_MONO) // hypothetical channel map of all mono channesl. + { + return false; // don't do it. + } + if (pos0 != snd_pcm_chmap_position::SND_CHMAP_FL && pos0 != snd_pcm_chmap_position::SND_CHMAP_FL) // surround formats always start with FL. Hypothetical quad formats could start with FC. + { + return false; // don't do it. + } + // accept a hypothetical channel map of mixed FL's and FR's, FC's and MONOs. (Multi-channel with mixed mono and stereo pairs). + // But otherwise assume it's a surround map, and use a stereo channel configuration instead. + for (size_t i = 0; i < chmap->map.channels; ++i) + { + snd_pcm_chmap_position pos = (snd_pcm_chmap_position)(chmap->map.pos[i]); + switch (pos) + { + case snd_pcm_chmap_position::SND_CHMAP_MONO: + case snd_pcm_chmap_position::SND_CHMAP_FL: + case snd_pcm_chmap_position::SND_CHMAP_FR: + case snd_pcm_chmap_position::SND_CHMAP_FC: + break; // keep going. + default: + return true; // probably a surround sound map. + } + } + return false; + }; + } + } + } + return true; // no matching channel map(!??). nonsensical case. may as well use the stereo config, which might be more sensible. + } + return false; + } + bool GetAlsaChannels(const JackServerSettings &jackServerSettings, std::vector &inputAudioPorts, std::vector &outputAudioPorts) @@ -2058,7 +2139,7 @@ namespace pipedal if (jackServerSettings.IsDummyAudioDevice()) { auto nChannels = GetDummyAudioChannels(jackServerSettings.GetAlsaInputDevice()); - + inputAudioPorts.clear(); outputAudioPorts.clear(); for (uint32_t i = 0; i < nChannels; ++i) @@ -2149,17 +2230,9 @@ namespace pipedal { throw PiPedalLogicException("No outut channels."); } - if (playbackChannels > 2 && channelsMin <=2 && channelsMin > 0) + if (ShouldForceStereoChannels(playbackHandle, playbackHwParams, channelsMin, playbackChannels)) { - snd_pcm_hw_params_t* test_params; - snd_pcm_hw_params_alloca(&test_params); - snd_pcm_hw_params_copy(test_params, playbackHwParams); - - - if (snd_pcm_hw_params_set_channels(playbackHandle,test_params,(unsigned int)2) >= 0) - { - playbackChannels = 2; - } + playbackChannels = 2; } err = snd_pcm_hw_params_get_channels_max(captureHwParams, &captureChannels); @@ -2167,22 +2240,13 @@ namespace pipedal { throw PiPedalLogicException("No input channels."); } - err = snd_pcm_hw_params_get_channels_min(captureHwParams,&channelsMin); + err = snd_pcm_hw_params_get_channels_min(captureHwParams, &channelsMin); if (err >= 0) { - if (captureChannels > 2 && channelsMin <= 2 && channelsMin > 0) + if (ShouldForceStereoChannels(captureHandle, captureHwParams, channelsMin, captureChannels)) { - snd_pcm_hw_params_t* test_params; - snd_pcm_hw_params_alloca(&test_params); - snd_pcm_hw_params_copy(test_params, captureHwParams); - - - if (snd_pcm_hw_params_set_channels(captureHandle,test_params,(unsigned int)2) >= 0) - { - captureChannels = 2; - } - - } + captureChannels = 2; + } } inputAudioPorts.clear(); diff --git a/src/AlsaDriverTest.cpp b/src/AlsaDriverTest.cpp index 827b6b6..8c3e4c2 100644 --- a/src/AlsaDriverTest.cpp +++ b/src/AlsaDriverTest.cpp @@ -223,11 +223,11 @@ public: for (size_t i = 0; i < inputs; ++i) { - inputBuffers[i] = audioDriver->GetInputBuffer(i,nFrames); + inputBuffers[i] = audioDriver->GetInputBuffer(i); } for (size_t i = 0; i < outputs; ++i) { - outputBuffers[i] = audioDriver->GetOutputBuffer(i,nFrames); + outputBuffers[i] = audioDriver->GetOutputBuffer(i); } if (this->testType == TestType::Oscillator) { diff --git a/src/AvahiServiceTest.cpp b/src/AvahiServiceTest.cpp index 6cec4da..be76579 100644 --- a/src/AvahiServiceTest.cpp +++ b/src/AvahiServiceTest.cpp @@ -30,18 +30,18 @@ using namespace pipedal; -TEST_CASE("Avahi Service Test", "[avahi_service][dev]") -{ +// TEST_CASE("Avahi Service Test", "[avahi_service][dev]") +// { - { - AvahiService service; +// { +// AvahiService service; - service.Announce(81, "Test Announcement", "0a6045b0-1753-4104-b3e4-b9713b9cc358","pipedal"); +// service.Announce(81, "Test Announcement", "0a6045b0-1753-4104-b3e4-b9713b9cc358","pipedal"); - sleep(10); +// sleep(10); - service.Unannounce(); - service.Announce(81, "Test Announcement 2", "0a6045b0-1753-4104-b3e4-b9713b9cc358","pipedal"); - sleep(10); - } -} \ No newline at end of file +// service.Unannounce(); +// service.Announce(81, "Test Announcement 2", "0a6045b0-1753-4104-b3e4-b9713b9cc358","pipedal"); +// sleep(10); +// } +// } \ No newline at end of file diff --git a/src/PiPedalAlsaTest.cpp b/src/PiPedalAlsaTest.cpp index 17f2db9..e7cf5d9 100644 --- a/src/PiPedalAlsaTest.cpp +++ b/src/PiPedalAlsaTest.cpp @@ -27,9 +27,11 @@ #include "PiPedalAlsa.hpp" using namespace pipedal; +using namespace std; -TEST_CASE( "ALSA Test", "[pipedal_alsa_test][Build][Dev]" ) { - +static void DiscoveryTest() +{ + cout << "--- Discovery" << endl; PiPedalAlsaDevices devices; auto result = devices.GetAlsaDevices(); std::cout << result.size() << " ALSA devices found." << std::endl; @@ -42,5 +44,18 @@ TEST_CASE( "ALSA Test", "[pipedal_alsa_test][Build][Dev]" ) { } +void ChannelConfigtest() +{ + cout << "--- Channel Config Test" << endl; + +} + +TEST_CASE( "ALSA Test", "[pipedal_alsa_test][Build][Dev]" ) { + DiscoveryTest(); + + ChannelConfigTest(); + +} + diff --git a/src/alsaCheck.cpp b/src/alsaCheck.cpp index e19670a..e2147e9 100644 --- a/src/alsaCheck.cpp +++ b/src/alsaCheck.cpp @@ -32,6 +32,83 @@ #include #include "Finally.hpp" +using namespace pipedal; + +static std::string GetChannelDescription( + snd_pcm_t* handle, + snd_pcm_hw_params_t*hwParams, + unsigned int channels) +{ + snd_pcm_chmap_query_t **chmaps = snd_pcm_query_chmaps(handle); + + if (chmaps) { + Finally ff([chmaps] { + snd_pcm_free_chmaps(chmaps); + }); + std::stringstream ss; + bool firstMap = true; + for (size_t i = 0; chmaps[i] != nullptr; ++i) + { + auto chmap = chmaps[i]; + if (chmap->map.channels == channels) + { + if (!firstMap) { + ss << "/"; + } + ss << channels << " "; + firstMap = false; + switch (chmap->type) + { + case SND_CHMAP_TYPE_NONE: + default: + ss << "Unknown"; + break; + case SND_CHMAP_TYPE_VAR: + case SND_CHMAP_TYPE_FIXED: + for (size_t ch = 0; ch < chmap->map.channels; ++ch) + { + if (ch != 0) + { + ss << " "; + } + snd_pcm_chmap_position pos =(snd_pcm_chmap_position)(chmap->map.pos[ch]); + if (pos == snd_pcm_chmap_position::SND_CHMAP_MONO) + { + ss << "ch" << (i+1); + } + ss << snd_pcm_chmap_name((snd_pcm_chmap_position)(chmap->map.pos[ch])); + } + break; + case SND_CHMAP_TYPE_PAIRED: + for (size_t pair = 0; pair < chmap->map.channels/2; ++pair) + { + if (pair != 0) + { + ss << " "; + } + ss << "L" << (pair+1) << " R" << (pair+1); + } + break; + } + if (chmap->type == SND_CHMAP_TYPE_VAR) + { + ss << " (var)"; + } + } + } + return ss.str(); + } + std::string config_name; + switch (channels) { + case 1: config_name = "Mono"; break; + case 2: config_name = "Stereo"; break; + case 4: config_name = "Quadraphonic"; break; + case 6: config_name = "5.1 Surround"; break; + case 8: config_name = "7.1 Surround"; break; + default: config_name = std::to_string(channels) + " channels"; + } + return config_name; +} void pipedal::check_alsa_channel_configs(const char* device_name) { @@ -81,16 +158,9 @@ void pipedal::check_alsa_channel_configs(const char* device_name) { snd_pcm_hw_params_copy(test_params, hw_params); if (snd_pcm_hw_params_set_channels(handle, test_params, channels) == 0) { - std::string config_name; - switch (channels) { - case 1: config_name = "Mono"; break; - case 2: config_name = "Stereo"; break; - case 4: config_name = "Quadraphonic"; break; - case 6: config_name = "5.1 Surround"; break; - case 8: config_name = "7.1 Surround"; break; - default: config_name = std::to_string(channels) + " channels"; - } - std::cout << " " << channels << " channels (" << config_name << ")" << std::endl; + std::string channel_description = GetChannelDescription(handle,test_params,channels); + + std::cout << " " << channels << " channels (" << channel_description << ")" << std::endl; } } } @@ -98,6 +168,15 @@ void pipedal::check_alsa_channel_configs(const char* device_name) { } +static bool hasChannelMaps(snd_pcm_t* pcm_handle) { + snd_pcm_chmap_query_t **chmaps = snd_pcm_query_chmaps(pcm_handle); + if (chmaps) { + snd_pcm_free_chmaps(chmaps); + return true; + } + return false; +} + static void print_device_info(snd_ctl_t* handle, int card, int device) { snd_pcm_info_t* pcminfo; snd_pcm_info_alloca(&pcminfo); @@ -140,8 +219,8 @@ static void print_device_info(snd_ctl_t* handle, int card, int device) { snd_pcm_hw_params_alloca(&hw_params); if (snd_pcm_hw_params_any(pcm_handle, hw_params) >= 0) { - unsigned int min_channels, max_channels; - unsigned int min_rate, max_rate; + unsigned int min_channels = 1024, max_channels = 1024; + unsigned int min_rate = 1, max_rate = 1; snd_pcm_hw_params_get_channels_min(hw_params, &min_channels); snd_pcm_hw_params_get_channels_max(hw_params, &max_channels); @@ -159,10 +238,28 @@ static void print_device_info(snd_ctl_t* handle, int card, int device) { } } std::cout << std::endl; + std::cout << " Channel maps:" << std::endl; + snd_pcm_hw_params_t* test_params; + snd_pcm_hw_params_alloca(&test_params); + + if (!hasChannelMaps(pcm_handle)) + { + std::cout << " No channel maps." << std::endl; + } else { + for (size_t i = min_channels; i <= max_channels; ++i) + { + snd_pcm_hw_params_copy(test_params, hw_params); + + if (snd_pcm_hw_params_set_channels(pcm_handle, test_params, i) == 0) { + + std::cout << " " << GetChannelDescription(pcm_handle,hw_params,i) << std::endl; + } + } + } } snd_pcm_close(pcm_handle); } else { - std::cout << " (device in use)" << std::endl; + std::cout << " (device not available)" << std::endl; } } }