Improve PlayHandle removal in Mixer
Nobody has claimed this yet.
- Dominant language
- C++
- Stars
- 10.4k
- Forks
- 1.3k
- Avg merge
- 2d 13h
- Merged PRs (30d)
- 7
Description
## Problem
In ```Mixer::renderNextBuffer()```, when a clear signal is received (by calling ```Mixer::clear()```, which sets ```m_clearSignal``` to ```true```), ```Mixer::clearInternal()``` gets called and it simply adds every ```PlayHandle``` except ```InstrumentPlayHandle```s from ```m_playHandles``` to ```m_playHandlesToRemove```. However, right after ```Mixer::clearInternal()``` is called in ```Mixer::renderNextBuffer()```, a loop iterates through ```m_playHandlesToRemove``` and removes the same ```PlayHandles``` from ```m_playHandles```. In other words, it unnecessarily stores a list of ```PlayHandles``` to be removed only to actually remove then right afterwards. ```Mixer::clear()``` seems to be called only within ```Song::stop()```.
Other than ```Mixer::clear()```, the only method that adds a play handle to m_playHandlesToRemove is ```Mixer::removePlayHandle()```, which is only called by the ```FileBrowser``` class to stop a preset preview ```PlayHandle```. Also, only one preset preview ```PlayHandle``` seems to be active at a time.
## Solution
1. Add the members ```PresetPreviewPlayHandle m_presetPreviewPlayHandle``` and ```bool
m_presetPreviewStopped``` to ```Mixer```.
2. Create the method ```Mixer::addPresetPreviewPlayHandle()```:
```C++
bool Mixer::addPresetPreviewPlayHandle( PresetPreviewPlayHandle *handle )
{
m_presetPreviewStopped = false;
m_presetPreviewPlayHandle = handle;
return addPlayHandle( handle );
}
```
3. Create the method ```Mixer::stopPresetPreview()```:
```C++
void Mixer::stopPreview()
{
m_presetPreviewStopped = true;
}
```
4. Modify ```Mixer::renderNextBuffer()``` so that:
```C++
if( m_clearSignal )
{
m_clearSignal = false;
clearInternal();
}
ConstPlayHandleList::Iterator it_rem = m_playHandlesToRemove.begin();
while( it_rem != m_playHandlesToRemove.end() )
{
// [...]
}
```
becomes something like:
```C++
if( m_presetPreviewStopped && m_presetPreviewPlayHandle != NULL ) {
//Actually stop m_presetPreviewPlayHandle [...]
m_presetPreviewPlayHandle = NULL;
}
if( m_clearSignal )
{
ConstPlayHandleList::Iterator it = m_playHandles.begin();
while( it != m_playHandles.end() )
{
// [...]
}
m_clearSignal = false;
}
```
5. Update ```FileBrowser.cpp``` with the new methods.
Contributor guide
No contributing guide indexed for this repository
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start in Mixer::renderNextBuffer() and Mixer::clearInternal(), then trace the play-handle calls from FileBrowser.cpp and Song::stop(). Compare how clear signals and preset-preview removal currently flow through m_playHandlesToRemove. Done means preset previews stop through the new Mixer methods and clearing no longer queues handles only to remove them immediately.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- cpp
- Domain
- desktop
- Issue type
- Refactor
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 45/100