AE: let sink decide what passthrough formats it wants to support #4177

Merged
merged 2 commits into from Feb 11, 2014

Conversation

Projects
None yet
6 participants
Member

FernetMenta commented Feb 9, 2014

audio formats in device info list were ignored for passthrough because ELD can be wrong. Instead of ignoring this info, sinks can push desired formats to the list.

@popcornmix @jmarshallnz
now you can exclude DTSHD or EAC3

@wsoltys
How does WASAPI get this information? Is a change required?

Member

fritsch commented Feb 9, 2014

No change needed on the PASink if I see it right. AC3, EAC3, DTS is added to the list you query iirc.

Member

wsoltys commented Feb 9, 2014

Nice. I always wondered why we use m_dataFormats only to print supported formats in the logfile.
The wasapi sink tests which formats it supports and pushes valid formats to m_dataFormats. So no change needed.
DS is kept at the short line and only pushes AE_FMT_FLOAT and AE_FMT_AC3 but could probably support more.

MartijnKaijser added this to the Pending for inclusion milestone Feb 9, 2014

Member

jmarshallnz commented Feb 9, 2014

Great. OSX should be fine with this change. I'll need to check iOS ofc.

@wsoltys if DS accepts AE_FMT_AC3 it can probably also accept AE_FMT_DTS as that's the same requirements. And maybe also AE_FMT_EAC3 (same as AC3 but you need a 192kHz/16bit/2ch stream) ?

Member

wsoltys commented Feb 9, 2014

@jmarshallnz I'm not sure. I need to dig into the net to see what ds is capable. Probably similar to wasapi but needs some different technique to test the formats. The ds sink never got much attention but I think thats something for G+1.

Member

wsoltys commented Feb 10, 2014

Just read a little about DS. We use DS in shared mode which means we have to use the format of the engine. It needs to be a PCM format but not necessarily the same of the engine. However the channel count and sample rate needs to be set to the values of the engine.
Unsure which other formats we might use as supported.

Member

FernetMenta commented Feb 10, 2014

this pr is only about passthrough formats. how does DS support passthrough is shared mode?

Member

wsoltys commented Feb 10, 2014

Good question and I can't answer that yet. The code is different from what I read :)
For passthrough we use WAVE_FORMAT_DOLBY_AC3_SPDIF with fixed channels:
https://github.com/xbmc/xbmc/blob/master/xbmc/cores/AudioEngine/Sinks/AESinkDirectSound.cpp#L213

Member

wsoltys commented Feb 10, 2014

Interesting. The first link when searching for that topic is our wiki: http://wiki.xbmc.org/index.php?title=Windows_Settings_for_AudioEngine
-> "Even more important for this thread is that you cannot pass through encoded formats, as DirectSound will not decode them and it would otherwise bit-mangle them, and there is a loss of sonic quality involved in the mixing and resampling."

Some more is written here: http://msdn.microsoft.com/en-us/library/windows/hardware/ff538350%28v=vs.85%29.aspx (but nothing about shared mode)

But thats just for information and probably not needed for the PR, indeed.

Contributor

t-nelson commented Feb 10, 2014

@FernetMenta are we good here?

jenkins build this please

Contributor

t-nelson commented Feb 11, 2014

I think Jenkins is hitting the sauce again. Log says PI succeeded.

@FernetMenta's button.

@FernetMenta FernetMenta added a commit that referenced this pull request Feb 11, 2014

@FernetMenta FernetMenta Merge pull request #4177 from FernetMenta/aeformat
AE: let sink decide what passthrough formats it wants to support
238b2fa

@FernetMenta FernetMenta merged commit 238b2fa into xbmc:master Feb 11, 2014

1 check failed

default Merged build #182 failed in 1 hr 26 min
Details

FernetMenta deleted the FernetMenta:aeformat branch Feb 11, 2014

That looks quite odd with the eac3 passthrough device

Owner

FernetMenta replied Feb 13, 2014

why? what's wrong with it?

It's okay - I searched the but here yesterday and was confused: http://forum.xbmc.org/showthread.php?tid=170338&pid=1626225#pid1626225

Don't know if you have seen it already, but I think some "string comparison" down there goes wrong.

On Windows WASAPI and DirectSound have the same name for specific Sinks. We need to loop over the first loop and compare dri and then when this matches go into the second loop to compare dev

fritsch/xbmc@16897aa <- okay to PR?

Member

FernetMenta commented on 3cd8aad Mar 24, 2014

@wsoltys seems we need the same for wasapi. ELD is not reliable and users complain about passthrough options disappeared.

Member

fritsch replied Mar 24, 2014

Be careful. Most of the time the WASAPI enumeration works as expected. If no DTS-HD is shown it also does not work. Have seen this mostly on AMD GPUs which refuse to handshake to DTS-HD / TrueHD formats.

Perhaps a whitelist approach would be good. But in every case, don't add DTS-HD if not at least DTS was found.

Member

FernetMenta replied Mar 24, 2014

If no DTS-HD is shown it also does not work

according to user reports on the forum you are wrong here.

Member

fritsch replied Mar 24, 2014

wrong and right is from a mathematical point of view a bit problematic.

I at least found one user where it was exactly the other way round. I consider that ELD parsing a feature, cause it can save us from a lot of problems, especially on non ELD parsing capable hardware (ION-1). Perhaps adding a LOGNOTICE warning to the log if EDID has failed to make clear, that we added them manually.

Edit: Concerning the cite from above I meant: "If dts-hd is not shown it also might not work"

Member

wsoltys replied Mar 24, 2014

Are we sure thats a wrong eld or if the users did switch on their equipment in the wrong order? I'm unsure if we add another can of worms if we let the user decide even when windows said its not supported.
Implementing it would be easy as we test any suitable format and if not supported we could log it an add it anyway.

Member

FernetMenta replied Mar 24, 2014

Are we sure thats a wrong eld or if the users did switch on their equipment in the wrong order?

we are quite sure because a couple of users reported regression to an an alpha before we changed this behavior. Windows is not smarter than Linux where we give users the chance to overrule ELD.
I discussed with @fritsch. Logging a message if we add the format despite ELD has not reported it should do for the short term.

Member

wsoltys replied Mar 24, 2014

something like this: wsoltys/xbmc@2ba4553 ?

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