move CastPlayer re-initialization to correct method #4862
Conversation
|
I believe this is also fixed in #4843 |
|
Ah, ok. You attach the initialization to the login event, so that is probably ok. |
|
My changes were primarily focusing on jellyfin/jellyfin-meta#45, I had to move the initialization because I needed the user settings. I chose to do it when a user is logged in because that way the data is already loaded when opening the "play to" menu. I'm not sure if it's the best way and honestly I'm too unfamiliar with the jellyfin-web code to think of one. |
|
I am not very familiar with Jellyfin in general, but learning as we go. At this moment I try to be useful for chromecast functionality. As to this PR, I attached the initialization to the Cast button, which is the tryPair method. This is also rather arbitrary, I guess. The ChromecastPlayer class apparently implements the Jellyfin notion of a player, while the CastPlayer class implements the Google CastSDK notion of a player. The functions in between aim at relaying between both classes. Initializing of the Google CastSDK seems quite tricky to me. It is performed by CastPlayer initializeCastPlayer, after loading the Google CastSDK library in ChromecastPlayer constructor with CastSenderApi().load(). I assume that the latter should be done at a fairly early stage of starting jellyfin-web, a may only be performed once. Calling CastPlayer.initializeCastPlayer probably is less demanding. Another issue to consider is that the jellifin-web code is also used in jellyfin-android. But you of course know better than me what that means with respect to calling CastSDK functionality. And by the way: design documentation is very scarce. Are there guidelines for where to store documentation? In the source code? Or in the PR's? |
|
Kudos, SonarCloud Quality Gate passed!
|
Cloudflare Pages deployment
|
|
Hey @Sky-High sorry I am a bit behind on reviews since we have had a lot come in in the past couple weeks now. Unfortunately there isn't a lot available in terms of documentation. There has been some discussions about having a developer focused documentation website but it doesn't exist yet. If you have any ideas on how to improve this or have any questions, I am almost always available on the matrix/discord. |
|
@thornbill , thanks! Setting up a full fledged documentation process is a lot of work (my experience is that proper documentation takes more effort than writing code itself), but it would already help if there is at least some place where one can just leave pieces of documentation for others to browse. Something like a blog, or may be something like jellyfin-meta would already help. |
Move the re-initialization call of the CastPlayer to the Cast button handler, as currently it is called too late.
Changes
The CastPlayer class is the interface of the Jellyfin ChromecastPlayer to the Google Cast SDK. The CastPlayer instance is created and initialized during startup of jellyfin-web. See
jellyfin-web/src/plugins/chromecastPlayer/plugin.js
Line 89 in c3c6ebe
Also when a new media is played CastPlayer is re-initialized. See:
jellyfin-web/src/plugins/chromecastPlayer/plugin.js
Line 533 in c3c6ebe
jellyfin-web/src/plugins/chromecastPlayer/plugin.js
Line 109 in c3c6ebe
The second CastPlayer initialization when starting to play a new media on the other hand is actually too late. It is triggered by the acknowledgement event 'playbackstart' of the jellyfin-chromecast cast receiver app when the media has started to play properly. So at this moment the media is already being played, and initialization therefore is 'after the fact'.
Therefore, this second initialization is moved the moment where the user selects and connects the cast receiver by means of the Cast button. See
jellyfin-web/src/plugins/chromecastPlayer/plugin.js
Line 590 in c3c6ebe
At this point the user is logged on, so the user specific settings are available. And it is also before a media is played, so initialization precedes the actual play of a media, as it should.