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

Fix HEVC allowed in transcode profile when not supported #3875

Merged
merged 2 commits into from
Aug 10, 2024

Conversation

PriceChild
Copy link
Contributor

Excludes Fire TV 1st Gen from requesting HEVC transcodes by device model.

Changes
The Fire TV 1st Gen doesn't support HEVC. jellyfin-androidtv detects as such but currently still ends up requesting a transcode to it. This MR offers working video+stereo audio, but despite support for 5.1 EAC3 & AAC only downmixing to stereo works.

Issues
Fixes #3842

@PriceChild
Copy link
Contributor Author

PriceChild commented Aug 10, 2024

I appreciate that this isn't the right way to fix this problem, DeviceUtils being discouraged. But I hope this partial solution to broken playback on my device will spur someone to support me finding the real problem & solution?

It also doesn't solve audio issues... 5.1 AAC passes is always being downmixed to stereo. If 5.1 EAC3 is passed direct, the video reverts to a frame each ~2s. If "downmix to stereo" is selected, it's fine. Enabling/disabling the "Bitstream Dolby Digital audio" checkbox doesn't change anything. See #3842 for more.

Part of me feels I should buy a new device released more recently than 10 years ago and this problem would go away... but I don't want to put perfectly good hardware in the bin!

@nielsvanvelzen
Copy link
Member

nielsvanvelzen commented Aug 10, 2024

Thanks for trying to figure this out on your own. My issue backlog is extremely big right now (and unfortunately keeps growing) so I wasn't able to read into it yet.

From my understanding your issue is that the client knows it doesn't support HEVC but still adds HEVC as a possible video codec in the transcoding profile, is that right?

The line you changed was wrong because that check will always succeed. In 0.17 we always add a codec profile for HEVC so if (deviceHevcCodecProfile.ContainsCodec(Codec.Video.HEVC, Codec.Container.TS)) will always be true. We should instead change that line to something like if (MediaTest.supportsHevc()).

edit: to be more specific, a new (lazy-)variable should be added in ProfileHelper to cache the result of the supportsHevc function. Then use that in the ExoPlayerProfile & ProfileHelper.

If you're interested you can update this PR to do that (and test it), otherwise let me know and I'll open a new pull request. Thanks for the help!

@PriceChild
Copy link
Contributor Author

@nielsvanvelzen Thank you very much for your help, I do appreciate your time and effort!

I've updated this MR and tested it again works fine for me. I hope this is what you intended (my Java experience is poor and Kotlin non-existant!) I don't have a more modern device available to test the other code path either, that it does enable HEVC for others when supported?

It also still doesn't solve my problem of H264+5.1 EAC3 giving perfect audio but broken video which I'll continue to play around with and treat as a separate issue.

@nielsvanvelzen
Copy link
Member

nielsvanvelzen commented Aug 10, 2024

Yes this is exactly what I meant. One additional change would be to use the supportsHevc property instead of calling MediaTest.supportsHevc() in the deviceHevcCodecProfile (so line 76).

I'm quite sure it will work fine on HEVC capable devices but I can test that part.

Also, don't forget to mark the PR as ready for review so I can merge it!

@PriceChild PriceChild marked this pull request as ready for review August 10, 2024 18:11
@PriceChild
Copy link
Contributor Author

PriceChild commented Aug 10, 2024

Tested on my fire tv 1st gen and still works fine for that case. Transcodes to h264 as desired.

@nielsvanvelzen
Copy link
Member

I've added one change that to re-use the new property in one more place. Otherwise these changes are working fine. Thanks for helping out!

@nielsvanvelzen nielsvanvelzen added this to the v0.18.0 milestone Aug 10, 2024
@nielsvanvelzen nielsvanvelzen added bug Something isn't working backportable Change may be backported to a point release (remove label once cherrypicked) labels Aug 10, 2024
@nielsvanvelzen nielsvanvelzen changed the title Don't request HEVC transcode on Fire TV 1st Gen Fix HEVC allowed in transcode profile when not supported Aug 10, 2024
@PriceChild
Copy link
Contributor Author

Brill, thank you & sorry for missing the other spot! Will look at the broken 5.1 again now...

@nielsvanvelzen
Copy link
Member

Oh whoops. Just noticed before I wanted to merge that you're targeting the release branch. You should switch the target to the master branch instead. Can you fix that?

@PriceChild PriceChild changed the base branch from release-0.17.z to master August 10, 2024 19:58
@nielsvanvelzen nielsvanvelzen enabled auto-merge (rebase) August 10, 2024 20:05
@nielsvanvelzen nielsvanvelzen linked an issue Aug 10, 2024 that may be closed by this pull request
@nielsvanvelzen nielsvanvelzen merged commit 6111cbe into jellyfin:master Aug 10, 2024
6 checks passed
@PriceChild PriceChild deleted the fire-tv-1-disable-hevc branch August 10, 2024 20:06
@PriceChild
Copy link
Contributor Author

Brill! Built and tested master and it is transcoding hevc fine still thankfully.

@nielsvanvelzen nielsvanvelzen modified the milestones: v0.18.0, v0.17.2 Aug 11, 2024
@nielsvanvelzen nielsvanvelzen removed the backportable Change may be backported to a point release (remove label once cherrypicked) label Aug 11, 2024
@nwg5817
Copy link

nwg5817 commented Aug 29, 2024

Did this PR not break NVidia Shield functionality? For weeks now the shield will transcode HEVC profiles that it's capable of running. Switching to VLC it direct plays fine. We fixed the fire stick to break the shield and other devices who could play hevc and are not reporting correctly. Can we not do exactly as Plex did and just add an h265 hevc override in the playback settings.

@PriceChild
Copy link
Contributor Author

That's annoying if true.

It would mean that org.jellyfin.androidtv.util.profile.ProfileHelper.supportsHevc isn't correctly detecting Hevc capability on the shield?

Let's raise a new issue for that specifically.

@nwg5817
Copy link

nwg5817 commented Aug 29, 2024

I tested the prepatch app and still getting transcoding. If you use external VLC it direct plays. Instead of trying to get a device profile at all, because so many of them report incorrectly, we should have a drop down selection of hevc level or off. It's the same case on Plex where you have to know the device and have to configure a few settings. That or every device that has issues is going to need custom profiling in jellyfin.

There's really no solution right now when you have a shield, VLC external is the only option and that is not ideal.

@PriceChild
Copy link
Contributor Author

PriceChild commented Aug 29, 2024

This should go into a separate issue. By all means link this ticket as the one that introduced the bad behaviour as context, but it's not going to get sorted in this issue.

(My personal opinion is that unnecessary transcoding on one device is better than another device being entirely broken, but agree it needs fixing and will help!)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

0.17.x fire TV 1st gen HEVC support claimed but unsupported?
3 participants