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 DTS playback - ask for transcoding when unsupported #3929

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

rickysixx
Copy link
Contributor

Changes

Check if DTS audio codec is supported by the device before adding it to the supported codec list.

Probably a similar check should be done also for other "advanced" audio codecs (e.g. Dolby TrueHD).

Issues

Fixes #3896, #3868

@jellyfin-bot jellyfin-bot added the merge conflict Conflicts prevent merging label Aug 25, 2024
Copy link
Member

@nielsvanvelzen nielsvanvelzen left a comment

Choose a reason for hiding this comment

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

I'm not sure how this will behave when using an external audio receiver with DTS support on a TV without support. Would it wrongly think it can't playback DTS?

@MichaelRUSF
Copy link
Contributor

I implemented these changes and it did not work as expected. It caused all DTS to transcode on my Gen2 Firetv Max which supports DTS. On my TV which also supports DTS it played the file directly.

Stream #0:1(eng): Audio: dts (DTS), 48000 Hz, 5.1(side), fltp, 1536 kb/s (default)
Stream mapping:
  Stream #0:0 -> #0:0 (copy)
  Stream #0:1 -> #0:1 (dts (dca) -> aac (libfdk_aac))

Most of the issues I've encountered have been with high-end resolution DT XLL files. And that playback issue has been solved by the transcode after direct play fails twice error handling.

@rickysixx
Copy link
Contributor Author

Thank you very much for your test @MichaelRUSF. My current setup does not support DTS in any form so I am not able to test that scenario.

In my case the change that forces transcoding after 2 playback errors does not fix the issue (the player closes after 3 errors when I try to play DTS audio).

Since listing supported MediaCodec does not always work, in the next days I will try to come up with a different solution.

@jellyfin-bot jellyfin-bot removed the merge conflict Conflicts prevent merging label Sep 13, 2024
@rickysixx rickysixx force-pushed the fix-dts-playback branch 2 times, most recently from 8c113f3 to 0c49d4d Compare September 17, 2024 18:02
@rickysixx
Copy link
Contributor Author

I have pushed another set of changes to check for DTS support in another way.

This code surely need some improvements/refactoring, but first I need to know if it actually works as expected also for DTS-capable hardware (I cannot test this directly).

I have already tested it in case of no DTS-capable hardware and it correctly triggers a transcode.

@MichaelRUSF
Copy link
Contributor

Re-tested and DTS support is correctly identified for both devices now, no transcoding. High-resolution DTS audio still errors out on the FireTV, but that's not b/c of this PR.

This change should help out devices that lack DTS support.

@rickysixx rickysixx force-pushed the fix-dts-playback branch 2 times, most recently from eb50f38 to cb6f05a Compare September 21, 2024 11:03
@rickysixx rickysixx force-pushed the fix-dts-playback branch 2 times, most recently from df4f1a2 to 72f9abc Compare September 21, 2024 13:55
@rickysixx
Copy link
Contributor Author

Thank you for testing this Michael.

@nielsvanvelzen Could you please take a look at this PR when you have a moment?

@nielsvanvelzen
Copy link
Member

@rickysixx can you address the review comments instead of constantly force-pushing the same changes over and over again?

Currently there is no reliable way to check if DTS audio is supported by
the device or by others it's connected to, therefore DTS playback must be
enabled/disabled manually by the user.

`false` is the safest default value for the preference, because it allows
media to be always played without errors on any device, even though it will
trigger a useless transcode for devices that do support DTS.

This commit reverts 38589ef, but now
the preference does have a logic.
@rickysixx
Copy link
Contributor Author

rickysixx commented Oct 20, 2024

I have pushed a new version of the code.

I was not able to find a reliable way to check for DTS support that

  1. considered both the device itself and the ones it is connected to and
  2. worked also for Android 5

I have also tried to use AudioCapabilities, but it didn't work as expected (it advertised DTS support for my device, which it's not the case).

Therefore I think the safest option is to add back the preference to enable/disable DTS playback (which it's there in the web player too) and let the user take care of it.

I'm aware that the same preference has been removed in 38589ef, but that was because the preference was never used. Now that it does have a logic, I think it's appropriate to have it back in the lack of automatic, reliable, DTS support detection.

Sorry for all the useless force pushes in these months @nielsvanvelzen. They were just to keep my fork up to date while waiting to have a moment to work on this PR. I was not aware that they triggered a notification to you.

@jellyfin-bot jellyfin-bot added the merge conflict Conflicts prevent merging label Nov 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merge conflict Conflicts prevent merging
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Movies with DTS audio: playback error
4 participants