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

Shutdown audio contexts and players in all the examples #275

Closed
wants to merge 2 commits into from

Conversation

@ferjm
Copy link
Member

ferjm commented Jul 5, 2019

No description provided.

@ferjm
Copy link
Member Author

ferjm commented Jul 5, 2019

r? @ceyusa

@ferjm ferjm force-pushed the ferjm:examples.shutdown branch from 0714276 to 57f8eaf Jul 5, 2019
@ceyusa
Copy link
Contributor

ceyusa commented Jul 5, 2019

lgtm

@bors-servo r+

@jdm
Copy link
Member

jdm commented Jul 5, 2019

Out of curiosity, what is the current behaviour without these changes?

@ceyusa
Copy link
Contributor

ceyusa commented Jul 5, 2019

Out of curiosity, what is the current behaviour without these changes?

AFAIK the media elements in the examples are not removed from the global hashmaps.

@Manishearth
Copy link
Member

Manishearth commented Jul 5, 2019

Why not have a destructor on the context handle that sends a shutdown message on death?

@ferjm
Copy link
Member Author

ferjm commented Jul 5, 2019

Out of curiosity, what is the current behaviour without these changes?

AFAIK the media elements in the examples are not removed from the global hashmaps.

Essentially that. Although these are weak references to the media elements. And they may be removed on following mutes/unmutes.

Why not have a destructor on the context handle that sends a shutdown message on death?

Could you elaborate on this, please? We can improve this by making the Player/AudioContext send such a message to the owner GStreamerBackend on drop. I am not sure if that's what you meant.

@ceyusa
Copy link
Contributor

ceyusa commented Jul 5, 2019

Just to clarify, the Backend, once instantiated, it is kept alive until the browser shutdowns, then we should remove the media elements when they are removed of the webpage while the browser keeps running.

@ceyusa
Copy link
Contributor

ceyusa commented Jul 5, 2019

bors-servo ignores me :(

Well, perhaps it is better to clarify these questions before merging :)

@ferjm
Copy link
Member Author

ferjm commented Jul 8, 2019

Closing in favour of #279

@ferjm ferjm closed this Jul 8, 2019
@ferjm ferjm deleted the ferjm:examples.shutdown branch Jul 8, 2019
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

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