null out currentSource_ on tech dispose and src change #3314

Closed
wants to merge 3 commits into
from

Projects

None yet

4 participants

@gkatsev
Member
gkatsev commented May 11, 2016

Follow up to #3285, we need currentSource_ to be cleared out if we're switching sources on a tech and we're no longer using a source handler. Plus, when the tech is disposed, we should clear out the currentSource_.

@gkatsev gkatsev commented on the diff May 11, 2016
src/js/tech/tech.js
@@ -813,6 +813,8 @@ Tech.withSourceHandlers = function(_Tech){
// than clear all of our current tracks
if (this.currentSource_) {
this.clearTracks(['audio', 'video']);
@gkatsev
gkatsev May 11, 2016 Member

@BrandonOCasey is there a reason why we don't want to just always call clearTracks() here?

@BrandonOCasey
BrandonOCasey May 11, 2016 Contributor

I'm not sure I entirely remember but I think it had to do with saving keeping Audio/Video Tracks that were added before we had a currentSource

@gkatsev
gkatsev May 11, 2016 Member

Ok, I'll see what I can find out. Currently, the tests break because of this.

@gkatsev
gkatsev May 11, 2016 Member

Ended up removing clearing out the currentSource_ in disposeSourceHandler and only doing it inside this if statement.

gkatsev added some commits May 11, 2016
@gkatsev gkatsev only clear out the currentSource_ in setSource if it's set 1e25c63
@gkatsev gkatsev fix naming issue in test case message
1d62d2d
@incompl
Contributor
incompl commented May 11, 2016

Looks good to me

@misteroneill
Member

LGTM

@gkatsev gkatsev closed this in 82d396a May 12, 2016
@gkatsev gkatsev deleted the gkatsev:null-out-currentSource_ branch May 12, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment