Enable hls.js with WebOS #6678
Enable hls.js with WebOS #6678
Conversation
Cloudflare Pages deployment
|
|
thornbill
added
p: webos
|
What does this "needs testing" mean? Looking other "needs testing" pull requests does not bring any hopes up. I have used weeks to test this with different scenarios (and I helped to make hls.js to handle Dolby Vision better to make this patch feasible). There seems to be at least few who have tested it on forum post that is mentioned in my pull request. Even though there is one person who does not seem to be fond of it. |
|
It works for me. Please consider it as testing. |
This is probably a bug in either ffmpeg or hls.js. Looking at the chromium sources webOS MSE does support AAC, MP3, Vorbis, AC3, EAC3, OPUS, FLAC, AC4, DTS(E/X) (only on select models). Supported video codecs are H264/5 VP8/9. |
Problem is WebOS. If you use isTypeSupported for audio/mp4; codecs="whatever" then you get true in those formats. If you use isTypeSupported for video/mp4; codecs="whatever-video,whatever-audio" then supported audio codecs are much more limited. For that reason MSE does not play videos with AC3, but it does play plain audio with AC3. I have talked about this issue on mentioned forum post and also on first "fixes" ticket I list. Also the solution would be to separate audio and video to their own mp4 streams (which hls allows). That would solve also other problems. Fixing that problem is just not required to make this pull request a must have for every WebOS user that have multichannel audio system at home. |
Maybe it's just the check that's broken? Did you try removing isTypeSupported from hls.js? |
Yes, I have tried to ignore check, but actual playback request using MSE does not work with eac3 either. EDIT: unless there is some sort of trick with addSourceBuffer not taking mimeType or taking some sort of generic text. I have given that function a proper mimeType and it still fails with eac3. I can't remember if I tested to just give video/mp4 without codecs. Maybe I need to test again. EDIT2 (to not fill this pull request with too many messages): Settings mimetype as video/mp4 without listing codecs is error on Chromium including WebOS Chromium. So I have no idea how I could try to start MSE playback without giving codec that it marks as unsupported. |
|
Is there any update to this being implemented? |
I don't know what I can do to make this reviewed. There is not much code to there, but quite a lot of testing to make sure it is good patch to offer. I have also explained why it is made like that and so on (here and in that forum thread). Of course I am not able to test other WebOS televisions (and WebOS versions) than my own so there is that. |
|
i tried it by manualy compiling it and put it in my server. indeed, the sound problem was fixed, but then i had issues with dolby vision :( |
You need to set "Prefer fMP4-HLS Media Container" as enabled when using this patch and want to have Dolby Vision working. It is mentioned on my first message (not that clearly though). Without this patch you need to have it disabled to have Dolby Vision working so it is probably disabled for you at the moment. |
|
This PR is helpful due to the numerous bugs in LG's HLS. However, killing Dolby Atmos makes it unfit for merging, in my opinion. An alternative is to enable hls.js only when content is delivered through HLS (e.g., when Prefer fMPG-HLS Media Container is selected). This would keep the default behavior of using LG's implementation of ts. Recently, on webos forums, we learned that LG is reviewing the fix for the bug that originated the need for this PR, but it will take some time to arrive through an update. |
Obviously Dolby Atmos works like before if HLS is not needed (if video is in container that can be played directly). I don't see we lose anything when using this patch. I wrote in my forum post (linked in my initial message) about possible way to make Dolby Atmos work too, but I do not have motivation to try to fix that since this pull request (and my forum post) is ignored even when fix is very much needed. If LG actually fixes this problem then this patch is not needed to make multi channel audio to work, but other problems still remain. For example that pause issue and fast forward issue. There was some sort of patch for those already implemented, but apparently it does not fully fix those problems. |
How is it "killing Dolby Atmos"? Dolby Atmos doesn't work with native HLS on LG WebOS, so this fix can't be "killing" something that is already "dead" :-p Also, timitt has provided a solution to the Atmos issue (separating video and audio streams). So, I would suggest the Jellyfin developers try to implement that change instead of leaving it up to LG to (maybe) fix it in a future update. Overall, I would argue that we should move away from the default behaviour of LG, given how broken their software is. Given that the issue is at least a year old (affecting WebOS 23 and 24), would it not be prudent to move away from utilising native software when Jellyfin can utilise its own version? Case in point, LG's implementation of hls.js broke Jellyfin for all LG TVs. It would be best practise to use embedded components in the Jellyfin-web client code, instead of relying on the native components provided by TV manufacturers. Clearly the likes of LG, Samsung, Sony, etc. can break Jellyfin during any of their numerous firmware updates, so the less Jellyfin relies on native software, the better. |
I feel your pain! The number of open pull requests and issues on Github, many dating back months (and even years), is frustrating. The devs really need to set up a testing repository so that fixes can be tested and signed off quicker. |
|
Would be nice to finally play HLS on my very expensive LG OLED... |
Trust me, I don't think anyone is more frustrated by the current state of reviews in this repo than me... this project with hundreds of thousands of users has around 2 regular contributors and the vast majority of reviews are done by just me. I am a volunteer the same as every other contributor with a day job, family, other life responsibilities, and other projects that I maintain within the Jellyfin ecosystem. I also struggle with getting reviews on my own PRs since I do the majority of reviews and obviously cannot review my own PRs. I am more than open to any suggestions on improving the process. PRs dealing with specific platforms can also be challenging as I have no webOS devices to test or verify this change. In the past, I relied pretty heavily on another contributor that tested TV specific changes, but they seem to be less active of late. It does look like a few people have tested and verified this change besides the author now, so I do agree that it can be considered tested, but generally I don't know how to communicate this to webOS users that a PR requires testing beyond using GitHub labels.
Again I'm welcome to any specific ideas on this front. We do have preview builds published to Cloudflare pages for every PR that is opened and of course anyone is able to pull the changes themselves, but if you have additional ideas please share them or join the matrix/discord chat to discuss it! Thanks for your patience. |
thornbill
force-pushed
the
enable_hlsjs_with_webos
branch
from
0318e1e to
85fc9de
Compare
|
Your (and other volunteers) work is much appreciated. Especially as I know that I would not be able to put myself to your spot and be active contributor.
For me the issue seems to be that there are several different offered ways to communicate about possible fixes or issues and the ones I have used does seem to be not read (or at least answered by anyone that does any development). Of course if there is mostly you then there is obviously too much work for one (or two) person. I did not try Matrix as I am on different timezone (Finland) but forums does not seem to have devs talking too much. Github discussions seems to be quite quiet too. So only way seemed to be to just do pull request.
I should probably have made builds to my repository with patch to make testing easier, but then again I myself find precompiled stuff from random people (like I am to all of you) bit dangerous. Still that would probably added more testers especially if I would have guessed it could help.
Thank you for merging. I believe that we have many more happy users now. I also hope that all those "probably fixes" tickets got fixed by this pull request. I could not confirm them even though they do seem to be ones that got fixed too. |
| @@ -48,6 +48,12 @@ export function enableHlsJsPlayer(runTimeTicks, mediaType) { | |||
| return false; | |||
| } | |||
|
|
|||
| // Native HLS support in WebOS only plays stereo sound. hls.js works better, but works only on WebOS 4 or newer. | |||
| // Using hls.js also seems to fix fast forward issues that native HLS has. | |||
| if (browser.web0sVersion >= 4) { | |||
There was a problem hiding this comment.
Shouldn't this be webOsVersion (e.g. a capital O, not a Zero)?
There was a problem hiding this comment.
No, the variable name is correct like this
There was a problem hiding this comment.
Is this some jellyfin infill, since the only GitHub search hits are in these in these (and clone) repos...
There was a problem hiding this comment.
Maybe you've been searching wrong?
jellyfin-web/src/scripts/browser.js
Line 296 in 855276e
There was a problem hiding this comment.
I have no idea why it is written with zero, but it is certainly correct.
There was a problem hiding this comment.
This is just because of the UserAgent string:
https://webostv.developer.lge.com/develop/specifications/web-api-and-web-engine
Mozilla/5.0 (Web0S; Linux/SmartTV) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/68.0.3440.106 Safari/537.36 WebAppManager
There was a problem hiding this comment.
Is this some jellyfin infill, since the only GitHub search hits are in these in these (and clone) repos...
Maybe you've been searching wrong?
jellyfin-web/src/scripts/browser.js
Line 296 in 855276e
That's in a JellyFin repo...
There was a problem hiding this comment.
Not sure if I understand correctly. jellyfin-webos is just a slim wrapper around jellyfin-web so it's correct that this code is in jellyfin-web.
There was a problem hiding this comment.
Not sure if I understand correctly. jellyfin-webos is just a slim wrapper around jellyfin-web so it's correct that this code is in jellyfin-web.
It's fine, I was previously unaware that the Web0S was driven by the Agent string (per @dmitrylyzo 's comment and thus asked a question about if the spelling was intentional. I did a Google and GitHub search to see if that spelling was used elsewhere and found that it was unique to the jellyfin-web repo (and the forks) and it was only here and being that it was browser.web0sVersion , I was concerned that no browser actually implemented that.
I didn't check to see that it was actually a infilled method declared in brower.js and filled into the global browser scope because I'm not in the habit of adding things to the global browser scope except for polyfills... and since this was not a spec-mentioned thing, I was surprised.
I am very sorry for wasting so much time just asking a question... I should have dived into the code first. Mea culpa.
Changes
This patch will put hls.js in use when using LG WebOS 4 or newer. Native HLS in WebOS seems to be broken in many ways. Worst way is that it breaks multichannel audio and everything plays as stereo. External amplifiers that are connected to TV through HDMI ARC or Toslink fail to show Dolby Digital marking no matter what channel count is forced to be used when native HLS is in use.
Note that we need 1.6.0 or newer version of hls.js to use this patch if we want Dolby Vision 8 to work.
Hls.js 1.6.0 now supports supplemental codecs and it also has much better support for Dolby Vision files generally.
Also note that users should enable use of fMP4 when this patch is enabled unlike with native hls.
Possible negatives with this patch is the fact that we need to transcode AC-3 and EAC-3 when HLS is needed as WebOS MSE does not support playing videos with those audio codecs.
It also kills Dolby Atmos, but I am not sure if it worked correctly with native hls either (at least it did not work when using HDMI ARC).
There is possible solution for that too here: https://forum.jellyfin.org/t-webos-with-hls-js
Issues
Fixes jellyfin/jellyfin-webos#275
Fixes #6522
Probably fixes jellyfin/jellyfin-webos#248
Probably fixes jellyfin/jellyfin-webos#285
Probably fixes jellyfin/jellyfin-webos#284
Relates jellyfin/jellyfin-webos#278
Relates jellyfin/jellyfin-webos#274
Relates jellyfin/jellyfin-webos#262
Relates #6533
Relates #6484