[FernetMenta]

and please update to this pull request: https://github.com/xbmc/xbmc/pull/3744 the commit I mentioned earlier is outdated.

[Shine]

please post full debug log (xbmclogs.com or pastebin)

and please update to this pull request: https://github.com/xbmc/xbmc/pull/3744 the commit I mentioned earlier is outdated.

Will do, might take a few days, though (work week begins...)

I suspect the downmixer/upmixer still tries to find an output format that contains BC when "Best Match" is on. But I never really tried to understand the AE code, so I might be completely off here...

One suggestion in advance, though. Wouldn't changing your commit to something like this:

Code:

// mix back center if not available in destination layout
if (srcHasBC && !dstHasBC)
{
  if (!newInfo.HasChannel(AE_CH_BL) && !newInfo.HasChannel(AE_CH_BR))
    newInfo += AE_CH_BL;
    newInfo += AE_CH_BR;
}

Allow upmixing 6.1 to 7.1 as well, in one go?

[Shine]

Re-tested with Pull Request 3744 applied to my last git clone (e3a145 - 2013-11-30).

• 6.1 channels are now upmixed to 7.1 (at least my AVR shows 8 channels now, not 6 like before, but I didn't do any listening tests).
  
• 4.0 with 3 front ch plus BC still results in total silence when using Best Match. Full debug log here.
  Edit: I uncommented the format match loop debug logging in AESinkWASAPI.cpp, that gives some more insight. No full log, just a log snippet for now.
  

[Shine]

So it turns out that WASAPI simply refuses to initialize the 3.0/5.0/7.0 channel layouts on my system, resulting in silence whenever XBMC decides to choose one of these as destination layout. I can reproduce by forcing anything to up- or downmix to 3.0, 5.0 or 7.0 (e.g. by changing XBMC audio settings to the respective layout). The "find something that works" loop in CAESinkWASAPI::InitializeExclusive will fail because it only loops through sample rates and formats, but not through channel layouts.

I "fixed" it by force-adding a LFE channel to anything that has a FC channel and is not mono - and might therefore result in one of the nonworking layouts.

I guess the proper fix would be that XBMC does not try to use channel layouts that aren't supported by the output device, just like it does with sample formats and -rates.

Here's my quick&dirty fix for anybody who's interested (diff based on Git rev. e3a145 with pull request 3744 applied):

Code:

diff --git "a/xbmc/cores/AudioEngine/Utils/AEChannelInfo.cpp" "b/xbmc/cores/AudioEngine/Utils/AEChannelInfo.cpp"
index c0963c9..7df0a00 100644
--- "a/xbmc/cores/AudioEngine/Utils/AEChannelInfo.cpp"
+++ "b/xbmc/cores/AudioEngine/Utils/AEChannelInfo.cpp"
@@ -56,6 +56,7 @@ void CAEChannelInfo::ResolveChannels(const CAEChannelInfo& rhs, bool mix)
   bool srcHasRL = false;
   bool srcHasRR = false;
   bool srcHasBC = false;
+  bool srcHasFC = false;

   bool dstHasSL = false;
   bool dstHasSR = false;
@@ -85,6 +86,7 @@ void CAEChannelInfo::ResolveChannels(const CAEChannelInfo& rhs, bool mix)
       case AE_CH_BL: srcHasRL = true; break;
       case AE_CH_BR: srcHasRR = true; break;
       case AE_CH_BC: srcHasBC = true; break;
+      case AE_CH_FC: srcHasFC = true; break;
       default:
         break;
     }
@@ -136,6 +138,10 @@ void CAEChannelInfo::ResolveChannels(const CAEChannelInfo& rhs, bool mix)
       newInfo += AE_CH_SR;
   }

+  // Add LFE to 3.0/5.0/7.0 formats
+  if (srcHasFC && !newInfo.HasChannel(AE_CH_LFE))
+      newInfo += AE_CH_LFE;
+
   *this = newInfo;
}

[FernetMenta]

Could you test this: https://github.com/FernetMenta/xbmc/comm...1db493f2bd
It tries standard layouts if requested one is not supported. As a last resort it opens stereo which is much better than silence.

[Shine]

I'm currently at work, will test tonight or later this weekend.

I do see a possible issue with your "same or higher channel count" approach, though. There are channel layouts that are not comparable at all, despite having the same channel count, e.g. 4.0 Quadrophonic and 3.1 Front+LFE. Or channels that shouldn't count because XBMC will never use them anyway (e.g. BC). Wouldn't it be more preferrable to loop through the layouts that contain at least all requested speakers first, and then perhaps backwards through layouts that are missing 1, 2, ... speakers using a priority list (e.g. missing LFE is less critical than missing back or center) before reverting to stereo?

If I figured this out correctly, when using your commit, I might end up with 4.1 (Quad+LFE) as a replacement for my unsupported 5.0, although 5.1 should be the preferred layout in this case.

Edit: Looking at the commit, I guess you already did this...

Note to self: Don't hang around in the XBMC forums when at work ;-)

[FernetMenta]

If I figured this out correctly, when using your commit, I might end up with 4.1 (Quad+LFE) as a replacement for my unsupported 5.0, although 5.1 should be the preferred layout in this case.

The first standard layout with at least 5 channels is 5.1 and will be taken in this case.

[fritsch]

Perfect Speaker Mapping of incomplete standard layouts btw. is a problem of distance measure in order to solve an optimization problem.

