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

DOCS/options: altered spdif passthrough warning #12072

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

Conversation

kwerner72
Copy link

The warning for not using spdif passthrough, does not take 3D audio formats into account.
Only when having up to 8 channel sources, decoding to PCM is feasible. With 3D audio formats like Atmos, Auro, ... passthrough needs to be used.

@sfan5
Copy link
Member

sfan5 commented Aug 8, 2023

I think the warning is still applicable, but partially.
Audio passthrough is complex, prone to issues (e.g. #8403) and not supported everywhere. We shouldn't be encouraging it in general unless as you said the user has some fancy proprietary format.

@kwerner72
Copy link
Author

Would agree that in most cases passthrough should not be encouraged.
So instead of removing the warning as a whole, it now has been altered.

@kwerner72 kwerner72 reopened this Aug 25, 2023
@christoph-heinrich
Copy link
Contributor

I don't have any opinion on this change in general, but it says FFmpeg's new DCA decoder. We could get rid of the new, because eventually it won't be new anymore (which is probably already the case, I didn't check).

@sfan5 sfan5 changed the title DOCS/options: removed spdif passthrough warning DOCS/options: altered spdif passthrough warning Aug 27, 2023
@kwerner72
Copy link
Author

I don't have any opinion on this change in general, but it says FFmpeg's new DCA decoder. We could get rid of the new, because eventually it won't be new anymore (which is probably already the case, I didn't check).

Sorry, dont know about the current state of DCA decoder, if it shoud still referred to as 'new' or if it is not needed anymore.

@christoph-heinrich
Copy link
Contributor

I don't have any opinion on this change in general, but it says FFmpeg's new DCA decoder. We could get rid of the new, because eventually it won't be new anymore (which is probably already the case, I didn't check).

Sorry, dont know about the current state of DCA decoder, if it shoud still referred to as 'new' or if it is not needed anymore.

It's not like that sentence becomes invalid when the new isn't there anymore. I don't see any value in it being there, even if the decoder is still recent enough to be considered new, and by removing it now there is no need to do so in the future.

Comment on lines +1908 to +1913
In general it is not recommended to use passthrough, since HDMI
supports uncompressed multichannel PCM, and mpv supports lossless
DTS-HD decoding via FFmpeg's new DCA decoder (based on libdcadec).

Nevertheless there are some cases in which it must be activated for
correct playback, such as 3D audio formats like Dolby Atmos or DTS:X.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
In general it is not recommended to use passthrough, since HDMI
supports uncompressed multichannel PCM, and mpv supports lossless
DTS-HD decoding via FFmpeg's new DCA decoder (based on libdcadec).
Nevertheless there are some cases in which it must be activated for
correct playback, such as 3D audio formats like Dolby Atmos or DTS:X.
Generally, passthrough is not recommended because HDMI supports
uncompressed multichannel PCM, and mpv supports lossless DTS-HD decoding.
However, there are cases where passthrough must be enabled for proper
playback, particularly for 3D audio formats such as Dolby Atmos or DTS:X.

How about something like this? Particularity mentioning "new" (now almost 10 years old) decoder is not important detail.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants