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

Fixes values spiking due to bad sample memory accesses #5

Merged
merged 1 commit into from
Jan 9, 2015

Conversation

DanielOaks
Copy link
Contributor

This fixes the clicks and staticy noises in the modules in #1. That is, the ones that make these messages:

xm_sample(): final sample value is 136597905678638606450688.000000, this is a bug
xm_sample(): final sample value is 296616456960244306673664.000000, this is a bug
xm_sample(): final sample value is 144714319948683478564864.000000, this is a bug

There are two things that cause this problem in the xm_next_of_sample(xm_channel_context_t* ch) function – either the sample length is 0, and when it grabs u = ch->sample->data[a]; it reads into memory that does not belong to it, so we have that if at the start to check the sample length is more than zero.

What can also happen is that the 'loop end point' extends past the actual end of the sample. Just by one tick, so ch->sample_position was getting set to 132 at the end of a sample that was only 132 long, resulting in the same sort of read into memory that does not belong to it.

Anyways, this fixes both of those issues and the clipping. I'm fairly sure Milkytracker handles this in the same way, clamping the loop end to the sample length.

edit: I'm not sure whether this sort of fix should also be applied to the other loop types. I had to quickly type this up a few minutes before running to work, but otherwise I'll look into it when I get home.

@Artefact2
Copy link
Owner

Thanks. I confirm this fixes the clipping issues in zalza-tequila_groove.xm, but not the others.

You should also add your name and the current year in the header for proper copyright assignment.

@DanielOaks
Copy link
Contributor Author

Ah, I was doing the checking a bit strangely. I've got that fixed now, will push once I confirm the copyright message.

Just to confirm, something like this in the header just under your Author: line, or did you have something else in mind? (And just in play.c, or across all the files?)

/* Patches: Daniel Oaks <[email protected]> */

@Artefact2
Copy link
Owner

Yes, add /* Contributor: name <email> */ in any file where you make non-trivial changes.

@DanielOaks
Copy link
Contributor Author

Alright, that should be right to go.

Doesn't fix those static sounds in jt_strng.xm near the start, but that's not caused by this bug so I'll need to take a look and see why that's happening, this does fix the clipping in binary_world.xm and zalza-tequila_groove.xm though.

@Artefact2 Artefact2 merged commit f24bcab into Artefact2:master Jan 9, 2015
@DanielOaks DanielOaks deleted the sample-memory-fix branch January 9, 2015 23:47
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