Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

[AE] AERemap: Fix mixing front and rear channels in some setups #2576

Merged
1 commit merged into from

4 participants

@manio

Fix the case when the input announces BR and BL instead of SR and SL,
e.g. when the following channels are in:
input: FL FR FC LFE SL SR
output: FL FR BL BR FC LFE
This leads to wrong mix front and rear channels,
(e.g. when playing AC3 test files).
The commit is handling those cases.

Special thanks to @fritsch for helping to track the problem down
and initial test patches.

@davilla
Collaborator

Please include a URL to any test files so all platforms can check and versify this PR.

@manio

Sample AC3 test file:
http://otakuland.de/AC3TEST.VOB

@anssih
Collaborator

Issue is real and the idea is correct.
However, I think it is wrong in case of 7.1 => 5.1(back).
EDIT: just add a "&& !m_mixInfo[AE_CH_BL].in_src" in there I think.

Also, I believe reading the code that there is a similar issue to the one this patch addresses in the reverse direction, i.e. 5.1(back) => 5.1(side), which is not addressed by the patch.

See also my related comment here: #1490 (comment)

@davilla: standard AC-3 5.1 test file in teamftp, my directory.

@fritsch
Collaborator

The patch looks good to me - but this terrain is not an easy one. In order to not run in a discussion like here: #1490 I would also match if num of input channels match num of output channels.

I personally see no point in mixing 6 channels, when there are matching number of capable speakers, but the audiophiles would perhaps not like my pragmatic thinking.

@anssih
Collaborator

On a second thought, not 100% sure what we want to do with 7.1 => 5.1(back).
Depending on whether the actual speaker arrangement is 5.1(back) or 5.1(side), we would optimally either
a) in case of 5.1(back) speakers, the 7.1 side channels should be mixed to rear and front (like the second patch, though we should mix more of it to rear than to front), or
b) in case of 5.1(side) speakers, the 7.1 .... er... both rear and side to rear, I guess, (like the first patch) did.

Duh, I guess I should see what my amp does :)

@anssih
Collaborator

My amplifier seems to do (b), and since it seems sensible, I think I'm going to backpedal and say that it was better before my suggested modification. Especially since we don't know the actual 5.1 speaker arrangement.

EDIT: However, it might make sense to not change anything at 7.1 right now to focus on the 5.1 issue. 7.1 can be meddled with later if wanted.

@manio

Ok guys, updated PR to cover this 5.1 case.

@anssih
Collaborator

Seems ok to me in principle, i.e. at least better than what we do now.

@manio manio [AE] AERemap: Fix mixing front and rear channels in some 5.1 setups
Fix the case when the input announces BR and BL instead of SR and SL,
e.g. when the following channels are in:
input:  FL FR FC LFE SL SR
output: FL FR BL BR  FC LFE
This leads to wrong mix front and rear channels,
(e.g. when playing AC3 test files).
The commit is handling those cases.

Special thanks to @fritsch for helping to track the problem down
and initial test patches.
ae788cf
@fritsch
Collaborator

Yes. Same for me. It fixes real user problems, so +1 from me.

@ghost ghost merged commit 8172cd2 into xbmc:master
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Apr 9, 2013
  1. @manio

    [AE] AERemap: Fix mixing front and rear channels in some 5.1 setups

    manio authored
    Fix the case when the input announces BR and BL instead of SR and SL,
    e.g. when the following channels are in:
    input:  FL FR FC LFE SL SR
    output: FL FR BL BR  FC LFE
    This leads to wrong mix front and rear channels,
    (e.g. when playing AC3 test files).
    The commit is handling those cases.
    
    Special thanks to @fritsch for helping to track the problem down
    and initial test patches.
This page is out of date. Refresh to see the latest.
Showing with 15 additions and 2 deletions.
  1. +15 −2 xbmc/cores/AudioEngine/Utils/AERemap.cpp
View
17 xbmc/cores/AudioEngine/Utils/AERemap.cpp
@@ -119,8 +119,21 @@ bool CAERemap::Initialize(CAEChannelInfo input, CAEChannelInfo output, bool fina
RM(AE_CH_TFC , AE_CH_TFL, AE_CH_TFR);
RM(AE_CH_TFR , AE_CH_FR);
RM(AE_CH_TFL , AE_CH_FL);
- RM(AE_CH_SR , AE_CH_BR, AE_CH_FR);
- RM(AE_CH_SL , AE_CH_BL, AE_CH_FL);
+
+ // Some 5.1 Setups announce a BR and BL instead of SR and SL - put out SR and SL on those instead
+ if (!m_mixInfo[AE_CH_BL].in_src && !m_mixInfo[AE_CH_BR].in_src && // make sure not to not mix 7.1 channels
+ (m_mixInfo[AE_CH_SL].in_src && !m_mixInfo[AE_CH_SL].in_dst && m_mixInfo[AE_CH_BL].in_dst) &&
+ (m_mixInfo[AE_CH_SR].in_src && !m_mixInfo[AE_CH_SR].in_dst && m_mixInfo[AE_CH_BR].in_dst))
+ {
+ RM(AE_CH_SR , AE_CH_BR);
+ RM(AE_CH_SL , AE_CH_BL);
+ }
+ else
+ {
+ RM(AE_CH_SR , AE_CH_BR, AE_CH_FR);
+ RM(AE_CH_SL , AE_CH_BL, AE_CH_FL);
+ }
+
RM(AE_CH_BC , AE_CH_BL, AE_CH_BR);
RM(AE_CH_FROC, AE_CH_FR, AE_CH_FC);
RM(AE_CH_FLOC, AE_CH_FL, AE_CH_FC);
Something went wrong with that request. Please try again.