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

Enable 'Prefer fMP4-HLS Container' by default on certain platforms #5289

Merged
merged 2 commits into from
Mar 22, 2024

Conversation

nyanmisaka
Copy link
Member

@nyanmisaka nyanmisaka commented Mar 19, 2024

fMP4 is enabled by default only on verified patforms. This
largely avoids transcoding HEVC and AV1, instead letting
the client handle them.

Changes

  • Enable 'Prefer fMP4-HLS Container' by default on certain platforms
  • Increase the minimum version of Tizen that supports native fMP4 to 5

Issues

  • Many users are unaware that this option exists and it was disabled by default.

@nyanmisaka nyanmisaka requested a review from a team as a code owner March 19, 2024 08:23
@@ -140,7 +140,7 @@ export class UserSettings {
return this.set('preferFmp4HlsContainer', val.toString(), false);
}

return toBoolean(this.get('preferFmp4HlsContainer', false), false);
return toBoolean(this.get('preferFmp4HlsContainer', false), true);
Copy link
Contributor

Choose a reason for hiding this comment

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

We need to disable fMP4 in device profile on some platforms. Then we can remove this option (always enable fMP4).
At least on Tizen 4, I've never been able to get it to work - it always waits for the remuxing job to complete.

Copy link
Member Author

@nyanmisaka nyanmisaka Mar 19, 2024

Choose a reason for hiding this comment

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

So we disable for this version of Tizen? Or enable fMP4 by default only for HLS.js and Safari?

It is unlikely that we will test every version of Tizen and WebOS one by one. I think they should be left to the user's choice.

Copy link
Contributor

Choose a reason for hiding this comment

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

We can trim

if (browser.tizenVersion >= 3 || browser.web0sVersion >= 3.5) {

but agree

It is unlikely that we will test every version of Tizen and WebOS one by one. I think they should be left to the user's choice.

As an option, true -> !browser.tizen && !browser.web0s or list the platforms that play fMP4 for sure. In this case it will be disabled by default on some platforms, but the user can still enable it.

Tbh, I don't know why it works that way on my TV. 🤷 Iirc, I tested fMP4 with AVPlay - same bug.

Copy link
Member Author

Choose a reason for hiding this comment

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

I increased the minimum version from 3 to 5 based on your experience.

-browser.tizenVersion >= 3
+browser.tizenVersion >= 5

Do these older Tizen versions support MSE? If so you can enable HLS.js as a replacement.

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah. Even Tizen 3.0 has been marked as the first version to support fMP4-HLS. Have you also tried the online m3u8 link provided by Apple? If the fMP4 in it cannot be played normally, then this should be a system issue and should be reported to Samsung.

https://developer.samsung.com/smarttv/develop/specifications/general-specifications.html#Streaming-Feature-Support

@nyanmisaka nyanmisaka force-pushed the enable-fmp4-hls-by-default branch from a190904 to e060084 Compare March 19, 2024 09:42
fMP4 is enabled by default only on verified patforms. This
largely avoids transcoding HEVC and AV1, instead letting
the client handle them.

Signed-off-by: nyanmisaka <[email protected]>
@nyanmisaka nyanmisaka force-pushed the enable-fmp4-hls-by-default branch from e060084 to 6875f28 Compare March 19, 2024 09:43
@nyanmisaka nyanmisaka changed the title Enable the 'Prefer fMP4-HLS Container' option by default Enable 'Prefer fMP4-HLS Container' by default on certain platforms Mar 19, 2024
Copy link

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

@jellyfin-bot
Copy link
Collaborator

Cloudflare Pages deployment

Latest commit 6875f28
Status ✅ Deployed!
Preview URL https://4a106afb.jellyfin-web.pages.dev
Type 🔀 Preview

View build logs
View bot logs

@GeorgeH005
Copy link
Contributor

Found this for webOS support. Don't know exactly which tags we use so I am linking the site here: https://webostv.developer.lge.com/develop/specifications/streaming-protocol-drm

@GeorgeH005
Copy link
Contributor

For Tizen, this should be the relevant page: https://developer.samsung.com/smarttv/develop/specifications/general-specifications.html#Streaming-Feature-Support

Sorry if I restated the obvious, found it useful to hhave the relevant links here.

@nyanmisaka
Copy link
Member Author

For Tizen, this should be the relevant page: https://developer.samsung.com/smarttv/develop/specifications/general-specifications.html#Streaming-Feature-Support

Sorry if I restated the obvious, found it useful to hhave the relevant links here.

Unfortunately according to dmitrylyzo's testing, the actual behavior on older Tizen contradicts the documentation. So until someone can fully test it on every Tizen version (3.0+) and WebOS, we're not going to risk breaking playback by enabling it by default.

@thornbill thornbill added enhancement Improve existing functionality or small fixes playback This PR or issue mainly concerns playback labels Mar 22, 2024
Copy link
Contributor

@dmitrylyzo dmitrylyzo left a comment

Choose a reason for hiding this comment

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

I am not sure about Tizen version, but I think it is fine for now. Tizen doesn't really need fMP4.

@thornbill thornbill added this to the v10.9.0 milestone Mar 22, 2024
@thornbill thornbill merged commit 56b9096 into jellyfin:master Mar 22, 2024
12 checks passed
@nyanmisaka nyanmisaka deleted the enable-fmp4-hls-by-default branch March 26, 2024 17:32
@ikheetjeff
Copy link

I've been sticking to version 10.8.13 for a while because higher versions had issues with out-of-sync subtitles. After trying many things, it turns out that disabling Prefer fMP4-HLS Container on Firefox and iOS (Jellyfin Mobile) mostly solves the desync.

After a lot of rewinding and fast-forwarding, desync does occur again, but that also happens on 10.8.13. However, it happens much less quickly and only occasionally. With this feature enabled by default, it almost always happens.

I would like to suggest not enabling this feature by default because, in my opinion, it doesn't work well. I've already made a custom build for myself because otherwise, this version is unusable and I would be forced to revert to 10.8.13. Or maybe I'm missing something crucial?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Improve existing functionality or small fixes playback This PR or issue mainly concerns playback
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

6 participants