Skip Digital Only Windows devices - #12
Andreas-Froyland wants to merge 1 commit into
Conversation
|
@RubenLarsen this should be ready |
|
@cubic-dev-ai review this |
@Andreas-Froyland I have started the AI code review. It will take a few minutes to complete. |
There was a problem hiding this comment.
3 issues found across 1 file
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/lib.rs">
<violation number="1" location="src/lib.rs:164">
P2: Each call now leaks the COM-allocated device ID returned by `IMMDevice::GetId`. Convert the ID, call `CoTaskMemFree` on the `PWSTR`, then handle the conversion result.</violation>
<violation number="2" location="src/lib.rs:194">
P2: This filter only affects application-session discovery. `GetDefaultAudioEnpointVolumeControl` and `GetAllAudioDevices` still add digital endpoints, so the public volume-control session list continues exposing the devices this change intends to skip.</violation>
<violation number="3" location="src/lib.rs:195">
P2: Digital-only `DigitalPassthrough` and `HDMI` endpoints are not skipped because this condition recognizes only SPDIF. Match all digital-only form-factor values before enumerating sessions.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
| let device_id = match device.GetId() { | ||
| Ok(id) => match id.to_string() { | ||
| Ok(id_str) => id_str, | ||
| Err(_) => { | ||
| eprintln!("WARNING: Skipping device {} - couldn't convert device ID to string", device_index); | ||
| error!("WARNING: Skipping device {} - couldn't convert device ID to string", device_index); | ||
| continue; | ||
| } | ||
| }, | ||
| Err(err) => { | ||
| eprintln!("WARNING: Skipping device {} - couldn't get device ID: {}", device_index, err); | ||
| error!("WARNING: Skipping device {} - couldn't get device ID: {}", device_index, err); | ||
| continue; | ||
| } | ||
| }; |
There was a problem hiding this comment.
P2: Each call now leaks the COM-allocated device ID returned by IMMDevice::GetId. Convert the ID, call CoTaskMemFree on the PWSTR, then handle the conversion result.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/lib.rs, line 164:
<comment>Each call now leaks the COM-allocated device ID returned by `IMMDevice::GetId`. Convert the ID, call `CoTaskMemFree` on the `PWSTR`, then handle the conversion result.</comment>
<file context>
@@ -136,36 +151,93 @@ impl AudioController {
+ };
+
+ // Get device ID for error reporting
+ let device_id = match device.GetId() {
+ Ok(id) => match id.to_string() {
+ Ok(id_str) => id_str,
</file context>
| let device_id = match device.GetId() { | |
| Ok(id) => match id.to_string() { | |
| Ok(id_str) => id_str, | |
| Err(_) => { | |
| eprintln!("WARNING: Skipping device {} - couldn't convert device ID to string", device_index); | |
| error!("WARNING: Skipping device {} - couldn't convert device ID to string", device_index); | |
| continue; | |
| } | |
| }, | |
| Err(err) => { | |
| eprintln!("WARNING: Skipping device {} - couldn't get device ID: {}", device_index, err); | |
| error!("WARNING: Skipping device {} - couldn't get device ID: {}", device_index, err); | |
| continue; | |
| } | |
| }; | |
| let device_id = match device.GetId() { | |
| Ok(id) => { | |
| let id_result = id.to_string(); | |
| CoTaskMemFree(Some(id.as_ptr().cast())); | |
| match id_result { | |
| Ok(id_str) => id_str, | |
| Err(_) => { | |
| eprintln!("WARNING: Skipping device {} - couldn't convert device ID to string", device_index); | |
| error!("WARNING: Skipping device {} - couldn't convert device ID to string", device_index); | |
| continue; | |
| } | |
| } | |
| }, | |
| Err(err) => { | |
| eprintln!("WARNING: Skipping device {} - couldn't get device ID: {}", device_index, err); | |
| error!("WARNING: Skipping device {} - couldn't get device ID: {}", device_index, err); | |
| continue; | |
| } | |
| }; |
| let device_name = get_device_friendly_name(&device, &format!("Device {}", device_index)); | ||
|
|
||
| // Skip SPDIF/digital outputs if detected by form factor | ||
| if let Some(form_factor) = get_device_form_factor(&property_store) { |
There was a problem hiding this comment.
P2: This filter only affects application-session discovery. GetDefaultAudioEnpointVolumeControl and GetAllAudioDevices still add digital endpoints, so the public volume-control session list continues exposing the devices this change intends to skip.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/lib.rs, line 194:
<comment>This filter only affects application-session discovery. `GetDefaultAudioEnpointVolumeControl` and `GetAllAudioDevices` still add digital endpoints, so the public volume-control session list continues exposing the devices this change intends to skip.</comment>
<file context>
@@ -136,36 +151,93 @@ impl AudioController {
+ let device_name = get_device_friendly_name(&device, &format!("Device {}", device_index));
+
+ // Skip SPDIF/digital outputs if detected by form factor
+ if let Some(form_factor) = get_device_form_factor(&property_store) {
+ if form_factor == FORM_FACTOR_SPDIF {
+ continue;
</file context>
|
|
||
| // Skip SPDIF/digital outputs if detected by form factor | ||
| if let Some(form_factor) = get_device_form_factor(&property_store) { | ||
| if form_factor == FORM_FACTOR_SPDIF { |
There was a problem hiding this comment.
P2: Digital-only DigitalPassthrough and HDMI endpoints are not skipped because this condition recognizes only SPDIF. Match all digital-only form-factor values before enumerating sessions.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/lib.rs, line 195:
<comment>Digital-only `DigitalPassthrough` and `HDMI` endpoints are not skipped because this condition recognizes only SPDIF. Match all digital-only form-factor values before enumerating sessions.</comment>
<file context>
@@ -136,36 +151,93 @@ impl AudioController {
+
+ // Skip SPDIF/digital outputs if detected by form factor
+ if let Some(form_factor) = get_device_form_factor(&property_store) {
+ if form_factor == FORM_FACTOR_SPDIF {
+ continue;
+ }
</file context>
| if form_factor == FORM_FACTOR_SPDIF { | |
| if form_factor == FORM_FACTOR_SPDIF || matches!(form_factor, 7 | 9 | 10) { |
Skip digital only devices as they are not needed in the context of volume control
It also now skips devices on an error with fetching device sessions instead of crashing