From the callback API pattern, theoretically it's possible to get
multiple events at the same time. We used to ignore
`kAudioDevicePropertyDeviceIsAlive` event and do nothing when the device
is system input without checking if we have other events at that moment
or not. If we do have other events, we should run other event handler
rather than do nothing in this case.
`test_register_device_changed_callback_to_check_default_device_changed_duplex`
fails when we have a single input device and many output devices.
In the test code, we try to switch the input device but we don't have
any available device at all so the test hangs.
We should not switch the input device for the duplex stream if we only
have one input device.
We might create a custom cubeb-private aggregate device as our
input-output device when creating a duplex stream. The lifetime of this
aggregate device follows the lifetime of the created duplex stream.
We should not expose it in our test-only device-list getter API.
Otherwise, if we set it to another stream's input or output device, that
aggregate device may die before we finish the test for another stream.
We've done the same thing in our cubeb API. We should do the same for
the test-only API as well.
If the stream is output-only, or its input device is system-default, its
reinit failure can lead to hitting an assertion in set_volume since the
program keeps running as the error doesn't occur. We should return an
error instead.
* Add comments explaining the purpose of the test
* Revise test for the custom group
This solves https://github.com/mozilla/cubeb-coreaudio-rs/issues/138.
When the Blackhole device is installed, both its input and output data
source are the same. As a result, the hash-map whose key is data source
value has collision when the hash-map is used to store both input and
output device's group ids.
To avoid the collision, we should use different hash-map for input
device and output device to store the group id values.
* BMO 1721496 - Append silence frames to when switching from A2DP to HFP/HSP
No silence frame is fed to resampler when the duplex stream runs on a
bluetooth when the device is switchiing from A2DP to HFP/HSP profile,
which leads to hit an assertion [1] within cubeb resampler.
When the duplex cubeb stream runs on a bluetooth device and the device
is switching from A2DP to HFP/HSP profile, the `input_buffer_manager`
has no input data to be appended [2] in `audiounit_input_callback`. And
then when `audiounit_input_callback` runs, the `buffered_input_frames`
is `0` but `input_frames_needed` is a positive integer greater than `0`.
However, the `input_frames` in this case is equal to
`buffered_input_frames` since neither `switching_device` nor
`frames_read > 0` is `true`.
When the above case happens in the first `audiounit_output_callback`
call, it makes the cubeb resampler hit the assertion in [1]. The `data_`
[3] in the `internal_input_buffer` [4] is `nullptr` when the resampler
is created (`passthrough_resampler` in my case). If the `length` in the
first `push*` [5] is `0`, then the `reserve` won't be called so `data_`
remains `nullptr`, which makes the resampler hit the assertion in [1]
We should append the silence frames to the resampler in this case not
only to avoid hitting the assertion but also to append the silence
frames correctly when we need.
[1] b608f59024/src/cubeb_utils.h (L31)
[2] 8c9bb1e745/src/backend/mod.rs (L409)
[3] b608f59024/src/cubeb_utils.h (L115)
[4] b608f59024/src/cubeb_resampler_internal.h (L112)
[5] b608f59024/src/cubeb_utils.h (L196)
[6] b608f59024/src/cubeb_utils.h (L172)
* Remove unnecessary checks
* Correct frames_read
`frames_read` is added in `audiounit_input_callback` with K frames but
it can be added by K+N frames again when `input_frame_needed` in
`audiounit_output_callback is K+N, where K is non-negative integer and N
is positive integer. The N is the number of the silence frames we add in
the input buffer when input_buffer_manager doesn't have enough frames
resampler needs. The `frames_read` should be added by N, which is
silence frames only, rather than K+N in this case.
* Make `frames_read` always follow input_buffer_manager
We always use `input_buffer_manager.get_linear_data(...)` to get the
input buffer with required frames, so `frames_read` should be the sum of
the required frames.
* Set github-action running tests with Rust stable
* Set github-action running tests with Rust nightly
* Skip some sanitizers
* Setup github-action
Add a Github-action setting to run tests with Rust stable and nightly. This setting installs the Rust manually instead of using actions-rs since external actions are generally not available inside the Mozilla GitHub org for security reasons.
* Remove nightly.yml and stable.yml
stable.yml and stable.yml can be replaced by test.yml
* Remove MATRIX_RUST
When we create/add/plug a new device to Mac OS, the newly added device
is possible to be the new default device (depends on system setting?).
In `test_unplug_a_device_on_an_active_stream`, we didn't check if the
default device is the expected one. The created device in that function
could be the default device in some tests, and it could be non-default
device in some tests. We need to check if the created device is default
or non-default before running the tests. Otherwise we may use the wrong
device to run the tests.
* Manual test: Add a command to set stream volume
* Test case - `assertion failed: !unit.is_null()` in `set_volume`
The following commit message demonstrating a possible cause of
the assertion-failed issue in [BMO 1692910](https://bugzilla.mozilla.org/show_bug.cgi?id=1692910)
This is a data-racing issue. The assertion will be hit when the
1. `set_volume` is called on thread A at the same time when the stream is
being reinitialized at thread B
2. `set_volume` is called on thread A just after the
`self.core_stream_data.close()` is called on the thread B, which will
uninitialize and dispose the AudioUnit called in `set_volume` on
thread A
This commit adds some delay to give us a room to call the
`AudioUnitStream.set_volume` in the manual test, `test_stream_tester`,
after the `AudioUnitStream.stream.core_stream_data.close()` is called
when the `AudioUnitStream` is being reinitialized
The following log illustrates the steps to hit that assertion in
`AudioUnitStream.set_volume` on purpose:
```
% cargo test test_stream_tester -- --ignored --nocapture
Compiling cubeb-coreaudio v0.1.0 (/Users/cchang/Work/cubeb-coreaudio-rs)
Finished test [unoptimized + debuginfo] target(s) in 3.24s
Running target/debug/deps/cubeb_coreaudio-29d6ac8d8020965d
running 1 test
commands:
'q': quit
'c': create a stream
'd': destroy a stream
's': start the created stream
't': stop the created stream
'r': register a device changed callback
'v': set volume
c
Select stream type:
1) Input 2) Output 3) In-Out Duplex 4) Back
2
Stream 0x7f86967160f0 created.
commands:
'q': quit
'c': create a stream
'd': destroy a stream
's': start the created stream
't': stop the created stream
'r': register a device changed callback
'v': set volume
s
state: Started
Stream 0x7f86967160f0 started.
commands:
'q': quit
'c': create a stream
'd': destroy a stream
's': start the created stream
't': stop the created stream
'r': register a device changed callback
'v': set volume
*******************************************************************
* Switch the audio device here to trigger stream reinitializatoin *
*******************************************************************
1 Reinit > Sleep for 5s
v
thread 'backend::tests::manual::test_stream_tester' panicked at 'assertion failed: !unit.is_null()', src/backend/mod.rs:268:5
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
test backend::tests::manual::test_stream_tester ... FAILED
failures:
failures:
backend::tests::manual::test_stream_tester
test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 182 filtered out
```
* Move `set_volume` to the task-queue running stream reinitialization
One simple way to avoid the data-racing issue mentioned in the previous
commit is to dispatch the `set_volume` task to the same task-queue that
runs the stream reinitialization tasks (it executes the stream
destroying task as well). That is, if `set_volume` task is executed
before stream reinitialization, the volume will be set before the stream
is reinitialized. If the `set_volume` task is executed after stream
reinitialization, the volume will be set after the stream finishes its
reinitialization task.
* Revert "Test case - `assertion failed: !unit.is_null()` in `set_volume`"
This reverts commit 8c1e3f91bd.
* Update README
* Create a custom Error enum for AggregateDevice
* Rename variables whose type is OSStatus
* Bail out if the aggregate device contains less than 2 sub devices
This works around the crash in [BMO 1677766](https://bugzilla.mozilla.org/show_bug.cgi?id=1677766).
It's not clear why the number of the owning sub-devices in the created
aggregate device is less than 2, after setting sub devices from input
and output sides via `AggregateDevice::set_sub_devices_sync`. The
sub-devices should be set synchronously when the above function is
called. Maybe `kAudioObjectPropertyOwnedObjects` is not synchronous or
symmetric to `kAudioAggregateDevicePropertyFullSubDeviceList`, which is
the property set in `AggregateDevice::set_sub_devices_sync`.
If the sub devices are less than 2 (we should have at least one input
and one output) in `AggregateDevice::activate_clock_drift_compensation`,
the `AggregateDevice::new` should bail out with an error returned. Then
the cubeb stream can follow the fallback mechanism to create a duplex
stream without using an aggregate-device.
* Correct LessDevicesThan2 to LessThan2Devices
* Use default channel layout if layout have non silent duplicate channels
This solves the [BMO 1675719](https://bugzilla.mozilla.org/show_bug.cgi?id=1675719).
The channel layout set in the output device might have non silent
duplicate channels (e.g., having two Front-Right in the channels), which
is not a valid case of our audio mixer usages. It will lead to a panic in
our code. The Audio MIDI Setup on MacOS disallows setting duplicate
channels within a channel layout. It will throw a "Overlapping Channels"
error in that case. It implies that the "overlapping channels" setting
is not defined by the user. It's likely to be defined in the firmware
instead. Since the user doesn't ask it explicitly, it's not necessary to
accept this kind of layout that is forbidden on Mac OS by default.
When there are duplicate non-silent channel in the output channel
layout, we can ignore this invalid channel layout and apply a standard
SMPTE layout instead.
* Rename
This patch works around a crash we get in BMO 1658982 [1].
When creating the aggregate device, somehow we will get
`kAudioHardwareBadObjectError` error during `set_sub_devices_sync` step.
This error occurs when removing the device-changed listeners for the
newly created aggregate device. This error implies the aggregate device
is somehow dead. In this case, it's ok to not remove the listener
manually since the listener should receive nothing from a dead device.
We can just return error to indicate the aggregate device isn't
initialized successfully.
We should return the error if sub devices cannot be set. However, this
patch only return the error when timeout of setting the sub devices. The
reason to do so is to investigate when `kAudioHardwareBadObjectError` is
thrown.
[1] https://bugzilla.mozilla.org/show_bug.cgi?id=1658982
Chun-Min Chang
Paul Adenot
Matthew Gregan
Markus Stange