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

Some pitfalls in the sources to be aware of #1

Open
tobybear opened this issue Jan 15, 2025 · 0 comments
Open

Some pitfalls in the sources to be aware of #1

tobybear opened this issue Jan 15, 2025 · 0 comments

Comments

@tobybear
Copy link

tobybear commented Jan 15, 2025

Just a hint that the latest VOPM sources published in 2006 (e.g. the ones you are using) still have some serious bugs in the code that prevent it from running smoothly or even starting up.

Some examples:

The variable "Xr_step" needs to be initialized to 0 in Op:Init() in op.cpp

In opm.cpp, the variables in the for loops in the constructor an destructor are not initialized and the second for loop increments i instead of j, creating endless loops:


Opm::Opm(int samplerate, int opmrate, GlobalStuff *glob,
         unsigned int (*irnd)(void))
    : samplerate(samplerate), opmrate(opmrate), glob(glob), irnd(irnd)
//  , op{samplerate, opmrate, glob, irnd}
//  , lfo{samplerate, opmrate}
{
  for (std::size_t i; i < 8; ++i) { // i needs to be initialized to 0 !
    for (std::size_t j; j < 4; ++i) { // needs to be ++j instead of ++i !, also j = 0
      op[i][j] = new Op(samplerate, opmrate, glob, irnd);
    }
  }
  for (std::size_t i; i < N_CH; ++i) { // i needs to be initialized to 0 !
    lfo[i] = new Lfo(samplerate, opmrate);
  }

Opm::~Opm() {
  for (std::size_t i; i < 8; ++i) { // i needs to be initialized to 0 !
    for (std::size_t j; j < 4; ++i) { // needs to be ++j instead of ++i !, also j = 0
      delete op[i][j];
    }
  }
  for (std::size_t i; i < N_CH; ++i) { // i needs to be initialized to 0 !
    delete lfo[i];
  }
  Free();
}

The (original) code for bank loading in opm_drv.cpp also has some bugs, e.g. can't deal with Windows line feed formatted vopm files.
So replace this:

    while (*pline != 0 &&
           *pline != 0x0a) { // Repeat the analysis to the end of the line.

with this:

    while (*pline != 0 &&
           *pline != 0x0a && *pline != 0x0d) { // Repeat the analysis to the end of the line.

There is also several bugs with with parsing the instrument name, so replace this:

        strcpy(name, "no Name");
        sscanf(pline, "%[^\n]", &name);
        name[15] = '\0';
        strncpy(pCh->Name, name, 15);
        errMsk |= 0x01;
        while (*pline != 0 && *pline != 0x0a) {
          pline++;

With this:

        strcpy(name, "no Name");
        sscanf(pline, "%[^\n]s", &name); // added "s" qualifier
        name[15] = '\0';
        strncpy(pCh->Name, name, 16); // 16 instead of 15
        errMsk |= 0x01;
        while (*pline != 0 && *pline != 0x0a && *pline != 0x0d) { // added Windows LF
          pline++;

Most of these changes should already be in the extended sources from the VOPMex project (http://picopicose.com/software.html) which however introduced other problems, so I just highlighted some significant changes directly.

Apart from that, after some cleanups, integration as a standalone Linux app should be straightforward (I just did it myself, triggering notes using keys and outputting audio). Let me know if you need more help with it.

Cheers,
Toby

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

No branches or pull requests

1 participant