Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support for BaseAudioContext.currentTime and state changes #32

Merged
merged 6 commits into from May 31, 2018

Conversation

@ferjm
Copy link
Member

ferjm commented May 28, 2018

Fixes #22 and #23

@ferjm ferjm requested a review from Manishearth May 28, 2018
@ferjm
Copy link
Member Author

ferjm commented May 28, 2018

use std::thread::Builder;

static NEXT_NODE_ID: AtomicUsize = ATOMIC_USIZE_INIT;

pub struct AudioGraph {
sender: Sender<AudioRenderThreadMsg>,
sample_rate: f32,
current_time: Arc<RwLock<f64>>,

This comment has been minimized.

@Manishearth

Manishearth May 29, 2018

Member

does the AudioGraph need this? I'd rather have this be owned by the actual render thread without an RWLock

This comment has been minimized.

@Manishearth

Manishearth May 29, 2018

Member

overall we should clean up the mutability story here but I have a plan for that

This comment has been minimized.

@ferjm

ferjm May 29, 2018

Author Member

The WebAudio API exposes sync getters for these values, so I figured that we should make the control thread own them. I guess we can have a sync channel with buffer size 0 for these getters (?).

@@ -117,7 +121,12 @@ impl AudioRenderThread {
}
// and push into the audio sink the result of processing a
// render quantum.
let _ = self.sink.push_data(self.process());
if let Ok(duration) = self.sink.push_data(self.process()) {

This comment has been minimized.

@Manishearth

Manishearth May 29, 2018

Member

we shouldn't need to ask the sink for the duration; we know how long it is. The sink calculates the duration from information on the buffer that we provided.

@Manishearth
Copy link
Member

Manishearth commented May 29, 2018

@ferjm ferjm force-pushed the ferjm:currentTime branch from 6af0794 to a805102 May 30, 2018
@ferjm ferjm changed the title Support for BaseAudioContext.currentTime Support for BaseAudioContext.currentTime and state changes May 30, 2018
@ferjm ferjm force-pushed the ferjm:currentTime branch from a805102 to 838baba May 30, 2018
@ferjm ferjm requested a review from Manishearth May 31, 2018
@Manishearth
Copy link
Member

Manishearth commented May 31, 2018

I don't think we need a sync sender here, we can use the same one -- just block on the receiver end.

@Manishearth Manishearth merged commit 3c81ef0 into servo:master May 31, 2018
@ferjm ferjm deleted the ferjm:currentTime branch May 31, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.