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

GCSW drumkit bugs #263

Merged
merged 5 commits into from
May 30, 2020
Merged

GCSW drumkit bugs #263

merged 5 commits into from
May 30, 2020

Conversation

paulfd
Copy link
Member

@paulfd paulfd commented May 29, 2020

  • Corrected a bug with sfz v1 velcurve. I made it use the Curve facilities now.
  • When you encounter something like key=64Garbage, it seems that ARIA actually reads key=64 and ignores the Garbage.

@paulfd paulfd marked this pull request as draft May 30, 2020 07:38
@paulfd paulfd marked this pull request as ready for review May 30, 2020 07:54
@jpcima
Copy link
Collaborator

jpcima commented May 30, 2020

When you encounter something like key=64Garbage, it seems that ARIA actually reads key=64 and ignores the Garbage.

Confirmed yes, I was able to make similar observations and it was a reason for disabling the following unit test cases.

sfizz/tests/FilesT.cpp

Lines 349 to 356 in 6043627

#if 0
// Note: test checked to be wrong under Sforzando 1.961
// It is the shorter matching $-variable which matches among both.
// The rest of the variable name creates some trailing junk text
// which Sforzando accepts without warning. (eg. `key=52Edge`)
REQUIRE( synth.getRegionView(0)->keyRange.getStart() == 52 );
REQUIRE( synth.getRegionView(0)->keyRange.getEnd() == 52 );
#endif

It's quite possible that ARIA uses stdlib.h conversions internally, but I suspect there is more than that.
I recall this from experimentation with CW, when setting an invalid value for a numeric opcode, it's still going to override the value with something (a default?). That's contrary to sfizz which would just ignore the value.
In relation to this, and #235, I'll like to verify, for instance, what happens if value is not only "-1" but also funny things like "-0.1", "-3.14", "foo".

src/sfizz/Curve.h Outdated Show resolved Hide resolved
@paulfd
Copy link
Member Author

paulfd commented May 30, 2020

In this particular case Sforzando read it as 57 doesn't it? At least it seemed so in my test. The issue is that in such cases sfizz does ignore it. The proper test should thus be

    //       It is the shorter matching $-variable which matches among both.
    //       The rest of the variable name creates some trailing junk text
    //       which Sforzando accepts without warning. (eg. `key=57Edge`)
    REQUIRE( synth.getRegionView(0)->keyRange.getStart() == 57 );
    REQUIRE( synth.getRegionView(0)->keyRange.getEnd() == 57 );

This was a crash cymbal that was suppose to be on note 52. The key opcode being ignored by sfizz, by default, the sound will play on all the keyboard and be pitch shifted accordingly. Coupled with the above velocity bug that output 0.0f gain in some cases, you had a veeeeeeeeeeeeery long silent sample.

@paulfd paulfd merged commit 014f9ff into sfztools:develop May 30, 2020
@paulfd paulfd deleted the gcsw branch May 30, 2020 13:28
@jpcima
Copy link
Collaborator

jpcima commented May 30, 2020

On the test yes, I recall it's something like that.
The reason that I disabled it, instead of correcting, it's because this changed what the test what meant to illustrate and made it confusing. (the test case was erroneous to start)

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