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.

Sign up for GitHub

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

Jump to bottom

Cast connection is not recover after create a new CastProxy #768

Closed
avelad opened this issue Apr 28, 2017 · 10 comments
Closed

Cast connection is not recover after create a new CastProxy #768

avelad opened this issue Apr 28, 2017 · 10 comments
Assignees
@theodab
Labels
status: archived Archived and locked; will not be updated type: bug Something isn't working correctly
Milestone
v2.3.0

Comments

@avelad
Copy link
Member

avelad commented Apr 28, 2017 •
edited
Loading

  • What version of Shaka Player are you using? 2.1.0

    • Can you reproduce the issue with our latest release version? Yes
    • Can you reproduce the issue with the latest code from master? Yes

  • Are you using the demo app or your own custom app? Custom app

    • If custom app, can you reproduce the issue using our demo app? This case it's not avalaible on demo app

  • What browser and OS are you using?
    Chrome 58 on Ubuntu 17.04

Error:

Create a CastProxy (and init the player) and load any manifest (eg: https://storage.googleapis.com/shaka-demo-assets/sintel/dash.mpd). Cast (canCast() returns true)and reproduce some seconds and destroy the player (shakaControl.player_.unload()) and then remove video tag of the current DOM (using components with AngularJS). Note: Cast Receiver is avalaible.

Next, add a new video tag (reusing the same component but, inserting it again in the dom), create a new CastProxy (and init the player) and load any manifest (eg: https://storage.googleapis.com/shaka-demo-assets/tos-ttml/dash.mpd). Now canCast() return false and it doesn't reuse the previous conection that it's available.

Note: if I reload the page instead of the previous, it's works.
@avelad avelad changed the title Cast conection is not recover after create a new CastProxy Cast connection is not recover after create a new CastProxy Apr 28, 2017
@theodab theodab self-assigned this Aug 11, 2017
@theodab theodab added type: bug Something isn't working correctly and removed needs triage labels Aug 30, 2017
@theodab
Copy link
Contributor

theodab commented Aug 30, 2017

I believe I have reproduced and found a fix for this bug. The fix will be out soon.
I found your bug report a little confusing, so if this fix does not in fact solve your problem, please let me know.

@joeyparrish joeyparrish added this to the v2.3.0 milestone Aug 30, 2017
shaka-bot pushed a commit that referenced this issue Aug 30, 2017
@theodab
Changed hasReceivers_ into a class variable.
f42edbe 
If you call chrome.cast.initialize a second time, it does not error but it
also does not fire off receiverStatusChanged in order to signal the
initial receiver status. This can result in problems if the CastProxy is
destroyed and then re-created; specifically, it will erroneously claim to be
unable to cast until the receiver status next changes.

This makes hasReceivers_ into a class variable, so that a new sender will use
the hasReceivers_ of previous ones.

The original bug report was kind of confusing, so I cannot say for sure if
this actually solves their problem or not. Hopefully it does.

Issue #768

Change-Id: I7839ed99a8c48c69567bbcaeb1f9b6728265d63b
@joeyparrish
Copy link
Member

@avelad, can you please retest with the master branch? We think this might fix the issue, but we are not certain if we understood the report correctly. Thanks!

@avelad
Copy link
Member Author

avelad commented Aug 31, 2017

@theodab @joeyparrish , I can reproduce the error, I'll try to share with you a code example.

joeyparrish pushed a commit that referenced this issue Aug 31, 2017
@theodab @joeyparrish
Changed hasReceivers_ into a class variable.
7c60e5c 
If you call chrome.cast.initialize a second time, it does not error but it
also does not fire off receiverStatusChanged in order to signal the
initial receiver status. This can result in problems if the CastProxy is
destroyed and then re-created; specifically, it will erroneously claim to be
unable to cast until the receiver status next changes.

This makes hasReceivers_ into a class variable, so that a new sender will use
the hasReceivers_ of previous ones.

The original bug report was kind of confusing, so I cannot say for sure if
this actually solves their problem or not. Hopefully it does.

Issue #768

Change-Id: I7839ed99a8c48c69567bbcaeb1f9b6728265d63b
@theodab
Copy link
Contributor

theodab commented Sep 11, 2017

Those code samples aren't enough for me to personally replicate your issue.

However, I did notice that when destroying the cast proxy, you do nothing with the return value when calling castProxy_.destroy().
CastProxy.destroy() is an asynchronous method, and it returns a promise that is fulfilled when the CastProxy is fully destroyed.
Not waiting for the proxy to be fully destroyed before setting everything to null could possibly be the source of your problem.
You could rewrite that unload method to be something along the lines of...

var destroyPromise = (isAvailable ? castProxy_.destroy() : Promise.resolve());
destroyPromise.then(function() {
  // the rest of the unload method goes here
});

@avelad
Copy link
Member Author

avelad commented Sep 12, 2017

@theodab, I sent you an email with the source code and video with the problem. I hope that help, if not contact me.

shaka-bot pushed a commit that referenced this issue Oct 2, 2017
@theodab
Remove listeners before destroying CastSender.
84bfdd3 
Beforehand, calling chrome.cast.session_.destroy() would typically cause an
error, as the message or update listener was called after the the CastSender
was destroyed.
This removes those listeners before destroying.

Issue #768

Change-Id: I7889adce7b829c3f24dac7a178c9be26e2fdc887
shaka-bot pushed a commit that referenced this issue Oct 5, 2017
@theodab
Store and re-use session_ in CastSender.
e63b4ab 
The cast sender API doesn't seem to be able to re-use an existing session
unless you reload the page. This stores the old session, so that it can be
re-used without reloading the page.

In order to enable this, CastProxy.destroy no longer leaves the current session;
I figure if you want to leave the session, you'll call forceDisconnect.
That part I am not fully sure about; perhaps it would be better to have a
separate optional argument about whether or not to leave, or make it a part of
the forceDisconnect argument.

Issue #768

Change-Id: Ie648372cea4b106ff85df3d0dcc563fca5d10d8c
@theodab
Copy link
Contributor

theodab commented Oct 5, 2017

Okay, I fixed a bug and changed the behavior of the cast sender so that you can re-connect to a session properly after destroying it.
However, that example code you sent me does have some bugs. I've made a modified version with the changes you'll want to make pointed out in comments. I'll send it to your email.

@avelad
Copy link
Member Author

avelad commented Oct 6, 2017

@theodab , thanks for all. I'll check the changes and I tell you if it works (I need some days).

@avelad
Copy link
Member Author

avelad commented Oct 11, 2017

hi @theodab , I just tested the latest commit, and it works perfectly!!! Do you think that it's possible make a cherry-pick to 2.2.3?

@theodab
Copy link
Contributor

theodab commented Oct 11, 2017

It's possible, since these changes are smallish bugfixes and not major feature work, but that decision is up to Joey.

@theodab theodab closed this as completed Oct 11, 2017
@joeyparrish
Copy link
Member

Yes, I see no problem cherry-picking these to v2.2.3, which I hope to release this week.

joeyparrish pushed a commit that referenced this issue Oct 16, 2017
@theodab @joeyparrish
Remove listeners before destroying CastSender.
9e2b739 
Beforehand, calling chrome.cast.session_.destroy() would typically cause an
error, as the message or update listener was called after the the CastSender
was destroyed.
This removes those listeners before destroying.

Issue #768

Change-Id: I7889adce7b829c3f24dac7a178c9be26e2fdc887
joeyparrish pushed a commit that referenced this issue Oct 16, 2017
@theodab @joeyparrish
Store and re-use session_ in CastSender.
d962d89 
The cast sender API doesn't seem to be able to re-use an existing session
unless you reload the page. This stores the old session, so that it can be
re-used without reloading the page.

In order to enable this, CastProxy.destroy no longer leaves the current session;
I figure if you want to leave the session, you'll call forceDisconnect.
That part I am not fully sure about; perhaps it would be better to have a
separate optional argument about whether or not to leave, or make it a part of
the forceDisconnect argument.

Issue #768

Change-Id: Ie648372cea4b106ff85df3d0dcc563fca5d10d8c
@shaka-project shaka-project locked and limited conversation to collaborators Mar 22, 2018
@shaka-bot shaka-bot added the status: archived Archived and locked; will not be updated label Apr 15, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@theodab theodab

Labels
status: archived Archived and locked; will not be updated type: bug Something isn't working correctly
Projects
None yet
Milestone
v2.3.0
Development

No branches or pull requests
4 participants
@joeyparrish @avelad @shaka-bot @theodab