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

Rare crash on tempo automation when stacking and/or arpeggio is on #4537

Closed
PhysSong opened this issue Aug 12, 2018 · 2 comments
Closed

Rare crash on tempo automation when stacking and/or arpeggio is on #4537

PhysSong opened this issue Aug 12, 2018 · 2 comments
Assignees
Milestone

Comments

@PhysSong
Copy link
Member

Reported by @FairPlay137-TTS#8972 on Discord. An example project that I made:
4536.zip

Here's the backtrace I've got:

(gdb) bt
#0  __memset_avx2_unaligned_erms ()
    at ../sysdeps/x86_64/multiarch/memset-vec-unaligned-erms.S:186
#1  0x000000000050e791 in NotePlayHandle::play(float (*) [2]) ()

It seems like NotePlayHandle::play calls memset when _working_buffer is nullptr.

if( m_totalFramesPlayed == 0 && ! ( m_instrumentTrack->instrument()->flags() & Instrument::IsSingleStreamed ) )
{
memset( _working_buffer, 0, sizeof( sampleFrame ) * offset() );
}

The reason for the crash

  • Stacking and/or arpeggio makes m_usesBuffer of the master note false.
  • NotePlayHandle::resize changes m_totalFramesPlayed on tempo change. SA Envelope + (Rapidly) varying tempo give undesirable output. #3775.
  • In some cases, m_totalFramesPlayed becomes 0 even though the note is being played.
  • Thus, there can be master notes with m_totalFramesPlayed == 0.
  • For those notes, NotePlayHandle::play calls memset with nullptr.

Possible solutions

@PhysSong PhysSong added this to the 1.2.0 milestone Aug 12, 2018
@PhysSong PhysSong self-assigned this Aug 12, 2018
@PhysSong
Copy link
Member Author

A little bit off-topic... Non-MIDI-based instruments check for m_totalFramesPlayed and create plugin data if the value equals 0. However, this issue shows that the method is unreliable. It means LMMS does unneeded allocations and removes the way to access previously allocated data.
I think we can replace the check with m_pluginData check instead.

@PhysSong
Copy link
Member Author

Drop the memset call

Should I implement this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant