Closed Bug 1530715 Opened 9 months ago Closed 4 months ago

Implement CoreAudio backend for Cubeb in Rust

Categories

(Core :: Audio/Video: cubeb, enhancement, P2)

enhancement

Tracking

()

RESOLVED FIXED
mozilla70
Tracking Status
firefox70 --- fixed

People

(Reporter: chunmin, Assigned: chunmin)

References

(Blocks 3 open bugs, Regression)

Details

Attachments

(46 files, 3 obsolete files)

50 bytes, text/plain
Details
3.03 KB, patch
Details | Diff | Splinter Review
10.32 KB, patch
Details | Diff | Splinter Review
2.58 KB, patch
Details | Diff | Splinter Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
Attached file repo on github

The project is currently hosted on github. The status is updated in README document there. Sync here for tracking bugs.

Assignee: nobody → cchang
Blocks: 1530713

Running tests in cubeb:

  1. Build cubeb by the instructions
  2. git clone cubeb-coreaudio-rs under cubeb/src
  3. Apply this patch in cubeb
    • it adds build configurations in CMakeLists.txt
    • and a c-to-rust interface in cubeb.c
  4. Goto cubeb-build and run
    • $ make clean
    • $ cmake --build .
    • $ ctest
Attachment #9046734 - Attachment description: cubeb config to build the coreaudio backend in rust → Build within cubeb

This patch will create a folder named cubeb-coreaudio-rs under <gecko-source>/media/libcubeb, which will be a mirror of the cubeb-coreaudio-rs repo. To import the cubeb-coreaudio-rs repo into gecko, run $ sh update.sh <upstream_src_directory>, where the <upstream_src_directory> is the path of the cubeb-coreaudio-rs repo. The update.sh just simply copies the source code from the repo to <path-to-gecko-libcubeb>/cubeb-coreaudio-rs. <path-to-gecko-libcubeb>/cubeb-coreaudio-rs will be one of the paths of the rust libraries. It will be linked on the next patch.

Another way to do this is to clone the repo directly under <gecko-source>/media/libcubeb, if it is just for testing only.

This patch will build the cubeb-coreaudio-rs as the default cubeb backend on macOS. cubeb-coreaudio-rs will be one of the dependencies of the gkrust-shared library and be set as the default cubeb backend on macOS in cubeb.c

Attachment #9046753 - Attachment description: [build within gecko] part 1: Build settings → [build within gecko] part 2: Build settings
Rank: 15
Priority: -- → P2

Building cubeb-coreaudio-rs within gecko

Steps

  1. cd to the gecko source code folder
  2. Import cubeb-coreaudio-rs by
  3. Build cubeb-coreaudio-rs within gkrust-shared and call it via cubeb.c
  4. Update gkrust-shared and load dependencies of cubeb-coreaudio-rs into third_party/rust/
    • Run $ cargo update -p gkrust-shared && ./mach vendor rust --ignore-modified, under <gecko-source>
  5. Build gecko
    • Run $ ./mach build

Notes

changes:

  • Use dummy git commit id and date as the initial values
Attachment #9046751 - Attachment is obsolete: true

changes:

  • fix some build issues on try server
  • set USE_AUDIOUNIT_RUST only when MOZ_AUDIOUNIT_RUST is set

note:
The rust version will be set as the default backend. It should be deprioritized when it's landed and be set as the default backend when the media.cubeb.backend is set to "audiounit-rust"

Attachment #9046753 - Attachment is obsolete: true

changes: Update update.sh

  • Clone all files except tests files
  • Apply patches for gecko automatically

