SID XRun bug: pause on 1-st SID note played.

[firewall1110]

System Information

GNU/Linux Debian stable

LMMS Version(s)

LMMS 1.3.0-alpha.1.681+gbda1a9c37 (master build 2024.08.14, using clang14)

Most Recent Working Version

lmms-1.3.0-alpha.1.102+g89fc6c9-linux-x86_64.AppImage

Bug Summary

When 1-st note played by SID all project paused for short time (if using jack audio back-end : XRun messages).
After 1-st note played (even in export mode) - no more problems.

SID_bug.mp4

Expected Behaviour

No unexpected pauses.

Steps To Reproduce

Open project, containing SID.
When 1-st SID note played will be pause (happens only once !).

Logs

Click to expand

Screenshots / Minimum Reproducible Project

SID_bug_demo.mmpz.zip

Please search the issue tracker for existing bug reports before submitting your own.

• I have searched all existing issues and confirmed that this is not a duplicate.

[michaelgregorius]

The problem can also be reproduced with demos/Greippi - Krem Kaakkuja (Second Flight Remix).mmpz. Steps to reproduce:

1. Start a debug build of LMMS.
2. Open the demo song demos/Greippi - Krem Kaakkuja (Second Flight Remix).mmpz.
3. Play the song.

Towards the end of the fourth measure you should experience a short dropout.

[firewall1110]

1-st negative result:
I tried to simply make new reSID::SID() once in SidInstrument::SidInstrument :

• no pause on 1-rst note;
• Segmentation Fault on 2-nd note.

Segmentation Fault in line ~426:
const auto num = static_cast<f_cnt_t>(sid_fillbuffer(sidreg.data(), sid, delta_t, buf, frames));

P.S.
@michaelgregorius ! Thank You for bug confirmation!

[firewall1110]

I treat issue #7638 as a hint:

libsid.so!reSID::EnvelopeGenerator::clock(reSID::EnvelopeGenerator * const this, reSID::cycle_count delta_t) (/home/michael/Eigene Dateien/Development/LMMS/lmms-dev-mg/lmms/plugins/Sid/resid/resid/envelope.h:269)
libsid.so!reSID::SID::clock(reSID::SID * const this, reSID::cycle_count delta_t) (/home/michael/Eigene Dateien/Development/LMMS/lmms-dev-mg/lmms/plugins/Sid/resid/resid/sid.cc:758)
libsid.so!reSID::SID::clock_fast(reSID::SID * const this, reSID::cycle_count & delta_t, short * buf, int n, int interleave) (/home/michael/Eigene Dateien/Development/LMMS/lmms-dev-mg/lmms/plugins/Sid/resid/resid/sid.cc:867)
libsid.so!reSID::SID::clock(reSID::SID * const this, reSID::cycle_count & delta_t, short * buf, int n, int interleave) (/home/michael/Eigene Dateien/Development/LMMS/lmms-dev-mg/lmms/plugins/Sid/resid/resid/sid.cc:841)
libsid.so!lmms::sid_fillbuffer(unsigned char * sidreg, reSID::SID * sid, int tdelta, short * ptr, int samples) (/home/michael/Eigene Dateien/Development/LMMS/lmms-dev-mg/lmms/plugins/Sid/SidInstrument.cpp:273)

(Edited:) This is wrong:
So may be in my solution (make memory allocation for emulator in SID constructor) I simply expose another bug ...
(Edited:) I has not understood how SID is working - it need new instance at every new note played, and this instance is deleted after note off (but this delete is not in SidInstrument.cpp)

P.S.
I hope to fix this bug but now I am "paralyzed" with my semi professional musician work ...

[firewall1110]

New observation:

If I place in constructor SidInstrument::SidInstrument( InstrumentTrack * _instrument_track )

line auto sid = new reSID::SID();
(and after line if (sid) {fprintf(stderr, "\nnew reSID::SID(); :: Done\n");} for no compiler warning)

It seems bug not noticed (but anyway SidInstrument implementation is not RT-safe)
So real problem is then auto sid = new reSID::SID(); called 1-st time.

