Emulator Issues #13182: HLE audio corruption - Emulator - Dolphin bug tracker

Actions

Copy link

Emulator Issues #13182

closed

HLE audio corruption

Added by savagejacek20 over 1 year ago. Updated over 1 year ago.

Status:

Fixed

Priority:

Normal

Assignee:

% Done:

Operating system:

Windows

Issue type:

Bug

Milestone:

Regression:

Relates to usability:

Relates to performance:

Easy:

Relates to maintainability:

Regression start:

Fixed in:

5.0-18729

Description

Game Name?

Mario Kart Double Dash

Game ID? (right click the game in the game list, Properties, Info tab)

GM4E01

MD5 Hash? (right click the game in the game list, Properties, Verify tab, Verify Integrity button)

97f9ebc39ab4244e419848793988561a

What's the problem? Describe what went wrong.

Using HLE audio emulation causes audio to be corrupted.

What steps will reproduce the problem?

Boot game and listen. Have HLE audio enabled. LLE audio doesn't have this bug. Only this game seems affected. Tested a few others and they sound fine.

Is the issue present in the latest development version? For future reference, please also write down the version number of the latest development version.

5.0-18725

Is the issue present in the latest stable version?

Idk

If the issue isn't present in the latest stable version, which is the first broken version? (You can find the first broken version by bisecting. Windows users can use the tool https://forums.dolphin-emu.org/Thread-green-notice-development-thread-unofficial-dolphin-bisection-tool-for-finding-broken-builds and anyone who is building Dolphin on their own can use git bisect.)

N/A

If your issue is a graphical issue, please attach screenshots and record a three frame fifolog of the issue if possible. Screenshots showing what it is supposed to look like from either console or older builds of Dolphin will help too. For more information on how to use the fifoplayer, please check here: https://wiki.dolphin-emu.org/index.php?title=FifoPlayer

N/A

What are your PC specifications? (CPU, GPU, Operating System, more)

R9 5950X, RTX 3080, 64 GB RAM

Is there anything else that can help developers narrow down the issue? (e.g. logs, screenshots,
configuration files, savefiles, savestates)

N/A

Actions

Copy link

Updated by JosJuice over 1 year ago

Regression changed from No to Yes
Regression start set to 5.0-18720

Actions

Copy link

Updated by Anonymous over 1 year ago

wrapping ZeldaAudioRenderer::StoreVPB with pragma optimize("",off) / on fixes the issue. Need to look a bit closer to see if it's dolphin's bug or actual msvc regression in the compiler shipped with vs 17.5 (cl 19.35.32215)

Actions

Copy link

Updated by pokechu22 over 1 year ago

The build log for 5.0-18718 says it used MSBuild version 17.4.1+9a89d02ff and the build log for 5.0-18720 used MSBuild version 17.5.0+6f08c67f3 (there's actually a second 5.0-18720 log for some reason, not sure what happened there). So it's probably an MSVC regression (though I haven't myself confirmed that dolphin actually regressed in 5.0-18720).

Actions

Copy link

Updated by Anonymous over 1 year ago

The optimized version looks like the following:

void __fastcall DSP::HLE::ZeldaAudioRenderer::StoreVPB(
        DSP::HLE::ZeldaAudioRenderer *this,
        unsigned __int16 voice_id,
        unsigned __int16 *vpb)
{
  __int64 voice_id_; // rdi
  unsigned __int16 *Pointer; // rax MAPDST
  unsigned int m_flags; // ecx
  __int64 vpb_size; // rbx
  __int64 base_idx; // rdi
  unsigned __int64 loop_count; // rdx
  unsigned __int64 v12; // r8
  char *v13; // rax
  __m128i v14; // xmm0
  unsigned __int16 *v15; // rax
  __int16 v16; // cx

  voice_id_ = voice_id;
  Pointer = (unsigned __int16 *)DSP::HLE::HLEMemory_Get_Pointer(this->m_vpb_base_addr);
  m_flags = this->m_flags;
  vpb_size = ~(4 * (unsigned __int8)this->m_flags) & 0x40 | 0x80i64;
  base_idx = vpb_size * voice_id_;
  if ( (m_flags & 0x10) != 0 )
    DSP::HLE::ZeldaAudioRenderer::VPB::Compress((DSP::HLE::ZeldaAudioRenderer::VPB *)vpb);
  loop_count = vpb_size - 0x40;                 // loop_count is 40 or 80
  v12 = 0i64;
  if ( loop_count < 4
    || _isa_available < 2
    || &Pointer[base_idx] <= &vpb[loop_count - 1] && &Pointer[loop_count - 1 + base_idx] >= vpb )
  {
    v15 = vpb;
    do
    {
      v16 = *v15++;
      *(unsigned __int16 *)((char *)Pointer + 2 * base_idx - (_QWORD)vpb + (_QWORD)v15 - 2) = __ROR2__(v16, 8);
      --loop_count;
    }
    while ( loop_count );
  }
  else if ( loop_count < 0x20 )
  {
    v13 = (char *)Pointer + 2 * base_idx - (_QWORD)vpb;
    do
    {
      v14 = _mm_loadl_epi64((const __m128i *)vpb);
      vpb += 4;
      v12 += 4i64;
      *(_QWORD *)&v13[(_QWORD)vpb - 8] = _mm_shuffle_epi8(v14, (__m128i)_xmm).m128i_u64[0];
    }
    while ( v12 < loop_count );
  }
}

The problem appears to be that loop_count < 0x20 is wrong, or there should have been another branch for larger loop_count. I suspect v12 should have been incremented by 1 instead of 4 in this case.

Forcing it to take the non-sse path (setting __isa_available) produces the correct output.

Actions

Copy link