note

  • The test files are verbose and unstructured. Omit them for now since they won't be run on tryserver.
  • The patch applied is to remove the test code(#[cfg(test)])
Attachment #9048724 - Attachment is obsolete: true

To import cubeb-coreaudio-rs crate later, creating a dummy folder in
advance in libcubeb. It should be the mirrow of cubeb-coreaudio-rs crate
on github. The information of cubeb-coreaudio-rs is in README_MOZILLA.

Attachment #9050852 - Attachment description: Bug 1530715 - P1: Create a dummy crate in libcubeb. r?kinetik → Bug 1530715 - P1: Create an empty cubeb-coreaudio-rs crate in libcubeb. r=kinetik
Attachment #9050856 - Attachment description: Bug 1530715 - P3: Build cubeb-coreaudio-rs in libcubeb. r?kinetik → Bug 1530715 - P3: Build cubeb-coreaudio-rs in libcubeb. r=kinetik
Attachment #9050857 - Attachment description: Bug 1530715 - P4: Vendor Rust dependencies for cubeb-coreaudio-rs. r?kinetik → Bug 1530715 - P4: Vendor Rust dependencies for cubeb-coreaudio-rs. r=kinetik

In the current implementation, we assume the aggregate device is created
immediately after calling the system API to create it, and we also
assume the sub-devices of the aggregate device are added immediately
upon we call the system API to set the sub-devices. Unfortunately, these
assumptions are not correct. Occasionally these settings are not
executed immediately, especially when they are not called on the main
thread (e.g., we may create an aggregate device off the main thread when
switching devices). Using the listeners monitoring the devices-changed
events can make sure those settings are done.

The counting of the active streams in a cubeb context is calculated in a
critical section created by locking our own custom mutex. To replace our
own mutex by standard Rust mutex, the critical section calculating the
active streams should be created by locking a standard Rust mutex. This
can be done by simply putting the active_streams to a struct with a Rust
mutex.

Depends on D29978

The global latency frames in the cubeb context is the latency frames of
the first cubeb stream created in that cubeb context. The reason is
because latency frames cannot be changed when there is an active stream
operating in parallel. To make sure the latency won't be changed, the
latency frames of the streams after the first stream will be overwritten
by the global latency frames. Since we always use the global latency
frames, instead of overwritting latency frames of the later streams, we
could, we could simply initialize all the streams with the global
latency frames.

Depends on D29979

The registration, unregistration, and notification for the device
collection changed is done in a critical section created by locking our
own custom mutex. To replace our own custom mutex by standard Rust
mutex, the critical section should be created by locking a standard Rust
mutex. This can be done by simply putting those varaibles for
device-collection-changed to a struct within a Rust mutex.

Depends on D29980

The mutex is considered poisoned whenever a thread panics while holding
the mutex. Registering a second device-collection-changed callback
without unregistering the original callback makes the thread panics while
holding the mutex for the device-collection-changed variables. This
mutex will be used when the cubeb context is dropped. If the mutex is
poisoned, we will get another panic when dropping the cubeb context
because of using a poisoned mutex.

Depends on D29981

The OwnedCriticalSection is a custom mutex directly translated from the
C code of the Cubeb library. Replacing the custom mutex by standard Rust
mutex make the code fit in the Rust design better and make debugging the
code easier. The variables protected by the custom mutex in the cubeb
context is moved to the Rust standard mutex in previous patches. The
custom mutex in the cubeb context is able to be removed since we no
longer rely on it.

Depends on D29982

Attachment #9062835 - Attachment description: Bug 1530715 - P10: Avoid poisoning the mutex for device-collection-changed. r?padenot → Bug 1530715 - P10: Avoid poisoning the mutex for device-collection-changed. r=padenot
Attachment #9062836 - Attachment description: Bug 1530715 - P11: Remove OwnedCriticalSection in cubeb context. r?padenot → Bug 1530715 - P11: Remove OwnedCriticalSection in cubeb context. r=padenot

Removing expected_output_callbacks_in_a_row since it is not being used.

When the stream is re-initialized, the input unit and/or output unit
should be reset if they are currently used, no matter what scope (input
or output or in-output) the parameter requests.

Depends on D34045

The code will be clearer if we can wrap the state-callback that notify
the state changed into a function. It will hide some details about
rendering the callback to C interface.

Depends on D34046

Setting variable by the Result value directly is rustier than setting
the variable by a mutual reference with a dummy Result that only
contains the error.

Depends on D34047

By narrowing down the parameters that set_volume API needs, we can avoid
the unnecessary mutual borrowing from the AudioUnitStream variable. This
will help to avoid potential borrowing-twice issues in the later mutex
replacement.

Depends on D34048

By moving get_voluem out of AudioUnitStream, we can avoid unnecessary
borrowing from the AudioUnitStream variable. This change will make the
get_volume become symmetric to the set_volume.

Depends on D34049

Setting variable by the Result value directly is rustier than setting
the variable by a mutual reference with a dummy Result that only
contains an error.

Depends on D34050

Setting variable by the Result value directly is rustier than setting
the variable by a mutual reference with a dummy Result that only
contains an error.

Depends on D34051

This change provides two benefits:

  1. Setting variable by the Result value directly is rustier than setting
    the variable by a mutual reference with a dummy Result that only
    contains an error.

  2. By moving out this API from the AudioUnitStream, we can avoid calling
    it by borrowing the AudioUnitStream variable as a mutable. This will
    help to avoid the potential borrowing-twice issues in the later mutex
    replacement.

Depends on D34052

By moving out this API from the AudioUnitStream, we can avoid calling
it by borrowing the AudioUnitStream variable as a mutable. This will
help to avoid the potential borrowing-twice issues in the later mutex
replacement

In addition, the change applies an idiomatic way to wait for the system
callback event without consuming CPU time.

Depends on D34053

The registration, unregistration, and notification for the device
property changed is done in a critical section created by locking our
own custom mutex. To replace our own custom mutex by standard Rust
mutex, the critical section should be created by locking a standard Rust
mutex. This can be done by simply merging device_changed_callback and
device_changed_callback_lock into a Rust mutex variable.

Depends on D34054

The mutex is considered poisoned whenever a thread panics while holding
the mutex. Registering a second device changed callback without unregistering
the original callback makes the thread panics while holding the mutex
for the device_changed_callback. This mutex will be used when device
property changed callback. If the mutex is poisoned then a device
property is changed, we will get another panic when firing the callback
because of using a poisoned mutex.

Depends on D34055

Using an aggregate-device struct to operate all the settings for the
aggregate device is a way to make the code clearer. In addition, we can
avoid calling some aggregate-device APIs by borrowing the
AudioUnitStream as a mutable. It will help to avoid potential
borrowing-twice issues in the later mutex replacement.

Depends on D34056

The aggregate device of the stream may be created, reinitialized, or
destroyed on different threads, so its operations should be in the
critical sections. We do create critical sections by our custom mutex.
However, this custom mutex will be gradually replaced by the standard
Rust mutex in the following patches.

To replace the custom mutex, we create a struct wrapped by a Rust mutex
and create critical sections by this Rust mutex to do operations for
aggregate-device settings. At the end, not only aggregate-device
callings, but also other operations that needs to be locked will be
operated in the critical section created by this struct.

This is the beginning patch to replace the custom mutex in
AudioUnitStream.

Depends on D34057

Using a resample struct to operate all its related operations will make
the code clearer.

Depends on D34058

The resampler of the stream will be created, reinitialized, used or
destroyed on different threads, so its operations should be in the
critical sections. We do create critical sections by our custom mutex.
However, this custom mutex will be gradually replaced by the standard
mutex in the following patches. To replace the custom mutex, we put the
resampler to the struct wrapped by a Rust mutex and do all the resampler
operations in the critical section created by this struct. At the end
when the custom mutex is removed, those operations are still in critical
sections.

Depends on D34059

Using an Mixer struct to operate all the mixing APIs is a way to make
the code clearer. In addition, we can avoid calling mix function by
borrowing the AudioUnitStream as a mutable. It will help to avoid
potential borrowing-twice issues in the later mutex replacement.

Depends on D34060

The mixer of the stream will be created, reinitialized, used or
destroyed on different threads, so its operations should be in the
critical sections. We do create critical sections by our custom mutex.
However, this custom mutex will be gradually replaced by the standard
Rust mutex in the following patches.

To replace the custom mutex, we put the mixer to the struct wrapped by a
Rust mutex and do all the mixer operations in the critical section
created by this struct. At the end when the custom mutex is removed,
those operations are still in critical sections.

Calling notify_state_changed needs to borrow AudioUnitStream as a
mutuable. To avoid the borrowing-twice issue, the notify_state_changed
calling is moved out the scope of the critical section created in the
output data callback.

Depends on D34061

  • Remove unnecessary mutabilities
  • Remove duplicate API callings

Depends on D34062

  1. If AudioUnitRender return kAudioUnitErr_CannotDoInCurrentContext
    within a input-only stream, the input-only stream will keep feed silence
    data into the buffer instead of reporting the error. With this change,
    the error will be rendered as the returned value of the data callback to
    the underlying CoreAudio framework.

  2. By merging the render_input into audiounit_input_callback and adjust
    the timing to call reinit_async, now the reinit_async is called at the
    line that is out of the main logic for feeding buffer data. In the scope
    of the main logic, there will be a critical section created by locking a
    Rust mutex within AudioUnitStream in the later mutex replacement.
    Without moving the reinit_async, which borrows AudioUnitStream as a
    mutable, out of the critical section, there will be a borrowing-twice
    issue.

Depends on D34063

Before reporting the error by the state callback, closing the stream to
make sure the data callback and device-changed callback are
unregistered.

Depends on D34064

  1. The code readability is slightly better if we can register and
    unregister the callbacks in the same places. The device-chaned callbacks
    are registered when stream setup and unregistered when stream close.

  2. In the later mutex replacement, the core stream variables that may be
    touched by different threads when reinitializing or destroying the
    stream, including {in, out}_unit, will be wrapped in a Rust mutex.
    Moving these callbacks (un)registration into the same places makes it
    easier to find where the critical sections are.

Depends on D34065

  1. Avoid calling layout_init by wrong AudioUnit value

  2. Avoid calling layout getting/setting APIs by borrowing the
    AudioUnitStream as a mutable. It will help to avoid the potential
    borrowing-twice issues in the later mutex replacement.

Depends on D34066

We store global layout info and channels info in the cubeb context.
However, the output devices may be different on different streams. We
should store the info of the output device in the stream directly
instead of in the context. The channels value is a duplicate of
output_desc.mChannelsPerFrame or mixer.out_channels so we don't need to
store this value.

Depends on D34067

Avoid calling minimum_resampling_input_frames by a borrowing from
AudioUnitStream

Depends on D34068

Calling configure_output requires borrowing AudioUnitStream as a
mutable. Merging configure_output into setup will help to avoid a
potential borrowing-twice issue in the later mutex replacement. There
will be a critical section created by a Rust mutex in the setup, which
will borrow the AudioUnitStream as an immutable.

Depends on D34069

Calling configure_input requires borrowing AudioUnitStream as a
mutable. Merging configure_input into setup will help to avoid a
potential borrowing-twice issue in the later mutex replacement. There
will be a critical section created by a Rust mutex in the setup, which
will borrow the AudioUnitStream as an immutable.

Depends on D34070

The listeners will be registered and unregistered on the different threads,
so the listeners should be used in the critical sections. We do create
critical sections by our own mutex. However, this custom mutex will be
gradually replaced by the standard Rust mutex in the following patches.

To replace the custom mutex, we put the listeners to the struct wrapped
by a Rust mutex and register or unregister the listeners in the critical
section created by this struct. At the end when the custom mutex is
removed, those operations are still in the critical sections.

Depends on D34071

The core stream data that will be touched on the different threads when
the stream is created, reinitialized, destoryed should be used in the
critical section. Those core data are initialized in stream-setup and
destroyed in stream-close. The stream-setup and stream-close is run in
the critical section so they won't be executed at the same time.
Currently, the critical section is created by our custom mutex. However,
this custom mutex will be removed in the later mutex replacement.

Instead of running these two operations in the critical sections created
by the custom mutex, they should be moved into a struct wrapped by a
standard Rust mutex. As a result, at the end when the custom mutex is
removed, these two operations are still in the critical sections that
are created by the Rust mutex.

Depends on D34072

The custom mutex, OwnedCriticalSection, in the current code comes with
the C-to-Rust translation work of cubeb_audiouniut.cpp. Its design is in
C style and not fitted well in the Rust. Rust has a better mutex design.

In the previous patches, all the data that may be touched on the
different threads are moved into a struct wrapped by a Rust mutex. Those
data will be touched in the critical sections created by the Rust mutex.
Therefore, this custom mutex becomes redundant. It's time to say goodbye
to it.

Depends on D34073

There are three potential data-race operations that may run at the same
time:

  1. Data callback and stream reinitialization
  2. Data callback and stream destroying
  3. Stream reinitialization and stream destroying

The case 1 and 2 won't happen as long as the AudioOutputUnitStop is
called at the beginning of stream reinitialization and stream
destorying. The AudioOutputUnitStop requires to lock a mutex inside
CoreAudio framework that is also used by the data callback. Thus, if
there is a running callback, which holds the mutex inside CoreAudio
framework, when AudioOutputUnitStop is called, then the calling will
block the current thread until the data callback ends since it is
waiting for the mutex. By calling AudioOutputUnitStop at the beginning
of the stream reinitialization and stream destroying, the data race of
case 1 and 2 can be avoided.

On the other hand, the case 3 won't happen since the stream
initialization and destroying is run on the same task queue. The two
tasks on the same serial task queue are impossible to be run at the same
time. The mutex in AudioUnitStream is unnecessary because it's used for
the case 3.

Depends on D34074

Depends on: 1557877
Attachment #9050854 - Attachment description: Bug 1530715 - P2: Import oxidized cubeb_audiounit.cpp. r?padenot → Bug 1530715 - P2: Import oxidized cubeb_audiounit.cpp. r=padenot
Attachment #9061008 - Attachment description: Bug 1530715 - P5: Remove the unused code when clamping latency frames. r?padenot → Bug 1530715 - P5: Remove the unused code when clamping latency frames. r=padenot
Attachment #9062831 - Attachment description: Bug 1530715 - P6: Fully initialize the aggregate device before using it. r?padenot → Bug 1530715 - P6: Fully initialize the aggregate device before using it. r=padenot
Attachment #9062832 - Attachment description: Bug 1530715 - P7: Move active_streams to a struct within a mutex. r?padenot → Bug 1530715 - P7: Move active_streams to a struct within a mutex. r=padenot
Attachment #9062833 - Attachment description: Bug 1530715 - P8: Always use global latency frames to initialize the cubeb stream. r?padenot → Bug 1530715 - P8: Always use global latency frames to initialize the cubeb stream. r=padenot
Attachment #9062834 - Attachment description: Bug 1530715 - P9: Move device-collection-changed variables to a struct within a mutex. r?padenot → Bug 1530715 - P9: Move device-collection-changed variables to a struct within a mutex. r=padenot
Attachment #9070408 - Attachment description: Bug 1530715 - P12: Remove the unused variable in AudioUnitStream. r?padenot → Bug 1530715 - P12: Remove the unused variable in AudioUnitStream. r=padenot
Attachment #9070409 - Attachment description: Bug 1530715 - P13: Remove unnecessary parameter in reinit. r?padenot → Bug 1530715 - P13: Remove unnecessary parameter in reinit. r=padenot
Attachment #9070410 - Attachment description: Bug 1530715 - P14: Wrap state-callback code into a function. r?padenot → Bug 1530715 - P14: Wrap state-callback code into a function. r=padenot
Attachment #9070411 - Attachment description: Bug 1530715 - P15: Replace set_device_info by create_device_info. r?padenot → Bug 1530715 - P15: Replace set_device_info by create_device_info. r=padenot
Attachment #9070412 - Attachment description: Bug 1530715 - P16: Implement an internal set_volume API. r?padenot → Bug 1530715 - P16: Implement an internal set_volume API. r=padenot
Attachment #9070413 - Attachment description: Bug 1530715 - P17: Move get_volume out of AudioUnitStream. r?padenot → Bug 1530715 - P17: Move get_volume out of AudioUnitStream. r=padenot
Attachment #9070414 - Attachment description: Bug 1530715 - P18: Replace audiounit_create_unit by create_audiounit and its friends. r?padenot → Bug 1530715 - P18: Replace audiounit_create_unit by create_audiounit and its friends. r=padenot
Attachment #9070415 - Attachment description: Bug 1530715 - P19: Replace audio_stream_desc_init by create_stream_description. r?padenot → Bug 1530715 - P19: Replace audio_stream_desc_init by create_stream_description. r=padenot
Attachment #9070416 - Attachment description: Bug 1530715 - P20: Replace init_input_linear_buffer by create_auto_array. r?padenot → Bug 1530715 - P20: Replace init_input_linear_buffer by create_auto_array. r=padenot
Attachment #9070417 - Attachment description: Bug 1530715 - P21: Replace set_buffer_size by set_buffer_size_sync and its friends. r?padenot → Bug 1530715 - P21: Replace set_buffer_size by set_buffer_size_sync and its friends. r=padenot
Attachment #9070418 - Attachment description: Bug 1530715 - P22: Replace device_changed_callback_lock by standard Rust mutex. r?padenot → Bug 1530715 - P22: Replace device_changed_callback_lock by standard Rust mutex. r=padenot
Attachment #9070419 - Attachment description: Bug 1530715 - P23: Avoid poisoning the mutex for device_changed_callback. r?padenot → Bug 1530715 - P23: Avoid poisoning the mutex for device_changed_callback. r=padenot
Attachment #9070420 - Attachment description: Bug 1530715 - P24: Create an AggregateDevice module. r?padenot → Bug 1530715 - P24: Create an AggregateDevice module. r=padenot
Attachment #9070421 - Attachment description: Bug 1530715 - P25: Move aggregate_device to a struct within a mutex. r?padenot → Bug 1530715 - P25: Move aggregate_device to a struct within a mutex. r=padenot
Attachment #9070422 - Attachment description: Bug 1530715 - P26: Create a Resampler module. r?padenot → Bug 1530715 - P26: Create a Resampler module. r=padenot
Attachment #9070423 - Attachment description: Bug 1530715 - P27: Move resampler to a struct within a mutex. r?padenot → Bug 1530715 - P27: Move resampler to a struct within a mutex. r=padenot
Attachment #9070424 - Attachment description: Bug 1530715 - P28: Create a Mixer module. r?padenot → Bug 1530715 - P28: Create a Mixer module. r=padenot
Attachment #9070425 - Attachment description: Bug 1530715 - P29: Move mixer to a struct within a mutex. r?padenot → Bug 1530715 - P29: Move mixer to a struct within a mutex. r=padenot
Attachment #9070426 - Attachment description: Bug 1530715 - P30: Some clean-up in audiounit_output_callback. r?padenot → Bug 1530715 - P30: Some clean-up in audiounit_output_callback. r=padenot
Attachment #9070427 - Attachment description: Bug 1530715 - P31: Merge render_input into audiounit_input_callback. r?padenot → Bug 1530715 - P31: Merge render_input into audiounit_input_callback. r=padenot
Attachment #9070428 - Attachment description: Bug 1530715 - P32: Close the stream if failing in stream reinitialization. r?padenot → Bug 1530715 - P32: Close the stream if failing in stream reinitialization. r=padenot
Attachment #9070429 - Attachment description: Bug 1530715 - P33: Register and unregister the device-changed callbacks when stream setup and close. r?padenot → Bug 1530715 - P33: Register and unregister the device-changed callbacks when stream setup and close. r=padenot
Attachment #9070430 - Attachment description: Bug 1530715 - P34: Merge layout_init into configure_output. r?padenot → Bug 1530715 - P34: Merge layout_init into configure_output. r=padenot
Attachment #9070431 - Attachment description: Bug 1530715 - P35: Store layout in the stream instead of context. r?padenot → Bug 1530715 - P35: Store layout in the stream instead of context. r=padenot
Attachment #9070432 - Attachment description: Bug 1530715 - P36: Move out minimum_resampling_input_frames of output callback. r?padenot → Bug 1530715 - P36: Move out minimum_resampling_input_frames of output callback. r=padenot
Attachment #9070433 - Attachment description: Bug 1530715 - P37: Merge configure_output into setup. r?padenot → Bug 1530715 - P37: Merge configure_output into setup. r=padenot
Attachment #9070434 - Attachment description: Bug 1530715 - P38: Merge configure_input into setup. r?padenot → Bug 1530715 - P38: Merge configure_input into setup. r=padenot
Attachment #9070435 - Attachment description: Bug 1530715 - P39: Move listeners (un)registrations into a strcut within a mutex. r?padenot → Bug 1530715 - P39: Move listeners (un)registrations into a strcut within a mutex. r=padenot
Attachment #9070436 - Attachment description: Bug 1530715 - P40: Move stream setup and close into a struct within a mutex. r?padenot → Bug 1530715 - P40: Move stream setup and close into a struct within a mutex. r=padenot
Attachment #9070437 - Attachment description: Bug 1530715 - P41: Remove OwnedCriticalSection in cubeb stream. r?padenot → Bug 1530715 - P41: Remove OwnedCriticalSection in cubeb stream. r=padenot
Attachment #9070439 - Attachment description: Bug 1530715 - P42: Remove unnecessary mutex. r?padenot → Bug 1530715 - P42: Remove unnecessary mutex. r=padenot

Results on try server are here.

Keywords: checkin-needed

Land it after code freeze instead.

Keywords: checkin-needed
Keywords: checkin-needed

The patches fail to land because there are 3 patch queues here (p1-p5, p6-p11, p12-p42).

p1-p5 cannot be landed, see https://lando.services.mozilla.com/D29011/ Please adjust the patch stack so it can be landed.

Flags: needinfo?(cchang)
Keywords: checkin-needed
Flags: needinfo?(cchang)
Keywords: checkin-needed

Pushed by apavel@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/30f0b9853b0f
P1: Create an empty cubeb-coreaudio-rs crate in libcubeb. r=kinetik
https://hg.mozilla.org/integration/autoland/rev/4bed62b601c6
P2: Import oxidized cubeb_audiounit.cpp. r=padenot
https://hg.mozilla.org/integration/autoland/rev/0500179d3bbf
P3: Build cubeb-coreaudio-rs in libcubeb. r=kinetik
https://hg.mozilla.org/integration/autoland/rev/48719d342b6f
P4: Vendor Rust dependencies for cubeb-coreaudio-rs. r=kinetik
https://hg.mozilla.org/integration/autoland/rev/4cd64e4037fa
P5: Remove the unused code when clamping latency frames. r=padenot
https://hg.mozilla.org/integration/autoland/rev/7042c5c9c764
P6: Fully initialize the aggregate device before using it. r=padenot
https://hg.mozilla.org/integration/autoland/rev/5ebf9b275f10
P7: Move active_streams to a struct within a mutex. r=padenot
https://hg.mozilla.org/integration/autoland/rev/0cec8139038c
P8: Always use global latency frames to initialize the cubeb stream. r=padenot
https://hg.mozilla.org/integration/autoland/rev/7562f780b687
P9: Move device-collection-changed variables to a struct within a mutex. r=padenot
https://hg.mozilla.org/integration/autoland/rev/a8d7741d9134
P10: Avoid poisoning the mutex for device-collection-changed. r=padenot
https://hg.mozilla.org/integration/autoland/rev/72c18b15e767
P11: Remove OwnedCriticalSection in cubeb context. r=padenot
https://hg.mozilla.org/integration/autoland/rev/e98c1c0d561d
P12: Remove the unused variable in AudioUnitStream. r=padenot
https://hg.mozilla.org/integration/autoland/rev/dda297f7755a
P13: Remove unnecessary parameter in reinit. r=padenot
https://hg.mozilla.org/integration/autoland/rev/b6139a17a5fd
P14: Wrap state-callback code into a function. r=padenot
https://hg.mozilla.org/integration/autoland/rev/bf15184c3c63
P15: Replace set_device_info by create_device_info. r=padenot
https://hg.mozilla.org/integration/autoland/rev/3742faad3806
P16: Implement an internal set_volume API. r=padenot
https://hg.mozilla.org/integration/autoland/rev/b4809ada60dd
P17: Move get_volume out of AudioUnitStream. r=padenot
https://hg.mozilla.org/integration/autoland/rev/9acd782f9450
P18: Replace audiounit_create_unit by create_audiounit and its friends. r=padenot
https://hg.mozilla.org/integration/autoland/rev/cd0e483df6be
P19: Replace audio_stream_desc_init by create_stream_description. r=padenot
https://hg.mozilla.org/integration/autoland/rev/dcd65347e392
P20: Replace init_input_linear_buffer by create_auto_array. r=padenot
https://hg.mozilla.org/integration/autoland/rev/de4e23af5054
P21: Replace set_buffer_size by set_buffer_size_sync and its friends. r=padenot
https://hg.mozilla.org/integration/autoland/rev/ecd20933073e
P22: Replace device_changed_callback_lock by standard Rust mutex. r=padenot
https://hg.mozilla.org/integration/autoland/rev/7fe0263794d3
P23: Avoid poisoning the mutex for device_changed_callback. r=padenot
https://hg.mozilla.org/integration/autoland/rev/0c5a0ed7203a
P24: Create an AggregateDevice module. r=padenot
https://hg.mozilla.org/integration/autoland/rev/8f10a1790557
P25: Move aggregate_device to a struct within a mutex. r=padenot
https://hg.mozilla.org/integration/autoland/rev/421e51bf612b
P26: Create a Resampler module. r=padenot
https://hg.mozilla.org/integration/autoland/rev/e527edaabd65
P27: Move resampler to a struct within a mutex. r=padenot
https://hg.mozilla.org/integration/autoland/rev/3f3677f2980f
P28: Create a Mixer module. r=padenot
https://hg.mozilla.org/integration/autoland/rev/a964627f40c6
P29: Move mixer to a struct within a mutex. r=padenot
https://hg.mozilla.org/integration/autoland/rev/fe28daa92fc4
P30: Some clean-up in audiounit_output_callback. r=padenot
https://hg.mozilla.org/integration/autoland/rev/bd809d3c5fd8
P31: Merge render_input into audiounit_input_callback. r=padenot
https://hg.mozilla.org/integration/autoland/rev/bdc3110a40e9
P32: Close the stream if failing in stream reinitialization. r=padenot
https://hg.mozilla.org/integration/autoland/rev/2a8d1c6faa33
P33: Register and unregister the device-changed callbacks when stream setup and close. r=padenot
https://hg.mozilla.org/integration/autoland/rev/9e10aa50ca0e
P34: Merge layout_init into configure_output. r=padenot
https://hg.mozilla.org/integration/autoland/rev/7c1e970c84a0
P35: Store layout in the stream instead of context. r=padenot
https://hg.mozilla.org/integration/autoland/rev/70e97244daa8
P36: Move out minimum_resampling_input_frames of output callback. r=padenot
https://hg.mozilla.org/integration/autoland/rev/aca1e6806fc9
P37: Merge configure_output into setup. r=padenot
https://hg.mozilla.org/integration/autoland/rev/518f85cf84a9
P38: Merge configure_input into setup. r=padenot
https://hg.mozilla.org/integration/autoland/rev/7506b654e92f
P39: Move listeners (un)registrations into a strcut within a mutex. r=padenot
https://hg.mozilla.org/integration/autoland/rev/7e7f5b9f2bf6
P40: Move stream setup and close into a struct within a mutex. r=padenot
https://hg.mozilla.org/integration/autoland/rev/586d1b42a31b
P41: Remove OwnedCriticalSection in cubeb stream. r=padenot
https://hg.mozilla.org/integration/autoland/rev/88ba94506737
P42: Remove unnecessary mutex. r=padenot

Keywords: checkin-needed

Backed out for SM build bustages

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&selectedJob=255667067&resultStatus=testfailed%2Cbusted%2Cexception&revision=7d15ef2158467c332ead1e77198dde94ebb69e23

Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=255667067&repo=autoland&lineNumber=250

[task 2019-07-10T07:46:40.145Z] Updating git repository https://github.com/ChunMinChang/coreaudio-sys/
[task 2019-07-10T07:46:40.162Z] error: failed to load source for a dependency on coreaudio-sys
[task 2019-07-10T07:46:40.162Z]
[task 2019-07-10T07:46:40.162Z] Caused by:
[task 2019-07-10T07:46:40.162Z] Unable to update https://github.com/ChunMinChang/coreaudio-sys/?branch=gecko-build#b1fe5ea0
[task 2019-07-10T07:46:40.162Z]
[task 2019-07-10T07:46:40.162Z] Caused by:
[task 2019-07-10T07:46:40.162Z] failed to clone into: /builds/worker/.cargo/git/db/coreaudio-sys-fe21c23e0002c5e9
[task 2019-07-10T07:46:40.162Z]
[task 2019-07-10T07:46:40.162Z] Caused by:
[task 2019-07-10T07:46:40.162Z] attempting to update a git repository, but --frozen was specified
[taskcluster 2019-07-10 07:46:40.649Z] === Task Finished ===
[taskcluster 2019-07-10 07:46:40.767Z] Artifact "public/build" not found at "/builds/worker/artifacts/"
[taskcluster 2019-07-10 07:46:41.364Z] Unsuccessful task run with exit code: 101 completed in 108.999 seconds

Backout: https://hg.mozilla.org/integration/autoland/rev/744a7f934cfa106dae8b0eb8ee896ba683ef2b3b

Flags: needinfo?(cchang)

I've added some settings in .cargo/config.in to fix this: https://treeherder.mozilla.org/#/jobs?repo=try&selectedJob=256854936&revision=d5d69629047bf1859a35cf11dfbcf4cdb884d7f5

I will do another run of tests in tryserver to see if the changes break other things or not. Thanks for pointing out the failures.

Flags: needinfo?(cchang)

Pushed by archaeopteryx@coole-files.de:
https://hg.mozilla.org/integration/autoland/rev/145f08b33417
P1: Create an empty cubeb-coreaudio-rs crate in libcubeb. r=kinetik
https://hg.mozilla.org/integration/autoland/rev/2c216d5dc4fa
P2: Import oxidized cubeb_audiounit.cpp. r=padenot
https://hg.mozilla.org/integration/autoland/rev/eb1664091d76
P3: Build cubeb-coreaudio-rs in libcubeb. r=kinetik
https://hg.mozilla.org/integration/autoland/rev/12dedc5de6d3
P4: Vendor Rust dependencies for cubeb-coreaudio-rs. r=kinetik
https://hg.mozilla.org/integration/autoland/rev/7290ce682e7b
P5: Remove the unused code when clamping latency frames. r=padenot
https://hg.mozilla.org/integration/autoland/rev/4c3cb7c2d225
P6: Fully initialize the aggregate device before using it. r=padenot
https://hg.mozilla.org/integration/autoland/rev/6a85d8f9f41a
P7: Move active_streams to a struct within a mutex. r=padenot
https://hg.mozilla.org/integration/autoland/rev/228fe83b4669
P8: Always use global latency frames to initialize the cubeb stream. r=padenot
https://hg.mozilla.org/integration/autoland/rev/971be34930a4
P9: Move device-collection-changed variables to a struct within a mutex. r=padenot
https://hg.mozilla.org/integration/autoland/rev/c25bac8e2a51
P10: Avoid poisoning the mutex for device-collection-changed. r=padenot
https://hg.mozilla.org/integration/autoland/rev/baf8f968e394
P11: Remove OwnedCriticalSection in cubeb context. r=padenot
https://hg.mozilla.org/integration/autoland/rev/426d20836719
P12: Remove the unused variable in AudioUnitStream. r=padenot
https://hg.mozilla.org/integration/autoland/rev/1ea5ba26b2f8
P13: Remove unnecessary parameter in reinit. r=padenot
https://hg.mozilla.org/integration/autoland/rev/4cf8bcd64647
P14: Wrap state-callback code into a function. r=padenot
https://hg.mozilla.org/integration/autoland/rev/0dc95f672782
P15: Replace set_device_info by create_device_info. r=padenot
https://hg.mozilla.org/integration/autoland/rev/9aa4f6cb2843
P16: Implement an internal set_volume API. r=padenot
https://hg.mozilla.org/integration/autoland/rev/95e4d3c3296b
P17: Move get_volume out of AudioUnitStream. r=padenot
https://hg.mozilla.org/integration/autoland/rev/60e2667ff306
P18: Replace audiounit_create_unit by create_audiounit and its friends. r=padenot
https://hg.mozilla.org/integration/autoland/rev/d7d4a5c9dde3
P19: Replace audio_stream_desc_init by create_stream_description. r=padenot
https://hg.mozilla.org/integration/autoland/rev/004f85742b4c
P20: Replace init_input_linear_buffer by create_auto_array. r=padenot
https://hg.mozilla.org/integration/autoland/rev/a80d27c3114d
P21: Replace set_buffer_size by set_buffer_size_sync and its friends. r=padenot
https://hg.mozilla.org/integration/autoland/rev/9fe7e1c2da27
P22: Replace device_changed_callback_lock by standard Rust mutex. r=padenot
https://hg.mozilla.org/integration/autoland/rev/c842af1a5ca7
P23: Avoid poisoning the mutex for device_changed_callback. r=padenot
https://hg.mozilla.org/integration/autoland/rev/976263f4ead5
P24: Create an AggregateDevice module. r=padenot
https://hg.mozilla.org/integration/autoland/rev/93348af70403
P25: Move aggregate_device to a struct within a mutex. r=padenot
https://hg.mozilla.org/integration/autoland/rev/970f3217c5a1
P26: Create a Resampler module. r=padenot
https://hg.mozilla.org/integration/autoland/rev/7fa4c29b3ec1
P27: Move resampler to a struct within a mutex. r=padenot
https://hg.mozilla.org/integration/autoland/rev/bacfef6665e7
P28: Create a Mixer module. r=padenot
https://hg.mozilla.org/integration/autoland/rev/cc39becdbbf9
P29: Move mixer to a struct within a mutex. r=padenot
https://hg.mozilla.org/integration/autoland/rev/39adde36627b
P30: Some clean-up in audiounit_output_callback. r=padenot
https://hg.mozilla.org/integration/autoland/rev/9bb78d3625b9
P31: Merge render_input into audiounit_input_callback. r=padenot
https://hg.mozilla.org/integration/autoland/rev/9919e152f9bb
P32: Close the stream if failing in stream reinitialization. r=padenot
https://hg.mozilla.org/integration/autoland/rev/11432ad63378
P33: Register and unregister the device-changed callbacks when stream setup and close. r=padenot
https://hg.mozilla.org/integration/autoland/rev/eaec698bcf3a
P34: Merge layout_init into configure_output. r=padenot
https://hg.mozilla.org/integration/autoland/rev/774c895c293d
P35: Store layout in the stream instead of context. r=padenot
https://hg.mozilla.org/integration/autoland/rev/69804f288c29
P36: Move out minimum_resampling_input_frames of output callback. r=padenot
https://hg.mozilla.org/integration/autoland/rev/b4f102752e7d
P37: Merge configure_output into setup. r=padenot
https://hg.mozilla.org/integration/autoland/rev/c18d984cb318
P38: Merge configure_input into setup. r=padenot
https://hg.mozilla.org/integration/autoland/rev/083df83e6977
P39: Move listeners (un)registrations into a strcut within a mutex. r=padenot
https://hg.mozilla.org/integration/autoland/rev/0332b07abeca
P40: Move stream setup and close into a struct within a mutex. r=padenot
https://hg.mozilla.org/integration/autoland/rev/cb5ec80eef46
P41: Remove OwnedCriticalSection in cubeb stream. r=padenot
https://hg.mozilla.org/integration/autoland/rev/0e11ebbb3d79
P42: Remove unnecessary mutex. r=padenot

Keywords: checkin-needed
Pushed by emilio@crisal.io:
https://hg.mozilla.org/integration/autoland/rev/060374c521b6
Run mach vendor rust on a CLOSED TREE. r=bustage

https://hg.mozilla.org/mozilla-central/rev/145f08b33417
https://hg.mozilla.org/mozilla-central/rev/2c216d5dc4fa
https://hg.mozilla.org/mozilla-central/rev/eb1664091d76
https://hg.mozilla.org/mozilla-central/rev/12dedc5de6d3
https://hg.mozilla.org/mozilla-central/rev/7290ce682e7b
https://hg.mozilla.org/mozilla-central/rev/4c3cb7c2d225
https://hg.mozilla.org/mozilla-central/rev/6a85d8f9f41a
https://hg.mozilla.org/mozilla-central/rev/228fe83b4669
https://hg.mozilla.org/mozilla-central/rev/971be34930a4
https://hg.mozilla.org/mozilla-central/rev/c25bac8e2a51
https://hg.mozilla.org/mozilla-central/rev/baf8f968e394
https://hg.mozilla.org/mozilla-central/rev/426d20836719
https://hg.mozilla.org/mozilla-central/rev/1ea5ba26b2f8
https://hg.mozilla.org/mozilla-central/rev/4cf8bcd64647
https://hg.mozilla.org/mozilla-central/rev/0dc95f672782
https://hg.mozilla.org/mozilla-central/rev/9aa4f6cb2843
https://hg.mozilla.org/mozilla-central/rev/95e4d3c3296b
https://hg.mozilla.org/mozilla-central/rev/60e2667ff306
https://hg.mozilla.org/mozilla-central/rev/d7d4a5c9dde3
https://hg.mozilla.org/mozilla-central/rev/004f85742b4c
https://hg.mozilla.org/mozilla-central/rev/a80d27c3114d
https://hg.mozilla.org/mozilla-central/rev/9fe7e1c2da27
https://hg.mozilla.org/mozilla-central/rev/c842af1a5ca7
https://hg.mozilla.org/mozilla-central/rev/976263f4ead5
https://hg.mozilla.org/mozilla-central/rev/93348af70403
https://hg.mozilla.org/mozilla-central/rev/970f3217c5a1
https://hg.mozilla.org/mozilla-central/rev/7fa4c29b3ec1
https://hg.mozilla.org/mozilla-central/rev/bacfef6665e7
https://hg.mozilla.org/mozilla-central/rev/cc39becdbbf9
https://hg.mozilla.org/mozilla-central/rev/39adde36627b
https://hg.mozilla.org/mozilla-central/rev/9bb78d3625b9
https://hg.mozilla.org/mozilla-central/rev/9919e152f9bb
https://hg.mozilla.org/mozilla-central/rev/11432ad63378
https://hg.mozilla.org/mozilla-central/rev/eaec698bcf3a
https://hg.mozilla.org/mozilla-central/rev/774c895c293d
https://hg.mozilla.org/mozilla-central/rev/69804f288c29
https://hg.mozilla.org/mozilla-central/rev/b4f102752e7d
https://hg.mozilla.org/mozilla-central/rev/c18d984cb318
https://hg.mozilla.org/mozilla-central/rev/083df83e6977
https://hg.mozilla.org/mozilla-central/rev/0332b07abeca
https://hg.mozilla.org/mozilla-central/rev/cb5ec80eef46
https://hg.mozilla.org/mozilla-central/rev/0e11ebbb3d79
https://hg.mozilla.org/mozilla-central/rev/060374c521b6

Status: NEW → RESOLVED
Closed: 4 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla70
Regressions: 1570077
Regressions: 1570080
Blocks: 1598148
Blocks: 1598188
You need to log in before you can comment on or make changes to this bug.