Make AudioStream.cpp real-time safe on macOS
Categories
(Core :: Audio/Video: Playback, defect, P1)
Tracking
()
People
(Reporter: padenot, Assigned: padenot)
References
Details
Attachments
(7 files)
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review |
Updated•2 years ago
|
Assignee | ||
Comment 1•2 years ago
|
||
This has two parts:
- 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. - 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.
Assignee | ||
Comment 2•2 years ago
|
||
It's doing annoying cross-thread accesses that require the lock, and it's not
needed anyways these days.
Depends on D137180
Assignee | ||
Comment 3•2 years ago
|
||
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
Assignee | ||
Comment 4•2 years ago
|
||
This makes it clear that it's safe to access it from any thread.
Assignee | ||
Updated•2 years ago
|
Updated•2 years ago
|
Updated•2 years ago
|
Updated•2 years ago
|
Assignee | ||
Comment 5•2 years ago
|
||
This is safe in fact safe. The data might be a bit late, but it's going to be
correct.
Depends on D137183
Assignee | ||
Comment 6•2 years ago
|
||
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
Comment 7•2 years ago
|
||
Not a new issue in 97 and we're going into RC week. Let's aim to get this fix shipped in 98.
Assignee | ||
Comment 8•2 years ago
|
||
Depends on D137303
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
Comment 10•2 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/450a555cb1eb
https://hg.mozilla.org/mozilla-central/rev/a8594fe4510a
https://hg.mozilla.org/mozilla-central/rev/6ec8d89ef226
https://hg.mozilla.org/mozilla-central/rev/0728f7f6f21e
https://hg.mozilla.org/mozilla-central/rev/6ccf61634a6e
https://hg.mozilla.org/mozilla-central/rev/5dba2de4456f
https://hg.mozilla.org/mozilla-central/rev/8d7472f1182f
Updated•2 years ago
|
Description
•