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

Streams registry #233

Merged
merged 1 commit into from Apr 5, 2019
Merged

Streams registry #233

merged 1 commit into from Apr 5, 2019

Conversation

@ferjm
Copy link
Member

ferjm commented Apr 3, 2019

No description provided.

@ferjm
Copy link
Member Author

ferjm commented Apr 3, 2019

@Manishearth
Copy link
Member

Manishearth commented Apr 3, 2019

Is there a reason the streams registry is only in the gstreamer backend? I'd imagine that the registry would be common to all backends.

There will be client implementations of MediaStream eventually, for example. (since the DOM can write custom media streams)

@Manishearth
Copy link
Member

Manishearth commented Apr 3, 2019

eventually we'll probably want to switch this over to an event loop model, but that's only for when we move all of servo-media to its own process, which comes much later, and can be done pretty easily.

@ferjm
Copy link
Member Author

ferjm commented Apr 3, 2019

Is there a reason the streams registry is only in the gstreamer backend? I'd imagine that the registry would be common to all backends.

No specific reason. It makes sense to share it among backends. I'll update the patch tomorrow. Thanks!

@ferjm ferjm force-pushed the ferjm:streams.registry branch from fd121de to 2be6cb7 Apr 4, 2019
@ferjm
Copy link
Member Author

ferjm commented Apr 4, 2019

I've just updated the PR with the suggested change.

fn create_audiostream(&self) -> Box<MediaStream> {
Box::new(DummyMediaStream)
fn create_audiostream(&self) -> MediaStreamId {
MediaStreamId::new()

This comment has been minimized.

@Manishearth

Manishearth Apr 4, 2019

Member

Should the dummy backend be registering dummy streams instead? register_stream(Box::new(DummyStream))?

Copy link
Member

Manishearth left a comment

r=me with or without the comment being addressed

@ferjm ferjm force-pushed the ferjm:streams.registry branch from 2be6cb7 to d65ba51 Apr 5, 2019
@ferjm
Copy link
Member Author

ferjm commented Apr 5, 2019

@bors-servo r=Manishearth

@bors-servo
Copy link
Contributor

bors-servo commented Apr 5, 2019

📌 Commit d65ba51 has been approved by Manishearth

bors-servo added a commit that referenced this pull request Apr 5, 2019
Streams registry
@bors-servo
Copy link
Contributor

bors-servo commented Apr 5, 2019

Testing commit d65ba51 with merge 03b771f...

@bors-servo
Copy link
Contributor

bors-servo commented Apr 5, 2019

☀️ Test successful - checks-travis
Approved by: Manishearth
Pushing 03b771f to master...

@bors-servo bors-servo merged commit d65ba51 into servo:master Apr 5, 2019
2 checks passed
2 checks passed
Travis CI - Pull Request Build Passed
Details
homu Test successful
Details
@ferjm ferjm deleted the ferjm:streams.registry branch May 25, 2020
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.