Closed Bug 1752345 Opened 2 years ago Closed 2 years ago

Make AudioStream.cpp real-time safe on macOS

Categories

(Core :: Audio/Video: Playback, defect, P1)

defect

Tracking

()

RESOLVED FIXED
98 Branch
Tracking Status
firefox-esr91 --- wontfix
firefox96 --- wontfix
firefox97 + wontfix
firefox98 + fixed

People

(Reporter: padenot, Assigned: padenot)

References

Details

Attachments

(7 files)

No description provided.

This has two parts:

  1. Getting the clock is wait-free in cubeb-coreaudio-rs, so we can remove the
    lock. This is only on macOS, the lock is still taken on other platforms.
  2. The frame history part now uses an SPSCQueue, that sends the callback history
    information to the non-realtime thread, that lazily computes the clock the next
    time the clock time is requested.

It's doing annoying cross-thread accesses that require the lock, and it's not
needed anyways these days.

Depends on D137180

This changes things so that the time stretcher is only ever touched on the
audio thread. Changes to the playback rate and preserves pitch attribute now
also go through an SPSC queue (but in the other direction).

Locking is made more granular, only around the audio queue now.

Depends on D137181

This makes it clear that it's safe to access it from any thread.

Summary: Make AudioStream.cpp real-time safe → Make AudioStream.cpp real-time safe on macOS
Attachment #9261104 - Attachment description: WIP: Bug 1752345 - Make getting the clock for an AudioStream wait-free on macOS. → Bug 1752345 - Make getting the clock for an AudioStream wait-free on macOS. r?alwu,kinetik
Attachment #9261105 - Attachment description: WIP: Bug 1752345 - Remove the prefill quirk in AudioStream.cpp. → Bug 1752345 - Remove the prefill quirk in AudioStream.cpp. r?alwu,kinetik
Attachment #9261106 - Attachment description: WIP: Bug 1752345 - Make playback rate / preserve pitch change wait-free. → Bug 1752345 - Make playback rate / preserve pitch change wait-free. r?alwu,kinetik

This is safe in fact safe. The data might be a bit late, but it's going to be
correct.

Depends on D137183

The queue directly contains the audio frames, not packets. We'll be able to tune
its size later. AudioSink::PopFrames pull from the queue, and is called in the
audio callback. The decoded frame refill logic is adjusted to be simpler.

Signaling the event to refill still takes a lock, that's going to be fixed later.

Ended(), the second member of the interface, is switched to using an atomic.

After this patch, the audio callback doesn't need to take the AudioStream lock.

Depends on D137302

Not a new issue in 97 and we're going into RC week. Let's aim to get this fix shipped in 98.

Pushed by padenot@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/450a555cb1eb
Make getting the clock for an AudioStream wait-free on macOS.  r=alwu
https://hg.mozilla.org/integration/autoland/rev/a8594fe4510a
Remove the prefill quirk in AudioStream.cpp. r=alwu,kinetik
https://hg.mozilla.org/integration/autoland/rev/6ec8d89ef226
Make playback rate / preserve pitch change wait-free.  r=alwu
https://hg.mozilla.org/integration/autoland/rev/0728f7f6f21e
Make AudioStream::mOutChannel const. r=alwu,media-playback-reviewers
https://hg.mozilla.org/integration/autoland/rev/6ccf61634a6e
Allow calling AvailableWrite or AvailableRead on any thread.  r=alwu
https://hg.mozilla.org/integration/autoland/rev/5dba2de4456f
Communicate the audio frames to the audio thread with a wait-free queue. r=alwu,media-playback-reviewers
https://hg.mozilla.org/integration/autoland/rev/8d7472f1182f
Reset SPSC queue thread ids when the underlying audio callback thread changes. r=alwu
Depends on: 1754006
Regressions: 1759325
See Also: → 1765097
Regressions: 1830206
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: