-
-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Rendering looped sections multiple times on export (#4624) #4639
Conversation
Downloads for this pull requestGenerated by the LMMS pull requests bot. |
I did some simple tests. It worked well even with automated time signatures. 🎉 |
@PhysSong, thanks. I haven't had a chance to address this yet, but I noticed that the existing code doesn't seem to follow the coding conventions you stated. At the risk of being a little too snarky, I think its worth mentioning that if the existing code doesn't follow the conventions that the project doesn't really have a convention... However, I will make the changes you've requested. @Umcaruje / @RebeccaDeField, please review the GUI changes. Thank you. |
@PhysSong, the whitespace changes you requested were addressed. I also corrected one inconsistency in the .ui file that didn't appear to me to do much, but might have been important for other platforms. |
Testing the AppImage above. It looks like the code has been changed since so I'll try and compile this later. I think this works really well and looks good.
This is too little. There is room for more there so think big. I saw one user complaining about issues with tracks of the size 2h and longer. One may want to render a long drone for a hippie convention etc. I'd go with |
This is good to merge. I haven't gone over the code with a magnifying glass but it looks good. Some thoughts.
|
Merge? |
Done. Bumped it to 99. |
@StevenChristy Merged, thanks for implementing this! |
m_playPos[Mode_PlaySong].m_timeLine->loopBegin() : MidiTime(0,0); | ||
m_exportLoopEnd = m_playPos[Mode_PlaySong].m_timeLine->loopBegin() < m_exportSongEnd && | ||
m_playPos[Mode_PlaySong].m_timeLine->loopEnd() <= m_exportSongEnd ? | ||
m_playPos[Mode_PlaySong].m_timeLine->loopEnd() : MidiTime(0,0); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I found that m_timeLine
is null if LMMS runs without GUI. As a consequence, this breaks the command-line rendering.
I think we can work around this by not setting m_exportLoopBegin
and m_exportLoopEnd
when the timeline is null because we don't use that in CLI renders.
FYI, that check is not needed elsewhere because LMMS doesn't support "export between loop markers" feature on the command line(for a similar reason).
@tresf suggested in #4624 that this feature might be useful for inclusion in LMMS.
This pull request adds the ability to render looped sections multiple times when exporting. A spinbox is added to the export dialog that allows one to specify how many times the looped section would be rendered while exporting. This defaults to 1 but the user can specify a value to allow the section between looping markers to be rendered up to 9 times (change the spinbox maximum in export_project.ui to increase this if a greater number than 9 is desired).
Feature was tested with both Export Project and Export Tracks but is not applicable to Export to MIDI. The method for calculating export progress needed to be changed to allow the progress bar to give a correct estimate for the percentage completion. There are no known limitations or side effects other than what is described above.