If you need something better, PR or Algorithm welcome :-)

[Shine]

Could you test this: https://github.com/FernetMenta/xbmc/comm...1db493f2bd
It tries standard layouts if requested one is not supported. As a last resort it opens stereo which is much better than silence.

Tested and still silence. Full log here, though it doesn't contain anything more than the last one.

[fritsch]

Here is something totally untested - but it's the brute force method. I hope you get it compiled:

Code:

commit 290c5e597a061514cc771d2388efb87731dfe549
Author: fritsch <[email protected]>
Date:   Fri Dec 6 22:36:20 2013 +0100

    AESinkWasapi: Workaround 5.0 and 7.0 speaker layouts by adding LFE chan

diff --git a/xbmc/cores/AudioEngine/Sinks/AESinkWASAPI.cpp b/xbmc/cores/AudioEngine/Sinks/AESinkWASAPI.cpp
index 847359d..a1de198 100644
--- a/xbmc/cores/AudioEngine/Sinks/AESinkWASAPI.cpp
+++ b/xbmc/cores/AudioEngine/Sinks/AESinkWASAPI.cpp
@@ -156,6 +156,16 @@ DWORD ChLayoutToChMask(const enum AEChannel * layout, unsigned int * numberOfCha
   unsigned int i;
   for (i = 0; layout[i] != AE_CH_NULL; i++)
     mask |= WASAPIChannelOrder[layout[i]];
+
+  //workaround 7.0 and 5.0 layout
+  if (mask == (KSAUDIO_SPEAKER_5POINT1 & ~SPEAKER_LOW_FREQUENCY)
+   || mask == (KSAUDIO_SPEAKER_5POINT1_SURROUND & ~SPEAKER_LOW_FREQUENCY)
+   || mask == (KSAUDIO_SPEAKER_7POINT1 & ~SPEAKER_LOW_FREQUENCY)
+   || mask == (KSAUDIO_SPEAKER_7POINT1_SURROUND & ~SPEAKER_LOW_FREQUENCY))
+  {
+   ++i;
+   mask |= SPEAKER_LOW_FREQUENCY;
+  }
  
   if (numberOfChannels)
     *numberOfChannels = i;

[Shine]

Thank you, but isn't this pretty much exactly the same that I did in the diff that I posted last night, just at a different place in the code?

It's fine for people like me who actually have a problem initializing the (3.0/)5.0/7.0 layout, but won't help others whose system refuses to use different channel layouts via WASAPI. An approach like FernetMenta's is better imo.

[fritsch]

As said in the code, it's just a workaround for exactly 7.0 and 5.0 layouts, it does nothing more. It nicely adds to the patches fernetmenta did. If you want to do it in a perfect way you need to define a "difference measure" between available and wanted channel configurations and find the optimum, e.g. minimal distance.

[Shine]

Mine's a workaround for 3.0/5.0/7.0. Those are the only ones of XBMC's standard channel layouts affected by my diff (layouts[3], layouts[7], layouts[9] if I counted correctly):

Code:

static enum AEChannel layouts[AE_CH_LAYOUT_MAX][9] = {
    {AE_CH_FC, AE_CH_NULL},
    {AE_CH_FL, AE_CH_FR, AE_CH_NULL},
    {AE_CH_FL, AE_CH_FR, AE_CH_LFE, AE_CH_NULL},
    {AE_CH_FL, AE_CH_FR, AE_CH_FC , AE_CH_NULL},
    {AE_CH_FL, AE_CH_FR, AE_CH_FC , AE_CH_LFE, AE_CH_NULL},
    {AE_CH_FL, AE_CH_FR, AE_CH_BL , AE_CH_BR , AE_CH_NULL},
    {AE_CH_FL, AE_CH_FR, AE_CH_BL , AE_CH_BR , AE_CH_LFE, AE_CH_NULL},
    {AE_CH_FL, AE_CH_FR, AE_CH_FC , AE_CH_BL , AE_CH_BR , AE_CH_NULL},
    {AE_CH_FL, AE_CH_FR, AE_CH_FC , AE_CH_BL , AE_CH_BR , AE_CH_LFE, AE_CH_NULL},
    {AE_CH_FL, AE_CH_FR, AE_CH_FC , AE_CH_BL , AE_CH_BR , AE_CH_SL , AE_CH_SR, AE_CH_NULL},
    {AE_CH_FL, AE_CH_FR, AE_CH_FC , AE_CH_BL , AE_CH_BR , AE_CH_SL , AE_CH_SR, AE_CH_LFE, AE_CH_NULL}
  };

But again, this fix is specific to my problem, which is a problem that others likely don't have or have in a different shape.

I agree that calculating the best channel mapping substitutes is quite a big task - that's why I opted for a quick and dirty hardcoded way last night, just to get it working for now and for my specific case.

[FernetMenta]

Due to a signed/unsigned comparison issue it did not loop into the for-loop. I really hate macros which prevented the compiler to throw a warning: https://github.com/FernetMenta/xbmc/comm...11d649b2bb

[Shine]

Tested, AVR detects 5.1 and I do get sound now. But... nothing from the rear speakers, only the front speakers are working.

Perhaps at this late point in the initialization, changing the channel layout kills the upmix matrix? Your previous PR combined with my hardcoded LFE addition in CAEChannelInfo::ResolveChannels does give me sound from the rear channels (with and without your additional commits applied, since the loop is never reached anyway).