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

Fix CRevModel process function loop stride #20

Merged
merged 1 commit into from
Mar 14, 2024

Conversation

xddxdd
Copy link

@xddxdd xddxdd commented Jan 9, 2024

According to the decompilation result of CRevModel_R::ProcessReplace, the for loop should process two elements in the buffer array each time, rather than iterating over each element.

This also explains why Reverberation::Process sets bufR = bufL + 1.

This MIGHT fix the reverberation effect, although I haven't tried the change on my phone (haven't set up a Android build environment yet).

Below is my decompilation result with Ghidra for reference:

/* CRevModel_R::ProcessReplace(float*, float*, int) */
void CRevModel_R::ProcessReplace(int *param_1,int param_2,int param_3,int param_4)
{
  // ... some variable definitions ...
  
  if ((*param_1 != 0) && (iVar5 = param_4 + -1, 0 < param_4)) {
    iVar3 = param_3 + 8;
    iVar4 = param_2 + 8;
    do {
      // Load value from param_2 (bufL) and param_3 (bufR)
      fVar12 = *(float *)(iVar4 + -8);
      fVar11 = *(float *)(iVar3 + -8);
      // ... some computations and write back result ...
      // Index variable increments by 8 bytes (2 floats) each time
      iVar3 = iVar3 + 8;
      iVar4 = iVar4 + 8;
    } while (iVar5 != -1);
  }
  return;
}

@iscle
Copy link
Collaborator

iscle commented Jan 9, 2024

Hi :)

You are right! Reverb was broken and I had no idea why, this explains it.

Thanks for the contribution, I will check the changes on my phone and if it works I'll accept the PR.

Regards,
Iscle

@iscle iscle changed the base branch from rewrite-continued to foss March 14, 2024 12:01
@iscle iscle merged commit 8cc3af5 into AndroidAudioMods:foss Mar 14, 2024
1 of 5 checks passed
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.

2 participants