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

Add "Remove unused channels" option to FX-Mixer #1545

Merged
merged 3 commits into from
Jan 4, 2015
Merged

Add "Remove unused channels" option to FX-Mixer #1545

merged 3 commits into from
Jan 4, 2015

Conversation

DanWin
Copy link
Contributor

@DanWin DanWin commented Jan 2, 2015

This makes it easier to delete all FX-Channels that are not in use.
It addresses Issue #1206
Side effect: If many (20+) channels get removed, this causes a segmentation fault. I could reproduce it by opening Zakarra-OneDay.mmpz in CoolSongs and going to the end of the FX-Line, clicking on the last FX-Channel and holding delete. So this is not my fault, but should be fixed somewhere else.

This makes it easier to delete all FX-Channels that are not in use.
@diizy
Copy link
Contributor

diizy commented Jan 2, 2015

On 01/02/2015 11:09 PM, Daniel Winzen wrote:

This makes it easier to delete all FX-Channels that are not in use.

How is "in use" determined? Also how is the UI implemented for this?

It addresses Issue #1206 #1206
Side effect: If many (20+) channels get removed, this causes a
segmentation fault.

Ok well, how about fixing that first? If you've found a condition that
causes a segfault, there's something that needs fixing...

@DanWin
Copy link
Contributor Author

DanWin commented Jan 2, 2015

How is "in use" determined? Also how is the UI implemented for this?

An FX-Channel is "in use" as soon as an instrument is set to use this FX-Channel.
I simply added a new entry to the context menu of the FX-Line.

Ok well, how about fixing that first? If you've found a condition that causes a segfault, there's something that needs fixing...

I agree, but I don't think that I am able to find a solution for this, as I don't have enough skill. Maybe someone more experienced should try fixing this.

@diizy
Copy link
Contributor

diizy commented Jan 2, 2015

On 01/02/2015 11:27 PM, Daniel Winzen wrote:

How is "in use" determined? Also how is the UI implemented for this?
An FX-Channel is "in use" as soon as an instrument is set to use
this FX-Channel.
I simply added a new entry to the context menu of the FX-Line.

Ok so what about channels that only get input from other channels?

Ok well, how about fixing that first? If you've found a condition
that causes a segfault, there's something that needs fixing...
I agree, but I don't think that I am able to find a solution for
this, as I don't have enough skill. Maybe someone more experienced
should try fixing this.

Where is the segfault originating from? What does the backtrace say?

@DanWin
Copy link
Contributor Author

DanWin commented Jan 2, 2015

Ok so what about channels that only get input from other channels?

I hadn’t thought about that. This is not considered and I should fix this.

Where is the segfault originating from? What does the backtrace say?

The backtrace when deleting with this function:

*** Error in `/usr/local/bin/lmms': corrupted double-linked list: 0x0000000003619590 ***

Program received signal SIGABRT, Aborted.
0x00007ffff4302e37 in __GI_raise (sig=sig@entry=6) at ../nptl/sysdeps/unix/sysv/linux/raise.c:56
56  ../nptl/sysdeps/unix/sysv/linux/raise.c: Datei oder Verzeichnis nicht gefunden.
(gdb) bt
0  0x00007ffff4302e37 in __GI_raise (sig=sig@entry=6) at ../nptl/sysdeps/unix/sysv/linux/raise.c:56
1  0x00007ffff4304528 in __GI_abort () at abort.c:89
2  0x00007ffff4344b04 in __libc_message (do_abort=do_abort@entry=1, 
    fmt=fmt@entry=0x7ffff444db00 "*** Error in `%s': %s: 0x%s ***\n") at ../sysdeps/posix/libc_fatal.c:175
3  0x00007ffff434ab87 in malloc_printerr (action=<optimized out>, str=0x7ffff4449bbc "corrupted double-linked list", 
    ptr=<optimized out>) at malloc.c:4996
4  0x00007ffff434c24f in _int_free (av=0x7ffff468a760 <main_arena>, p=<optimized out>, have_lock=0) at malloc.c:3996
5  0x00007ffff692e8dc in QObject::~QObject() () from /usr/lib/x86_64-linux-gnu/libQtCore.so.4
6  0x0000000000511ce8 in ~FloatModel (this=0x3617d98, __in_chrg=<optimized out>)
    at /home/daniel/LMMS/include/AutomatableModel.h:400
