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

Crash on changing velocity of a note outside the piano roll #2713

Closed
kamnxt opened this Issue Mar 27, 2016 · 9 comments

Comments

Projects
None yet
6 participants
@kamnxt
Contributor

kamnxt commented Mar 27, 2016

LMMS segfaults after changing the velocity of a note that has been pushed above B8.

How to reproduce:

  1. Create two notes, one higher than the other.
  2. Select both notes.
  3. Drag bottom note all the way up until the top note is above B8.
  4. Change velocity of the top note.

Notes can't be dragged below C0, but they can be dragged above B8 (which causes the problem).

@zonkmachine

This comment has been minimized.

Show comment
Hide comment
@zonkmachine

zonkmachine Mar 27, 2016

Member

Yup, crashes it does. It's also possible to crash it in step 3 above by just draging the bottom note as high as you can if the interval is a Major second or bigger.

Member

zonkmachine commented Mar 27, 2016

Yup, crashes it does. It's also possible to crash it in step 3 above by just draging the bottom note as high as you can if the interval is a Major second or bigger.

@zonkmachine zonkmachine added the bug label Mar 27, 2016

@musikBear

This comment has been minimized.

Show comment
Hide comment
@musikBear

musikBear Mar 28, 2016

I can not reproduce on win32 (1.1.90)
Both in top and bottom, notes that are dragged beyond the resp. c0 & b8, will just 'line up' on one note, no matter the previous position. Changing either velocity or pan for the out-of-view notes causes not a crash. If i use ctrl+a, i can bring the out-of-view note back into piano-roll, and it plays resizes and moves correctly.

I can not reproduce on win32 (1.1.90)
Both in top and bottom, notes that are dragged beyond the resp. c0 & b8, will just 'line up' on one note, no matter the previous position. Changing either velocity or pan for the out-of-view notes causes not a crash. If i use ctrl+a, i can bring the out-of-view note back into piano-roll, and it plays resizes and moves correctly.

@zonkmachine

This comment has been minimized.

Show comment
Hide comment
@zonkmachine

zonkmachine Mar 29, 2016

Member

Bug introduced in f33d1f4 🚜

f33d1f4972b08234d372130be2b6633bfdf85f8d is the first bad commit
commit f33d1f4972b08234d372130be2b6633bfdf85f8d
Author: Vesa <contact.diizy@nbl.fi>
Date:   Wed Jul 9 19:18:17 2014 +0300

    Instrument track, mixer...

@Fastigium I have a hunch this is a case for you. Crash, playhandles, mutex...

The crash you get from changing velocity/pan is only when you drag the handle, not when you use the mouse scroll wheel.

bt full

Program terminated with signal SIGSEGV, Segmentation fault.
#0  0x000000000048e1bb in InstrumentTrack::processInEvent (this=0x20b13c0, event=..., time=..., offset=0)
    at /home/zonkmachine/builds/lmms/lmms/src/tracks/InstrumentTrack.cpp:360
360                         m_notes[event.key()]->setPanning( event.panning() );
(gdb) bt
#0  0x000000000048e1bb in InstrumentTrack::processInEvent (this=0x20b13c0, event=..., time=..., offset=0)
    at /home/zonkmachine/builds/lmms/lmms/src/tracks/InstrumentTrack.cpp:360
#1  0x000000000049b062 in PianoRoll::testPlayNote (this=0x1cd7320, n=0x2579a80) at /home/zonkmachine/builds/lmms/lmms/src/gui/PianoRoll.cpp:1994
#2  0x000000000049cb74 in PianoRoll::mouseMoveEvent (this=0x1cd7320, _me=0x7ffe0bcacfa0) at /home/zonkmachine/builds/lmms/lmms/src/gui/PianoRoll.cpp:2375
#3  0x00000000004a5995 in PianoRoll::mousePressEvent (this=0x1cd7320, _me=0x7ffe0bcacfa0) at /home/zonkmachine/builds/lmms/lmms/src/gui/PianoRoll.cpp:1624
#4  0x00007f774ae2138b in QWidget::event(QEvent*) () from /usr/lib/x86_64-linux-gnu/libQtGui.so.4
#5  0x00007f774add1e2c in QApplicationPrivate::notify_helper(QObject*, QEvent*) () from /usr/lib/x86_64-linux-gnu/libQtGui.so.4
#6  0x00007f774add85dd in QApplication::notify(QObject*, QEvent*) () from /usr/lib/x86_64-linux-gnu/libQtGui.so.4
#7  0x00007f774a6554dd in QCoreApplication::notifyInternal(QObject*, QEvent*) () from /usr/lib/x86_64-linux-gnu/libQtCore.so.4
#8  0x00007f774add7d93 in QApplicationPrivate::sendMouseEvent(QWidget*, QMouseEvent*, QWidget*, QWidget*, QWidget**, QPointer<QWidget>&, bool) ()
   from /usr/lib/x86_64-linux-gnu/libQtGui.so.4
