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

Reduce latency by setting appsrc max-bytes to the block size #67

Merged
merged 1 commit into from Jul 2, 2018

Conversation

@ferjm
Copy link
Member

ferjm commented Jul 2, 2018

With this patch I can notice a much lower latency running the theremin demo.

r? @Manishearth @sdroege

@ferjm ferjm requested a review from Manishearth Jul 2, 2018
let bpf = audio_info.bpf() as usize;
let n_samples = FRAMES_PER_BLOCK.0 as u64;
let buf_size = (n_samples as usize) * (bpf);
self.appsrc.set_max_bytes(buf_size as u64);

This comment has been minimized.

@sdroege

sdroege Jul 2, 2018

Contributor

This makes no sense for reducing latency :) It would mean that any additional data is dropped (see block property) instead of being queued up. Which would mean that you either should implement some kind of backpressure here or you're just producing too much data to begin with.

This needs some closer investigation why it has an effect at all. The appsrc is forwarding data as fast as it can (not once the limit is reached), and downstream is consuming data in real-time.

This comment has been minimized.

@ferjm

ferjm Jul 2, 2018

Author Member

We have a backpressure mechanism here. We stop feeding the appsrc with data if we reached max_bytes (which, after this patch, will happen every time we push a single rendered block into the appsrc). IIUC this would essentially be the same as setting the block property on. But IIRC we couldn't use the block property because we wanted to continue processing messages sent to the render thread while we are waiting for the sink to process the audio.

This comment has been minimized.

@sdroege

sdroege Jul 2, 2018

Contributor

Ah I see. You could as well set max-bytes to 1 if you only ever want to allow a single chunk btw :)

Makes sense then, I was forgetting that context.

@ferjm ferjm force-pushed the ferjm:latency branch from a95e063 to edd2d98 Jul 2, 2018
@Manishearth Manishearth merged commit ae7d5f7 into servo:master Jul 2, 2018
@ferjm ferjm deleted the ferjm:latency branch Jul 3, 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

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