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.

Sign up for GitHub

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

Jump to bottom

Don't restore audio device during exporting #4083

Merged
merged 2 commits into from Mar 2, 2018
Merged

Don't restore audio device during exporting #4083

merged 2 commits into from Mar 2, 2018

Conversation

PhysSong
Copy link
Member

This PR fixes deadlock on multi-track export with SDL.
Fixes #4032.

@PhysSong
Don't restore audio device during exporting
e97ec96 
Fixes deadlock on multi-track export with SDL
@PhysSong PhysSong requested review from zonkmachine and removed request for zonkmachine December 30, 2017 12:25
PhysSong
PhysSong commented Jan 10, 2018
View reviewed changes
src/core/Mixer.cpp
@@ -630,6 +627,17 @@ void Mixer::setAudioDevice( AudioDevice * _dev,



void Mixer::storeAudioDevice()
{
if( !m_oldAudioDev )
Copy link
Member Author

Choose a reason for hiding this comment

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

I've included this check, but looks redundant. Maybe removing is better?

zonkmachine
zonkmachine reviewed Jan 11, 2018
View reviewed changes
src/core/ProjectRenderer.cpp
@@ -103,8 +102,6 @@ ProjectRenderer::ProjectRenderer( const Mixer::qualitySettings & qualitySettings

ProjectRenderer::~ProjectRenderer()
{
Engine::mixer()->restoreAudioDevice(); // also deletes audio-dev
Engine::mixer()->changeQuality( m_oldQualitySettings );
Copy link
Member

Choose a reason for hiding this comment

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

Make sure #3680 still works. If these test projects renders, as instructed, it's a 'go' from me.

Copy link
Member Author

Choose a reason for hiding this comment

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

@zonkmachine I'm pretty sure #3680 will still work because RenderManager destructor is called after ProjectRenderer destructor. However, testing is always good. I'll double-check it!

Copy link
Member Author

Choose a reason for hiding this comment

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

@zonkmachine Tested. It still works!

Copy link
Member

Choose a reason for hiding this comment

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

OK. I think you should merge.

zonkmachine
zonkmachine approved these changes Jan 30, 2018
View reviewed changes
Copy link
Member

@zonkmachine zonkmachine left a comment

Choose a reason for hiding this comment

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

Tested, works!
I don't have insight enough to review the code. It looks mighty fine though.

@zonkmachine
Copy link
Member

Merge?

@zonkmachine
Copy link
Member

I've managed to get it hung on the last step but can't replicate it. I don't know if I had #4189 cherry-picked on top when this happened or not. I think so but it seem to work fine generally, with or without it.

[Thread 0x7fffb1b10700 (LWP 15233) exited]
[New Thread 0x7fffb1b10700 (LWP 15309)]
[Thread 0x7fffb1b10700 (LWP 15309) exited]
^C
Program received signal SIGINT, Interrupt.
__lll_lock_wait () at ../nptl/sysdeps/unix/sysv/linux/x86_64/lowlevellock.S:135
135	../nptl/sysdeps/unix/sysv/linux/x86_64/lowlevellock.S: No such file or directory.
(gdb) bt
#0  __lll_lock_wait () at ../nptl/sysdeps/unix/sysv/linux/x86_64/lowlevellock.S:135
#1  0x00007ffff7bc6664 in _L_lock_952 () from /lib/x86_64-linux-gnu/libpthread.so.0
#2  0x00007ffff7bc64c6 in __GI___pthread_mutex_lock (mutex=0xd80320) at ../nptl/pthread_mutex_lock.c:114
#3  0x00007ffff62ab5fe in SDL_mutexP (mutex=<optimized out>) at ./src/thread/pthread/SDL_sysmutex.c:108
#4  0x000000000058265c in AudioSdl::stopProcessing (this=0xe7a5e0) at /home/zonkmachine/builds/lmms/lmms/src/core/audio/AudioSdl.cpp:110
#5  0x0000000000533fe3 in Mixer::stopProcessing (this=0xc45210) at /home/zonkmachine/builds/lmms/lmms/src/core/Mixer.cpp:251
#6  0x0000000000534f0a in Mixer::changeQuality (this=0xc45210, _qs=...) at /home/zonkmachine/builds/lmms/lmms/src/core/Mixer.cpp:564
#7  0x0000000000552036 in RenderManager::~RenderManager (this=0x189c050, __in_chrg=<optimized out>)
    at /home/zonkmachine/builds/lmms/lmms/src/core/RenderManager.cpp:54
#8  0x00000000005520a6 in RenderManager::~RenderManager (this=0x189c050, __in_chrg=<optimized out>)
    at /home/zonkmachine/builds/lmms/lmms/src/core/RenderManager.cpp:55
#9  0x000000000059b4a9 in ExportProjectDialog::accept (this=0x7fffffffcc30) at /home/zonkmachine/builds/lmms/lmms/src/gui/ExportProjectDialog.cpp:114
#10 0x00000000006438c2 in ExportProjectDialog::qt_static_metacall (_o=0x7fffffffcc30, _c=QMetaObject::InvokeMetaMethod, _id=2, _a=0x7fffffffc400)
    at /home/zonkmachine/builds/lmms/lmms/build/src/moc_ExportProjectDialog.cpp:56
#11 0x00007ffff697087a in QMetaObject::activate (sender=0x189c050, m=<optimized out>, local_signal_index=<optimized out>, argv=0x0)
    at kernel/qobject.cpp:3539
#12 0x000000000064ba9b in RenderManager::finished (this=0x189c050) at /home/zonkmachine/builds/lmms/lmms/build/src/moc_RenderManager.cpp:113
#13 0x00000000005521a1 in RenderManager::renderNextTrack (this=0x189c050) at /home/zonkmachine/builds/lmms/lmms/src/core/RenderManager.cpp:77
#14 0x000000000064b91e in RenderManager::qt_static_metacall (_o=0x189c050, _c=QMetaObject::InvokeMetaMethod, _id=2, _a=0x7fff9c0013c0)
    at /home/zonkmachine/builds/lmms/lmms/build/src/moc_RenderManager.cpp:57

@zonkmachine zonkmachine mentioned this pull request Feb 25, 2018
Merged
@PhysSong
Copy link
Member Author

@zonkmachine What is the backtrace? When did you get that?

@zonkmachine
Copy link
Member

@zonkmachine What is the backtrace? When did you get that?

The backtrace is from crashing/freezing at the end of rendering the project Ashore to separate tracks. I can't replicate this however and think I may have confused this branch and stable-1.2 as I would probably have tested the later to replicate the issue prior to testing the fix. The differences between the logs seem to be that this one is after the very last track is finished rendering and the one in #4032 (comment) is after rendering some two tracks or so.

I think we can ignore this backtrace and merge #4083.

@PhysSong
Copy link
Member Author

The backtrace seems to be from this PR. However, the problem looks like a separate issue and an edge case. I think I can fix that, but probably in another PR.

@zonkmachine
Copy link
Member

I think I can fix that, but probably in another PR.

OK. Merge?

@PhysSong
Copy link
Member Author

PhysSong commented Mar 2, 2018

OK. Merge?

I'll do that soon once Travis-CI build finishes.

@PhysSong PhysSong merged commit 926b654 into LMMS:stable-1.2 Mar 2, 2018
@PhysSong PhysSong deleted the multiexport branch March 2, 2018 04:29
@PhysSong PhysSong mentioned this pull request Jun 6, 2018
Closed
sdasda7777 pushed a commit to sdasda7777/lmms that referenced this pull request Jun 28, 2022
@PhysSong
Don't restore audio device during exporting (LMMS#4083)
bd45599 
Fixes deadlock on multi-track export with SDL
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@zonkmachine zonkmachine zonkmachine approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

SDL - Deadlock with gui option: File->Export Tracks
2 participants
@PhysSong @zonkmachine