#9  0x00007f774ae4c9eb in ?? () from /usr/lib/x86_64-linux-gnu/libQtGui.so.4
#10 0x00007f774ae4c289 in QApplication::x11ProcessEvent(_XEvent*) () from /usr/lib/x86_64-linux-gnu/libQtGui.so.4
#11 0x00007f774ae73b32 in ?? () from /usr/lib/x86_64-linux-gnu/libQtGui.so.4
#12 0x00007f77476d0e04 in g_main_context_dispatch () from /lib/x86_64-linux-gnu/libglib-2.0.so.0
#13 0x00007f77476d1048 in ?? () from /lib/x86_64-linux-gnu/libglib-2.0.so.0
#14 0x00007f77476d10ec in g_main_context_iteration () from /lib/x86_64-linux-gnu/libglib-2.0.so.0
#15 0x00007f774a6827a1 in QEventDispatcherGlib::processEvents(QFlags<QEventLoop::ProcessEventsFlag>) () from /usr/lib/x86_64-linux-gnu/libQtCore.so.4
#16 0x00007f774ae73be6 in ?? () from /usr/lib/x86_64-linux-gnu/libQtGui.so.4
#17 0x00007f774a6540af in QEventLoop::processEvents(QFlags<QEventLoop::ProcessEventsFlag>) () from /usr/lib/x86_64-linux-gnu/libQtCore.so.4
#18 0x00007f774a6543a5 in QEventLoop::exec(QFlags<QEventLoop::ProcessEventsFlag>) () from /usr/lib/x86_64-linux-gnu/libQtCore.so.4
#19 0x00007f774a659b79 in QCoreApplication::exec() () from /usr/lib/x86_64-linux-gnu/libQtCore.so.4
#20 0x00000000004830b3 in main (argc=1, argv=<optimized out>) at /home/zonkmachine/builds/lmms/lmms/src/core/main.cpp:503
Member

zonkmachine commented Mar 29, 2016

Bug introduced in f33d1f4 🚜

f33d1f4972b08234d372130be2b6633bfdf85f8d is the first bad commit
commit f33d1f4972b08234d372130be2b6633bfdf85f8d
Author: Vesa <contact.diizy@nbl.fi>
Date:   Wed Jul 9 19:18:17 2014 +0300

    Instrument track, mixer...

@Fastigium I have a hunch this is a case for you. Crash, playhandles, mutex...

The crash you get from changing velocity/pan is only when you drag the handle, not when you use the mouse scroll wheel.

bt full

Program terminated with signal SIGSEGV, Segmentation fault.
#0  0x000000000048e1bb in InstrumentTrack::processInEvent (this=0x20b13c0, event=..., time=..., offset=0)
    at /home/zonkmachine/builds/lmms/lmms/src/tracks/InstrumentTrack.cpp:360
360                         m_notes[event.key()]->setPanning( event.panning() );
(gdb) bt
#0  0x000000000048e1bb in InstrumentTrack::processInEvent (this=0x20b13c0, event=..., time=..., offset=0)
    at /home/zonkmachine/builds/lmms/lmms/src/tracks/InstrumentTrack.cpp:360
#1  0x000000000049b062 in PianoRoll::testPlayNote (this=0x1cd7320, n=0x2579a80) at /home/zonkmachine/builds/lmms/lmms/src/gui/PianoRoll.cpp:1994
#2  0x000000000049cb74 in PianoRoll::mouseMoveEvent (this=0x1cd7320, _me=0x7ffe0bcacfa0) at /home/zonkmachine/builds/lmms/lmms/src/gui/PianoRoll.cpp:2375
#3  0x00000000004a5995 in PianoRoll::mousePressEvent (this=0x1cd7320, _me=0x7ffe0bcacfa0) at /home/zonkmachine/builds/lmms/lmms/src/gui/PianoRoll.cpp:1624
#4  0x00007f774ae2138b in QWidget::event(QEvent*) () from /usr/lib/x86_64-linux-gnu/libQtGui.so.4
#5  0x00007f774add1e2c in QApplicationPrivate::notify_helper(QObject*, QEvent*) () from /usr/lib/x86_64-linux-gnu/libQtGui.so.4
#6  0x00007f774add85dd in QApplication::notify(QObject*, QEvent*) () from /usr/lib/x86_64-linux-gnu/libQtGui.so.4
#7  0x00007f774a6554dd in QCoreApplication::notifyInternal(QObject*, QEvent*) () from /usr/lib/x86_64-linux-gnu/libQtCore.so.4
#8  0x00007f774add7d93 in QApplicationPrivate::sendMouseEvent(QWidget*, QMouseEvent*, QWidget*, QWidget*, QWidget**, QPointer<QWidget>&, bool) ()
   from /usr/lib/x86_64-linux-gnu/libQtGui.so.4