7  FxChannel::~FxChannel (this=0x3617af0, __in_chrg=<optimized out>) at /home/daniel/LMMS/src/core/FxMixer.cpp:83
8  0x00000000005121e0 in ~FxChannel (this=0x3617af0, __in_chrg=<optimized out>) at /home/daniel/LMMS/src/core/FxMixer.cpp:86
9  FxMixer::deleteChannel (this=0xb73dc0, index=index@entry=57) at /home/daniel/LMMS/src/core/FxMixer.cpp:325
10 0x0000000000571cfe in FxMixerView::deleteChannel (this=0xfcaee0, index=57) at /home/daniel/LMMS/src/gui/FxMixerView.cpp:363
11 0x00000000005721ba in FxMixerView::deleteUnusedChannels (this=0xfcaee0) at /home/daniel/LMMS/src/gui/FxMixerView.cpp:424



The backtrace when holding down delete:
Program received signal SIGSEGV, Segmentation fault.
0x00007ffff434bb5d in _int_free (av=0x7ffff468a760 <main_arena>, p=<optimized out>, have_lock=0) at malloc.c:3987
3987    malloc.c: Datei oder Verzeichnis nicht gefunden.
(gdb) bt
0  0x00007ffff434bb5d in _int_free (av=0x7ffff468a760 <main_arena>, p=<optimized out>, have_lock=0) at malloc.c:3987
1  0x00000000004d16f0 in clear (this=0x1d7b768, this=0x1d7b768) at /home/daniel/LMMS/include/ValueBuffer.h:74
2  AutomatableModel::~AutomatableModel (this=0x1d7b6d8, __in_chrg=<optimized out>)
    at /home/daniel/LMMS/src/core/AutomatableModel.cpp:79
3  0x0000000000511d14 in ~BoolModel (this=0x1d7b6d8, __in_chrg=<optimized out>)
    at /home/daniel/LMMS/include/AutomatableModel.h:432
4  FxChannel::~FxChannel (this=0x1d7b4f0, __in_chrg=<optimized out>) at /home/daniel/LMMS/src/core/FxMixer.cpp:83
5  0x00000000005121e0 in ~FxChannel (this=0x1d7b4f0, __in_chrg=<optimized out>) at /home/daniel/LMMS/src/core/FxMixer.cpp:86
6  FxMixer::deleteChannel (this=0xb60000, index=index@entry=33) at /home/daniel/LMMS/src/core/FxMixer.cpp:325
7  0x0000000000571cfe in FxMixerView::deleteChannel (this=0xc38370, index=33) at /home/daniel/LMMS/src/gui/FxMixerView.cpp:363
8  0x00000000005733ac in FxMixerView::keyPressEvent (this=0xc38370, e=<optimized out>)
    at /home/daniel/LMMS/src/gui/FxMixerView.cpp:487
9  0x00007ffff70e081f in QWidget::event(QEvent*) () from /usr/lib/x86_64-linux-gnu/libQtGui.so.4
10 0x00007ffff74a55ce in QFrame::event(QEvent*) () from /usr/lib/x86_64-linux-gnu/libQtGui.so.4

@tresf
Copy link
Member

tresf commented Jan 2, 2015

I'm on stable-1.1 and coincidentally I just ran into this bug. Software segfaults when unused channels are removed. This is most likely related to the new mixer and not Dan's commit as normal usage on stable branch can trigger this.

@tresf
Copy link
Member

tresf commented Jan 2, 2015

In my case, the channels that were removed were just like the OPs tests, it was a 1.0.x project with the extra empty channels. Removing the empty channels triggers this. It isn't every time an unused channel is removed, but it has crashed twice on me now so it's easy enough to reproduce, at least on the OS I'm using at this very moment (Apple).

@diizy
Copy link
Contributor

diizy commented Jan 2, 2015

On 01/02/2015 11:43 PM, Daniel Winzen wrote:

Ok so what about channels that only get input from other channels?

I hadn’t thought about that. This is not considered and I should fix this.

Yep.

Where is the segfault originating from? What does the backtrace say?

The backtrace when deleting with this function:

Seems familiar. Looks very much like a bug I ran into in the lmms2
branch, could be I only fixed it there and didn't think to import the
fix to master... I'll have to look into that.

tracks += Engine::getBBTrackContainer()->tracks();

// go through all FX Channels
for(int i = m_fxChannelViews.size()-1; i>0; --i)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

coding style: always put spaces between operators and values
i > 0 // right
i>0 // wrong

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, adjusted. But for some reason this isn't consistently used everywhere in the code. Sometimes there are spaces and sometimes not.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On 01/04/2015 05:38 PM, Daniel Winzen wrote:

Ok, adjusted. But for some reason this isn't consistently used
everywhere in the code. Sometimes there are spaces and sometimes not.

That's why we have this

https://github.com/LMMS/lmms/wiki/Coding-conventions

diizy added a commit that referenced this pull request Jan 4, 2015
Add "Remove unused channels" option to FX-Mixer
@diizy diizy merged commit b874d3f into LMMS:master Jan 4, 2015
@DanWin DanWin deleted the channel branch January 4, 2015 17:33
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.

3 participants