Closed Bug 750596 Opened 8 years ago Closed 4 years ago

Rework AudioSink to be pull-based using cubeb's callback

Categories

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

defect

Tracking

()

RESOLVED WONTFIX

People

(Reporter: kinetik, Unassigned)

References

(Depends on 1 open bug, Blocks 1 open bug)

Details

No description provided.
Depends on: 782507, 694484, 698328
Depends on: 852401
OS: Linux → All
Hardware: x86_64 → All
Assignee: kinetik → nobody
Status: ASSIGNED → NEW
Blocks: 948267
This will allow the push based model to be removed, including the thread per playing element required for the blocking AudioStream::Write.
Summary: Remove the builtin decoder's audio thread and use cubeb's callback API for audio playback → Rework AudioSink to be pull-based using cubeb's callback
Depends on: 948269
Also, remove "friend class AudioSink" (and fix things as necessary) from MediaDecoderStateMachine during this pass of the refactoring work.
Blocks: 542635
Also, avoid the confusing existing behaviour that the assert removed by bug 948269 comment 38 was trying to pretend didn't exist.
Bobby: Here's the bug on removing the audio thread.

I suspect that Matthew Gregan and/or Paul Adenot have thought more about this, but my thoughts are that we can "produce" on the state machine task queue and cubeb can call us back on its thread and "consume" from the audio queue.

Looks to me like AudioSink::AudioLoop() basically just exists to wait for more input and write it into the AudioStream if we're playing; it basically is the play/pause gate for audio playback.

The first step could probably be in MSDM::OnAudioDecoded() to just write the decoded sample directly into the AudioSink, if we're playing. We'd need to push everything buffered in the MDSM::AudioQueue() into the AudioStream when we started playing too. 

I also wonder if we should remove the "write silence" logic in AudioSink::AudioLoop(). I don't think other browsers do this.

Matthew/Paul may have other, and potentially more thought through ideas on how to approach this.
Flags: needinfo?(bobbyholley)
This is on my list, but no particular reason to leave it NI.
Flags: needinfo?(bobbyholley)
I had a look at this this morning. I think this would be good to do, but it would require me to page in more audio code than I'd like to, and I think I can get better bang-for-buck on other things with my remaining time working on media code. So this is up for grabs, presumably by either matthew or paul.
Matthew -- If you could take this, that'd be much appreciated.  Paul is super busy right now. Thanks!
Flags: needinfo?(kinetik)
Happy to grab this, but it's about 5th on my list, so will be a couple of weeks before I can get to it.
Assignee: nobody → kinetik
Status: NEW → ASSIGNED
Flags: needinfo?(kinetik)
Component: Audio/Video → Audio/Video: MSG/cubeb/GMP
Component: Audio/Video: MSG/cubeb/GMP → Audio/Video: Playback
Assignee: kinetik → nobody
Status: ASSIGNED → NEW
The work has been done in bug 948267.
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.