AX: Implement loop_counter and support UCodes without LPF #5211

merryhime · 2017-04-06T13:07:53Z

~~Uses constexpr if and hence requires C++17. This approach is possible without constexpr if but would be much uglier.~~

~~I decided to replace the preprocessor AX_GC and AX_WII defines with a configuration template parameter. This allowed for further variants in AX functionality to be supported via constexpr if.~~

~~While one can now semi-trivially remove the need for the global variables in AXVoice.h after this change, I've decided to leave that to another PR.~~

(P.S.: delroth did all the hard work, this just implements the fix.)

Edit: Now uses copies for layout differences.

merryhime · 2017-04-06T13:11:08Z

~~Requires at least cmake 3.8-rc4 for C++17 support.~~
~~Ugly formatting due to clang-format's lack of support for constexpr if: LLVM Bug.~~

BhaaLseN · 2017-04-06T16:35:29Z

MSVC doesn't support if constexpr, not even with VS 2017 :(

Other than that, why do you need a dedicated NoLpf PB struct? Unless I missed something, the only difference is the lpf member (and hence the different padding size); which you could just ignore in the NoLpf case on GC (unless this triggers an unused member warning due to being removed by the if constexpr)

merryhime · 2017-04-06T16:55:55Z

@BhaaLseN The existence of the lpf member moves the location/offset of loop_counter.

BhaaLseN · 2017-04-06T16:58:29Z

Ah, duh. Forgot about that. Nvm then.

JMC47 · 2017-04-07T10:33:07Z

So, with MS not supporting that (yet), what's going to happen to this?

merryhime · 2017-04-07T13:10:41Z

My plan was to wait until C++17 is a thing in Dolphin and for MSVC if constexpr support.

BhaaLseN · 2017-04-07T13:18:31Z

Looking at this again, isn't this the reason why we have to care about the struct layout (and potentially create duplicates)?
I could imagine an alternate solution where the members are swapped/copied by hand, using the correct offsets for the given AX variant...not sure if this is something we want to consider though.

delroth · 2017-04-07T18:53:24Z

@BhaaLseN that's what I ended up doing for Zelda HLE, see https://github.com/dolphin-emu/dolphin/blob/master/Source/Core/Core/HW/DSPHLE/UCodes/Zelda.cpp#L885

I'm not against doing the same thing for AX.

shuffle2 · 2017-04-07T19:48:30Z

can't you just use unions for this (instead of the #ifdef stuff)?

Required for Rogue Squadron.

merryhime · 2017-04-07T20:14:12Z

Updated to use copies for the alternate layout.

Source/Core/Core/HW/DSPHLE/UCodes/AX.cpp

+{
+  switch (crc)
+  {
+  case 0x4e8a8b21:


Source/Core/Core/HW/DSPHLE/UCodes/AXStructs.h

@@ -253,8 +253,9 @@ struct AXPB
  PBSampleRateConverter src;
  PBADPCMLoopInfo adpcm_loop_info;
  PBLowPassFilter lpf;


JMC47 · 2017-04-08T13:57:24Z

LGTM. I completed the first three missions of RS2 (reported to crash with the hack in Ishiiruka) and they seem to be working fine here. Rogue Squadron 3 isn't working well in Dolphin right now, but, in the parts of the game I got to it also works.

As a bonus, it appears that Pac-Man World Rally is fixed. The audio now seems to be roughly the same as LLE. Instead of having some sounds super loud, and some super quiet, they all now play fairly loud, but can be adjusted in the options. The game appears louder than most on console as well.

Source/Core/Core/HW/DSPHLE/UCodes/AXVoice.h

@@ -81,17 +81,50 @@ union AXBuffers
 };

 // Read a PB from MRAM/ARAM
-void ReadPB(u32 addr, PB_TYPE& pb)
+void ReadPB(u32 addr, PB_TYPE& pb, bool has_lpf = true)


Source/Core/Core/HW/DSPHLE/UCodes/AX.cpp

+  {
+  case 0x4e8a8b21:
+    return false;
+  default:


delroth

Overall LGTM, thanks a lot.

Affects: * Star Wars: Rogue Leader * Star Wars: Rebel Strike

BhaaLseN

Theres also

dolphin/Source/Core/Core/HW/DSPHLE/UCodes/AX.cpp

Line 285 in 7fa1416

// TODO: find other UCode versions with different mixer_control values

referencing the magic CRC. Is that mixer control thingy related to LPF, or is that something different?

Source/Core/Core/HW/DSPHLE/UCodes/AXVoice.h

@@ -80,18 +80,63 @@ union AXBuffers
 #endif
 };

+// Determines if this version of the UCode has a PBLowPassFilter in its AXPB layout.
+bool HasLpf(u32 crc)


delroth · 2017-04-08T15:22:45Z

I'm fine with merging this now because even if other UCode versions have the same weirdness, it should only impact LPF emulation which is presently disabled due to other issues. I think it's a good compromise.

AX: Implement loop counter

8f8e768

Required for Rogue Squadron.

merryhime force-pushed the ax branch from c3f9aed to c52a5e7 Compare April 7, 2017 20:09

merryhime force-pushed the ax branch from c52a5e7 to da690fa Compare April 7, 2017 20:16

merryhime changed the title ~~[RFC] AX: Implement loop_counter and support UCodes without LPF~~ AX: Implement loop_counter and support UCodes without LPF Apr 8, 2017

Warepire reviewed Apr 8, 2017

View reviewed changes

BhaaLseN reviewed Apr 8, 2017

View reviewed changes

Source/Core/Core/HW/DSPHLE/UCodes/AXStructs.h Outdated

@@ -253,8 +253,9 @@ struct AXPB

PBSampleRateConverter src;

PBADPCMLoopInfo adpcm_loop_info;

PBLowPassFilter lpf;

This comment was marked as off-topic.

Sign in to view

This comment was marked as off-topic.

Sign in to view

delroth reviewed Apr 8, 2017

View reviewed changes

Source/Core/Core/HW/DSPHLE/UCodes/AXVoice.h Outdated

@@ -81,17 +81,50 @@ union AXBuffers

};

