Comment 21 for bug 1024621

Revision history for this message
Daniel Schürmann (daschuer) wrote :

Hi RJ,

thank you for your review!

> .. delay the loading of tracks on the CueControl ..

I think you refer to my changes in EngineBuffer::slotTrackLoaded. The intention was to prevent the track to be loaded at position 0 and the seek to cue position. Now the track is loaded instantly at at cue position. I think this will not put an additional delay on loading tracks because CueControl::loadTrack was called from the GUI thread just after EngineBuffer::slotTrackLoaded and before the track was is playable.

> .. using -1 playposition ..

"track_samples" = 0 is no reliable indicator to detect that no track is loaded, because EngineBuffer::slotTrackLoaded() sets
"track_samples" to a valid value but "visual_playposition" was still pointing to the playposition of the track loaded before.
"visual_playposition" is reliable after seeking to cue point and after the next EngineBuffer::process() cycle.

In my work for waveform de-jerking in daschuers_trunk I have discard "visual_playposition" in favour of a VisualPlayPosition class
which is designed to sync sound output with the waveform display and deals with the limitations of the Qt event queue.

Do we need such a high precise "visual_playposition" CO for MIDI as well or is it enough for now to refer to the "playposition" CO for Midi?

> .. lack of error handling in the waveforms. ..

For me, my changes are fixing the root bug of this problem as explained above. In engine-control-refactor branch you have moved EngineBuffer::slotTrackLoaded to the Engine thread. When merging, we have to ensure that CueControl::loadTrack is still called from the engine worker pool.

Kind regards,

Daniel