[firewall1110]

New observation:

(1) In SidInstrument::SidInstrument( InstrumentTrack * _instrument_track )
I can put line:
reSID::SID sid;

And this fix XRun bug

(2) In SID::SID()
I can (with (1)) remove line:
set_sampling_parameters(985248, SAMPLE_FAST, 44100);

And this fix XRun bug

So it seems, that calling SidInstrument::SidInstrument 1-st time some file is loaded from disk .

[sakertooth]

So it seems, that calling SidInstrument::SidInstrument 1-st time some file is loaded from disk .

It actually seems like its calculating tables (which are then later cached), based on the code in sid.cc.

[sakertooth]

Though, its still somewhat confusing because its presumably being recalculated for each note being played.

[PhysSong]

FYI, the actual place where a cached calculation occurs is reSID::Filter::Filter, which has a initialization guarded with class_init.

[sakertooth]

FYI, the actual place where a cached calculation occurs is reSID::Filter::Filter, which has a initialization guarded with class_init.

I was referring to the code in SID::set_sampling_parameters, which is also called on construction. In there, there's some kind of caching near the end. Seemed like a possible reason for a dropout. Could be wrong though. I was expecting some kind of static initialization somewhere if the solution was to infact do the initialization first before trying to play notes, so this might not be the right place to look, however the code in reSID::SID doesn't seem to be doing any of that.

  /* Determine if we need to recalculate table, or whether we can reuse earlier cached copy.
   * This pays off on slow hardware such as current Android devices.
   */
  if (fir && fir_RES_new == fir_RES && fir_N_new == fir_N && beta == fir_beta && f_cycles_per_sample == fir_f_cycles_per_sample && fir_filter_scale == filter_scale) {
      return true;
  }
  fir_RES = fir_RES_new;
  fir_N = fir_N_new;
  fir_beta = beta;
  fir_f_cycles_per_sample = f_cycles_per_sample;
  fir_filter_scale = filter_scale;

  // Allocate memory for FIR tables.
  delete[] fir;
  fir = new short[fir_N*fir_RES];

  // Calculate fir_RES FIR tables for linear interpolation.

[firewall1110]

however the code in reSID::SID doesn't seem to be doing any of that.

Yes I have tested (#7673) with one line removed:

SID::SID()
{
  // Initialize pointers.
  sample = 0;
  fir = 0;
  fir_N = 0;
  fir_RES = 0;
  fir_beta = 0;
  fir_f_cycles_per_sample = 0;
  fir_filter_scale = 0;

  sid_model = MOS6581;
  voice[0].set_sync_source(&voice[2]);
  voice[1].set_sync_source(&voice[0]);
  voice[2].set_sync_source(&voice[1]);

 // set_sampling_parameters(985248, SAMPLE_FAST, 44100); // REMOVED LINE

  bus_value = 0;
  bus_value_ttl = 0;
  write_pipeline = 0;

  databus_ttl = 0;

  raw_debug_output = false;
}

And all work, and this is very strange ...

So I do not understand , how #7673 fix bug, but it fix ...

P.S.
There are a lot of not RT safe code , and I plane to make it better (or even RT-safe) in #7638
context. But #7638 is not critical bug (my opinion).

[firewall1110]

If insert line (in SID::set_sampling_parameters)

  // FIR initialization is only necessary for resampling.
  if (method != SAMPLE_RESAMPLE && method != SAMPLE_RESAMPLE_FASTMEM)
  {
    delete[] sample;
    delete[] fir;
    sample = 0;
    fir = 0;
    return true;
  }

fprintf(stderr, "SID::set_sampling_parameters - get after\n"); // DEBUG LINE INSERTED

fprintf(stderr, "SID::set_sampling_parameters - get after\n"); is never called.
So all after (in SID::set_sampling_parameters) seems be a "dead code" ...

[PhysSong]

So I do not understand , how #7673 fix bug, but it fix ...

SID has member variables with type Filter, so creating a SID instance will ensure the initialization code in Filter's constructor is executed, which runs only once.