// Read a PB from MRAM/ARAM

void ReadPB(u32 addr, PB_TYPE& pb)

void ReadPB(u32 addr, PB_TYPE& pb, bool has_lpf = true)

This comment was marked as off-topic.

Sign in to view

delroth reviewed Apr 8, 2017

View reviewed changes

Source/Core/Core/HW/DSPHLE/UCodes/AX.cpp Outdated

{

case 0x4e8a8b21:

return false;

default:

This comment was marked as off-topic.

Sign in to view

delroth reviewed Apr 8, 2017

View reviewed changes

MerryMage added 2 commits April 8, 2017 16:04

AX: Add support for UCodes without LPF

8ad6f2c

GameIni: DSP LLE no longer required for Star Wars: Rogue Squadron

cd2d089

Affects: * Star Wars: Rogue Leader * Star Wars: Rebel Strike

merryhime force-pushed the ax branch from 8869645 to cd2d089 Compare April 8, 2017 15:04

BhaaLseN reviewed Apr 8, 2017

View reviewed changes

delroth merged commit e863604 into dolphin-emu:master Apr 8, 2017

merryhime deleted the ax branch April 8, 2017 15:23

AX: Implement loop_counter and support UCodes without LPF #5211

AX: Implement loop_counter and support UCodes without LPF #5211

merryhime commented Apr 6, 2017 •

edited

merryhime commented Apr 6, 2017 •

edited

BhaaLseN commented Apr 6, 2017

merryhime commented Apr 6, 2017

BhaaLseN commented Apr 6, 2017

JMC47 commented Apr 7, 2017 •

edited

merryhime commented Apr 7, 2017

BhaaLseN commented Apr 7, 2017

delroth commented Apr 7, 2017

shuffle2 commented Apr 7, 2017

merryhime commented Apr 7, 2017

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

JMC47 commented Apr 8, 2017

This comment was marked as off-topic.

This comment was marked as off-topic.

delroth left a comment

BhaaLseN left a comment

This comment was marked as off-topic.

This comment was marked as off-topic.

delroth commented Apr 8, 2017

AX: Implement loop_counter and support UCodes without LPF #5211

AX: Implement loop_counter and support UCodes without LPF #5211

Conversation

merryhime commented Apr 6, 2017 • edited

merryhime commented Apr 6, 2017 • edited

BhaaLseN commented Apr 6, 2017

merryhime commented Apr 6, 2017

BhaaLseN commented Apr 6, 2017

JMC47 commented Apr 7, 2017 • edited

merryhime commented Apr 7, 2017

BhaaLseN commented Apr 7, 2017

delroth commented Apr 7, 2017

shuffle2 commented Apr 7, 2017

merryhime commented Apr 7, 2017

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

JMC47 commented Apr 8, 2017

This comment was marked as off-topic.

This comment was marked as off-topic.

delroth left a comment

Choose a reason for hiding this comment

BhaaLseN left a comment

Choose a reason for hiding this comment

This comment was marked as off-topic.

This comment was marked as off-topic.

delroth commented Apr 8, 2017

merryhime commented Apr 6, 2017 •

edited

merryhime commented Apr 6, 2017 •

edited

JMC47 commented Apr 7, 2017 •

edited