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

BRR decoding improvements and some cleanup #18

Merged
merged 5 commits into from
Feb 13, 2022

Conversation

PhoenixBound
Copy link
Collaborator

This PR combines some general refactoring of brr.c with a new feature that fixes the way instruments 17 and 19 sound. It's not really cleaned up yet but I wanted to create a PR before the week ended. Sorry about that.

  • The first commit is mostly a reorganization of brr.c, though it adds some additional error checking in some places to prevent undefined behavior. It also changes the calculation a bit to be closer to "bit-perfect"; I figured this would result in cycles being shorter, since there are fewer possible values for a sample to have (LSB is always clear).
  • The second commit adds the main feature: at the end of a sample, EBMusEd now checks to see if the sample would sound different when the first block is re-decoded, and if it does, it tries to decode the loop again and again until it finds a cycle. Many looping samples are apparently affected by this, to varying degrees (some samples take around 10 loops before they settle into a cycle), but the noise samples were hit hardest.
  • The third commit changes a strategy introduced in the first commit, so there's slightly less confusing pointer arithmetic. It also should match what emulators (and probably real hardware?) do a little better.

Included in the first commit is code to handle failure for malloc, and the second commit includes realloc as well. There's a to-do comment left in the code about what to do if realloc fails. Right now, it just treats it the same way as the case where a cycle couldn't be found: it creates a loop segment with the last decoded loop portion of the sample. I'm interested in different ideas for what to do there, including whether doing nothing (like the rest of EBMusEd's code) is preferred.

- Split calculating sample length and decoding a single block of a BRR sample
  into functions.
- Prevent obvious undefined behavior related to malloc and BRR filters at the
  beginning of a sample.
- Add skeleton of the control flow necessary to reallocate the sample buffer.
  For now it just logs what samples generate a different sample after loop end
  compared to loop start, which would be the conditions for reallocation.

Current operating theory for why samples 17 and 19 don't work properly is that
they sound vastly different after a loop. SNES emulators certainly don't suffer
from that problem.
We now decode the sample multiple times until it settles into a loop.
Pokey's House sounds correct now! And the Starman Jr spawn/Carpainter lightning
noise sounds good too. 17 and 19 are fixed basically.
According to most SNES emulators, the header of a sample's first block is
ignored on key on. So a sample will never read data from a past sample or
anything like that.

This isn't 100% accurate, but more accuracy would require bigger changes
(supporting more than one loop point, for example) that wouldn't benefit anyone
right now.
@ChrisBlueStone ChrisBlueStone merged commit b083695 into PKHackers:master Feb 13, 2022
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.

None yet

2 participants