#9  0x00007f774ae4c9eb in ?? () from /usr/lib/x86_64-linux-gnu/libQtGui.so.4
#10 0x00007f774ae4c289 in QApplication::x11ProcessEvent(_XEvent*) () from /usr/lib/x86_64-linux-gnu/libQtGui.so.4
#11 0x00007f774ae73b32 in ?? () from /usr/lib/x86_64-linux-gnu/libQtGui.so.4
#12 0x00007f77476d0e04 in g_main_context_dispatch () from /lib/x86_64-linux-gnu/libglib-2.0.so.0
#13 0x00007f77476d1048 in ?? () from /lib/x86_64-linux-gnu/libglib-2.0.so.0
#14 0x00007f77476d10ec in g_main_context_iteration () from /lib/x86_64-linux-gnu/libglib-2.0.so.0
#15 0x00007f774a6827a1 in QEventDispatcherGlib::processEvents(QFlags<QEventLoop::ProcessEventsFlag>) () from /usr/lib/x86_64-linux-gnu/libQtCore.so.4
#16 0x00007f774ae73be6 in ?? () from /usr/lib/x86_64-linux-gnu/libQtGui.so.4
#17 0x00007f774a6540af in QEventLoop::processEvents(QFlags<QEventLoop::ProcessEventsFlag>) () from /usr/lib/x86_64-linux-gnu/libQtCore.so.4
#18 0x00007f774a6543a5 in QEventLoop::exec(QFlags<QEventLoop::ProcessEventsFlag>) () from /usr/lib/x86_64-linux-gnu/libQtCore.so.4
#19 0x00007f774a659b79 in QCoreApplication::exec() () from /usr/lib/x86_64-linux-gnu/libQtCore.so.4
#20 0x00000000004830b3 in main (argc=1, argv=<optimized out>) at /home/zonkmachine/builds/lmms/lmms/src/core/main.cpp:503
@Fastigium

This comment has been minimized.

Show comment
Hide comment
@Fastigium

Fastigium Mar 29, 2016

Contributor

@zonkmachine Thanks for the 🚜!

But about this issue: isn't the real problem that it's possible at all to push notes beyond piano roll boundaries? If I were to fix that, this bug would be fixed, too, right?

Contributor

Fastigium commented Mar 29, 2016

@zonkmachine Thanks for the 🚜!

But about this issue: isn't the real problem that it's possible at all to push notes beyond piano roll boundaries? If I were to fix that, this bug would be fixed, too, right?

@zonkmachine

This comment has been minimized.

Show comment
Hide comment
@zonkmachine

zonkmachine Mar 29, 2016

Member

If I were to fix that, this bug would be fixed, too, right?

Probably...
I'm a bit curious about why you can scroll but not drag the pan/velocity handle.

Member

zonkmachine commented Mar 29, 2016

If I were to fix that, this bug would be fixed, too, right?

Probably...
I'm a bit curious about why you can scroll but not drag the pan/velocity handle.

@Fastigium

This comment has been minimized.

Show comment
Hide comment
@Fastigium

Fastigium Mar 30, 2016

Contributor

@zonkmachine Scrolling takes a different code path, via PianoRoll::wheelEvent() instead of PianoRoll::mousePressEvent() 😉

Contributor

Fastigium commented Mar 30, 2016

@zonkmachine Scrolling takes a different code path, via PianoRoll::wheelEvent() instead of PianoRoll::mousePressEvent() 😉

Teuthis added a commit to Teuthis/lmms that referenced this issue Apr 15, 2016

@Teuthis

This comment has been minimized.

Show comment
Hide comment
@Teuthis

Teuthis Apr 15, 2016

Contributor

I have submitted a pull request for this bug. This is my very first pull request (and my first time using Git), so I hope I observed proper procedure and etiquette.

Contributor

Teuthis commented Apr 15, 2016

I have submitted a pull request for this bug. This is my very first pull request (and my first time using Git), so I hope I observed proper procedure and etiquette.

@tresf

This comment has been minimized.

Show comment
Hide comment
@tresf

tresf Apr 17, 2016

Member

I have submitted a pull request for this bug. This is my very first pull request (and my first time using Git), so I hope I observed proper procedure and etiquette.

Yes you did. One nice trick is to put the words Closes #2713 in the description (rather than the title), which links the merge and the close operations from a bug-tracking perspective.

Also, the title of the PR could be a bit more descriptive. Fix for issue #2713 is hard to understand when reviewing history. Prevent crash when changing note velocity would be a bit better.

This is all minor though. Thanks for the patch. Merging.

Member

tresf commented Apr 17, 2016

I have submitted a pull request for this bug. This is my very first pull request (and my first time using Git), so I hope I observed proper procedure and etiquette.

Yes you did. One nice trick is to put the words Closes #2713 in the description (rather than the title), which links the merge and the close operations from a bug-tracking perspective.

Also, the title of the PR could be a bit more descriptive. Fix for issue #2713 is hard to understand when reviewing history. Prevent crash when changing note velocity would be a bit better.

This is all minor though. Thanks for the patch. Merging.

tresf added a commit that referenced this issue Apr 17, 2016

@tresf

This comment has been minimized.

Show comment
Hide comment
@tresf

tresf Apr 17, 2016

Member

Closed via 946ae85

Member

tresf commented Apr 17, 2016

Closed via 946ae85

@tresf tresf closed this Apr 17, 2016

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