-
-
Notifications
You must be signed in to change notification settings - Fork 43
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
Misc fixes #107
Misc fixes #107
Conversation
1990b76
to
4a8abe2
Compare
Yeah so this means that there is another parameter that is arbitrarily restricted when using HLS: Width. I suspect that this is used to pick the best stream that makes sense in a scenario where the manifest has many alternatives. For example if you loaded up some youtube video, a cast device connected to 720p would only consider streams up to 720p. If you take your fancy CC Ultra and connect it to 720p, it doesn't really make sense for it to load up some 4k video. My guess is that for HLS it doesn't know the difference between something that is not ideal and something that wouldn't play at all. However it would still work for directplay because it doesn't seem to care about this check. This means that we really have to treat direct streamed mkv vs HLS completely differently from a parameter perspective. I also think we need to cap the HLS bitrate based on device because of the inefficient HLS implemenetation. |
6163426
to
feac1e3
Compare
added #105 |
Added #102 |
Added #108 |
Possible regression on playback of some audio files delivered as opus.. |
When changing the transcoding stream (enabling subtitles mid-transcode), waitingcontainer covers the video. |
Waiting on #93 |
still want documentmanager in first |
eca4cdc
to
a9ef049
Compare
I've put this on top of DocumentManager, so this will look nicer after DocumentManager is merged :) |
688fa8e
to
b34cd01
Compare
@YouKnowBlom Any update on the state of this PR?
If the maintainers can provide guidance on the best path forward, I'd additionally like to get the ball rolling and contribute towards bringing this project back to a working state. Please let me know. Although I understand the Android TV app works fine, having the ability to cast to a Chromecast (such as when away from home with friends and family) is an important feature to me - and one that I miss terribly after migrating from Plex. Although I have done everything I can to get it the best chance of working (HTTPS with Lets Encrypt, loopback NAT and split DNS, valid |
@YouKnowBlom @hawken93 @nielsvanvelzen Considering the state of the repository (completely borked), I don't understand the risk in merging this PR. It will likely fix more than it will break. This PR is effectively blocking any further wider contribution to the repository. I hope you'll consider just sending it - but ultimately I understand this decision isn't up to me. If I don't hear anything further, in 10 days I will proceed down the path of creating smaller more manageable PRs from this one. @tnyeanderson Are you still willing to help contribute to this? In 10 days I'll make an issue to track the work of getting this repo back on track. @nielsvanvelzen Are you still happy to review smaller more manageable PRs if we go down this route? |
@itsmejoeeey I'm not sure if setting it via env does something differently than setting it via GUI, but I was able to solve most of my problems by setting this env variable on the docker container running jellyfin: JELLYFIN_PublishedServerUrl=https://example.com/ (replace example.com with your server url) I'm running with nginx proxy manager. |
@sfwr yes that has been known to help in some cases. But in many cases, it's just buggy or flat-out doesn't work anyways. @itsmejoeeey At this point, I am going to say that I do not think that "sending it" is a good answer here. You can posit that this might fix more than it breaks, but how was that determined? This PR is so complex and old that merging it in at this point honestly seems dangerous, unless someone can explain in detail what it does and can thoroughly test it. It seems clear to me that hardly anyone really knows exactly what this PR does, we are just desperate for a fix. And I am too! I am absolutely willing to contribute smaller PRs. I think we need to start by dissecting this PR and figuring out all the changes it proposes. Then we should make manageable, atomic goals that can actually be met, reviewed, and merged in a reasonable time. |
So as you've probably noticed the casting support is not great. This is mainly because none of our contributors use it (most use the native Android TV app that I maintain) and developing for the cast framework is a pain. I'd be willing to help out merging the smaller pull requests but I'm not able to do releases/troubleshooting/complex reviews as my knowledge about the framework is too low. We need to find solutions for that in the long term. |
I would like to add to the discussion topic that there are those of us who choose to stick to Chromecast devices (in the classical sense) and do not want to switch to Android TV or other such devices. Not trying to say anyone related to Jellyfin stuff is pressuring me (or others) to make the switch, more-so that there are users who intentionally plan (such as myself) to not have a future-path to migrate away from said devices. I have no real way of telling how many there are like this, but I know there's at least one ;) Either way, I hope Chromecasty stuff for Jellyfin (and related client apps) gets to the point of actually being usable, as in my exhaustive testing (and yes I know I have a complex setup too) I cannot get more than two songs to play before playback just refuses to continue. |
Like others in this thread I am quite interested in the webreceiver as well. Actually, I have created my own version for testing by merging https://github.com/YouKnowBlom/jellyfin-chromecast/tree/misc-fixes-tnye-merge with the current unstable, and that works remarkably well. For me many major bottlenecks are removed, and it is much, much better than the current unstable (and in fact, the stable released version). But that might be due to me having only a simple cast setup. A lot of issues obviously will remain, but hey, no application without bugs ;-) From my point of view, proceeding with the YouKnowBlom/tnyeanderson branch seems easier than taking the current unstable as starting point, but that may well be a personal perspective. I do not understand everything of that branch, but neither do I of the unstable. I intend to continue with gradually fixing some more in my clone, even if it is only out of interest. I am obviously quite willing to share my fixes if others are interested, but like @itsmejoeeey not sure how to do that. Create my own fork for that? Based on the unstable, or on YouKnowBlom's fork? If another approach is chosen, I of course don't mind to follow that as well. |
I will help out with the pull requests opened in this repository targeting the master branch. It should be fine to use the existing open pull requests and fix them (fix conflicts/bugs/pending review comments etc.). All branches from forks that have no pull request to this repository will not be accepted without explicit from the owner of the fork. When there has been some activity I will discuss future steps for the repository with the Jellyfin team and make the results public. |
It warms my heart that it looks like this is going in a productive direction <3 |
Oh man, it was clearly a mistake when like 18 months ago, I set my notifications for this project to "All Activity". It was maybe one email a month until today. Then, BAM! Glad to see things are moving again! |
Haha sorry about that, I started with some dependency updates but there are a lot. Should be less busy from now on. |
I'll try to submit an appropriate PR after the weekend for review :-) |
As the present PR contains a lot of commits already, it seems cleaner to create a separate PR to just implement the YouKnowBlom/tnyeanderson branch. That PR is #465. |
Thank you @Sky-High! I'm still working on getting access to the cast developer console. I was billed twice but didn't get any access yet so I have to deal with Google Support to get that fixed. You might have also noticed that all dependency updates I did have not been deployed to the unstable instance, this is because the SSH key expired. I've rewritten the deployment workflow for GitHub Actions (#464) and am working with our infrastructure & secret people to get that running again. |
@nielsvanvelzen thanks. I was indeed wondering: jellyfin-web refers to two registered webreceiver app versions Is the unstable version with applicationID=6F511C87 automatically available when a PR is merged? I suppose the 'dist' for this is somewhere at a jellyfin server, altough I don't know the URL. |
Yes, the "unstable" option is automatically deployed from the master branch and the "stable" one is deployed from tags (what you see in the releases tab). They are deployed to apps1.jellyfin.org. In 10.9 you will be able to customize the available chromecast clients if you want to self-host them / use a developer version / use a third-party one. See discussion here. |
Yay to self-hosting! \o/ |
🥳🎉 Exciting! Big appreciation to the many people that have worked hard to get to this stage. And a big thanks to @Sky-High and @nielsvanvelzen for getting it across the line (especially considering the latter doesn't even personally use this project). |
How soon before us non-dev plebs can see the fixes in Jellyfin for Chromecast so I can test and see good/bad/ugly results? Also, would I be looking for the stable or unstable version to select? 🤔 |
After a pull request is merged it should be live in about 10 minutes. You need to use the unstable version. |
@nielsvanvelzen okay so I finally got around to testing/validating the "Unstable" Chromecast App in the Jellyfin app. Here's what I found:
So what I did to do blocking, I took the MAC address for the Chromecast I wanted to affect, and created a static IP assignment in my gateway (pfSense), so the DHCP server gives it the same IP every time. In this static IP assignment (via DHCP Server) I also explicitly declare only the gateway as the DNS server, and no other DNS servers (on the internet or on LAN). I then went over to my friendly-neighbourhood Firewall > Rules section of pfSense, then for the LAN interface added a "Pass" rule with the following parameters (it's at the bottom of the list so I don't think it needs to execute first):
Then hit save, and APPLY. It also just occurred to me as I was writing this that I should also validate other Chromecast clients if they can still work against the Chromecast in this configuration, and these apps worked, one on LAN, the other from the internet:
At this point it looks like EVERYTHING WORKS! Oh, I also went to the states section of pfSense and confirmed that the affected Chromecast is having it's DNS traffic to 8.8.8.8 and 8.8.4.4 explicitly and 100% reliably redirected to the 127.0.0.1 IP. Now, I really do appreciate all this work and such! Here are some areas I'm interested in improvement for Jellyfin stuff:
Either way this looks 100% workable!!!! Thanks so much! :))) |
This is probably because of your setup. Most apps will set-up a tunnel between their cloud and your local server, so all traffic from your chromecast goes trough that instead. Jellyfin does NOT use cloud servers and we have NO way of sniffing this traffic. The cast receiver needs to connect to your server and to do that it needs to be started with a valid host (DNS resolvable on the public internet or an reachable ip address). 10.9 includes many fixes to prevent the web client from sending "localhost" or "127.0.0.1" as address solving most connectivity issues. I have a prototype branch already that also displays the server address below the "Ready to cast!" text to help figuring out what address is sent. I'd also like to ask everyone to use proper issues from now on instead of this pull request. There is also the option to use our Matrix/Discord chats for quick questions. |
Trying to collect small fixes here while the big PR is merged.
Will rebase this after.
Currently in: