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

Prevent destruction of session, so that an app restart can reconnect (chromecast). #1469

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

Tukajo
Copy link

@Tukajo Tukajo commented Aug 25, 2024

This is my first commit but I'd like to keep things small-ish/have targeted improvement.

There is an issue where the controls have repeatedly disappeared for various users:
#459

This change set at least allows the user to forcibly close the app and re-open it, allowing for a reconnection to the session.

The downside is that the session will continue until the user intentionally stops it. Which means that it could potentially continue playing in the background indefinitely unless the server is somehow configured with an "Are you still here?" style of message & session booting based on no user input.

I still think this is better for the time being, because the alternative is that every time I lose control of the chromecast controls, I have to restart the app, which actually stops my TV from chromecasting, and I lose all my progress. This is exceedingly annoying as is.

By comparison, when YouTube is closed while chromecasting, it continues to operate (eventually displaying that "are you still here?" message).

Further notes:

  1. I do not think this should be the final state. I am having trouble trying to figure out how to get the app (on opening) to automatically reconnect to any still-alive session, and re-display the controls automatically. Today the user has to manually navigate the controls again to reconnect, and hit "remote control" again. At least it's better than the entire casting session stopping.

Changes

  • Do not destroy the session when app is closed.
  • Use of implicit this
  • Some build debug changes (can remove these if we want, it just sped up some of my builds).

Issues
#459

@jellyfin-bot jellyfin-bot added this to the v2.7.0 milestone Aug 25, 2024
@@ -190,6 +190,7 @@ class MainActivity : AppCompatActivity() {
fragment.onUserLeaveHint()
}
}
super.onUserLeaveHint()
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was added due to the linter/editor complaining it should not have been left out.

@@ -33,7 +33,7 @@ public final class Chromecast implements IChromecast {
/**
* Object to control the media.
*/
private ChromecastSession media;
private ChromecastSession chromecastSession;
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

changed the variable name to be more "descriptive" as to what this variable actually was. For the longest time I got a little confused and thought this was the actual media, not the session.

org.gradle.jvmargs=-Xmx2G -XX:MaxMetaspaceSize=512m
org.gradle.jvmargs=-Xmx8G -XX:MaxMetaspaceSize=1G
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have a beefier debugging machine, but we can reset this down if folks want. This helped speed up my build times quite a bit.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This might be interesting for you. I'd prefer to keep the lower defaults in this repo.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Definitely I'll remove these changes when I can get back on my computer.

@Tukajo Tukajo changed the title Prevent destruction of session, so that an app restart can reconnect. Prevent destruction of session, so that an app restart can reconnect (chromecast). Aug 25, 2024
@jellyfin-bot jellyfin-bot added the merge conflict Conflicts prevent merging label Sep 1, 2024
@jellyfin-bot jellyfin-bot removed the merge conflict Conflicts prevent merging label Sep 1, 2024
@3flex
Copy link
Contributor

3flex commented Sep 2, 2024

Would this also fix #184? Or is #184 describing this limitation:

I am having trouble trying to figure out how to get the app (on opening) to automatically reconnect to any still-alive session, and re-display the controls automatically. Today the user has to manually navigate the controls again to reconnect, and hit "remote control" again

@Tukajo
Copy link
Author

Tukajo commented Sep 2, 2024

Would this also fix #184? Or is #184 describing this limitation:

I am having trouble trying to figure out how to get the app (on opening) to automatically reconnect to any still-alive session, and re-display the controls automatically. Today the user has to manually navigate the controls again to reconnect, and hit "remote control" again

It will allow you to reconnect. (You can build this yourself to see).

But it's not automatic yet. I was having trouble getting the automatic reconnect to work.

However you can manually reconnect by reperforming the normal steps you would do.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants