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

AESinkALSA: Open 5.1 Layouts according to CEA-861-D #3721

Merged
merged 1 commit into from
Nov 28, 2013

Conversation

fritsch
Copy link
Member

@fritsch fritsch commented Nov 27, 2013

This will handle 5.1 Side and 5.1 Wide the same way. Which is okay according to the above spec.

@laurimyllari
Copy link
Contributor

Tested on top of OE adb0cf1 and now 5.1 source is output as 5.1.

@fritsch
Copy link
Member Author

fritsch commented Nov 28, 2013

v2: Don't influence bitstream passthrough. v1 also influence channel numbers of DTS-HD / TrueHD / EAC3

@laurimyllari: Thanks for testing, please redo if you find time.

@FernetMenta
Copy link
Contributor

Now if engine requests FL/FR/FC/LFE/SL/SR you return FL/FR/FC/BL/BR/LFE, right? This will work but also force the engine through the process of rematrixing/mixing. You could save those extra cycles by returning the channels requested: FL/FR/FC/SL/SR/LFE

@fritsch
Copy link
Member Author

fritsch commented Nov 28, 2013

According to the Spec I could return something like: FL/FR/FC/RL/RR/LFE to get the ball back to the Engine. Basically this PR makes clear, that according to the spec SL/SR and BR/BL are equivalent if only one pair of them is requested.

Edit: if only SL/SR are requested but BR/BL is missing and returns the minimal layout.

@FernetMenta
Copy link
Contributor

If you return BL/BR if SL/SR is requested, this causes a lot of extra cycles. Just return the pair which is requested on that position.

@fritsch
Copy link
Member Author

fritsch commented Nov 28, 2013

Wait, to make that fully correct:

If someone requests:
FL/FR/FC/LFE/SL/SR i return FL/FR/BL/BR/FC/LFE which is a 5.1 alsa channel map.

static enum AEChannel ALSAChannelMap[ALSA_MAX_CHANNELS + 1] = {
  AE_CH_FL      , AE_CH_FR      , AE_CH_BL      , AE_CH_BR      , AE_CH_FC      , AE_CH_LFE     , AE_CH_SL      , AE_CH_SR      ,
  AE_CH_UNKNOWN1, AE_CH_UNKNOWN2, AE_CH_UNKNOWN3, AE_CH_UNKNOWN4, AE_CH_UNKNOWN5, AE_CH_UNKNOWN6, AE_CH_UNKNOWN7, AE_CH_UNKNOWN8, /* for p16v devices */
  AE_CH_NULL
};

If you see in the code, we land at position 8: AE_CH_SR which would mean to open a 7.1 layout, the patch reduces the 8 to 6 and therefore opens a layout with 5.1.

This is how Alsa defines the channel layouts. If I return you something "non alsa like" I have to remap them later in the sink, which also causes a lot of extra cycles at a position where we don't want them.

@laurimyllari
Copy link
Contributor

Works nicely as far as I can tell. I had problems with DTS-HD/MA, but that seems to be a Haswell issue (audio device got stuck) that I've seen on previous versions too.

Tried 5.1 PCM, ac3/dts passthrough, TrueHD and 5.1 FLAC.

@fritsch
Copy link
Member Author

fritsch commented Nov 28, 2013

v3: Return SL/SR at the position of BL/BR which saves us the cycles and is also valid.

@FernetMenta: Thx much for helping with that one: http://imalbum.aufeminin.com/album/D20070912/338700_OP2IONTWQ13SZKS3733VBU38BQZ6LS_brett-vorm-kopf_H144817_L.jpg

Second patch is an alternative implementation which gets rid of the +5 which could harm, when the map changes. Open for discussion.

@fritsch
Copy link
Member Author

fritsch commented Nov 28, 2013

jenkins build this please

@wsnipex
Copy link
Member

wsnipex commented Nov 28, 2013

jenkins build this please
@Memphiz can you please check why fritsch can't trigger jenkins?

@Memphiz
Copy link
Member

Memphiz commented Nov 28, 2013

He successfully triggered a build - see

http://jenkins.xbmc.org/view/helpers/job/XBMC-BuildMulti-PR/642/

But while building or after that he force pushed to his repo which removes the indicators from the PR.

@fritsch
Copy link
Member Author

fritsch commented Nov 28, 2013

jenkins build this please sorry for force push and the 0xFFF changes you had to see today.

FernetMenta added a commit that referenced this pull request Nov 28, 2013
AESinkALSA: Open 5.1 Layouts according to CEA-861-D
@FernetMenta FernetMenta merged commit 037800e into xbmc:master Nov 28, 2013
@fritsch fritsch deleted the ae-use-cea-for-speaker-layout branch December 23, 2013 17:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants