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

EngineBuffer: avoid std::bad_alloc() in WaveformRenderBeat::draw() when loading a track with active passthrough #4787

Closed
wants to merge 1 commit into from

Conversation

ronso0
Copy link
Member

@ronso0 ronso0 commented Jun 6, 2022

https://bugs.launchpad.net/mixxx/+bug/1977662

See comments for explanations.
I'm not sure if this is the best solution, though I consider it safe since the queued seek position is set as soon as passthrough is disabled.
We may as well prohibit loading tracks in passtrhough decks, though I didn't look into this approach.

The fix for main is slightly different: with mixxx::audio::FramePos we can use setNewPlaypos(mixxx::audio::kStartFramePos);

Note: the 'symptom' of the bug, "Non-terminating iterator, unlimited growth of memory, crash when calling m_beats.resize()." as @uklotzde pointed out, in WaveformRenderBeat::draw() is not addressed here (and I don't know how to fix it).

@ronso0
Copy link
Member Author

ronso0 commented Jun 7, 2022

Note that the confusing CUE blinking with passthrough is also present in 2.3:

  • load a track
  • activate passthough
  • load another track (with empty waveforms, obviously, otherwise it'd crash now)
    = CUE is blinking (if any blining mode is selected of course)
  • click hotcue (other than at CUE pos), click CUE
    = blinking stops, feels like CUE to set it to hotcue position (no visual feedback about current position)
  • deactivate passthrough
    = position jumps to hotcue, CUE is blinking again

So basically any hotcue + cue interaction as well as trying to change the playpos in the track overview is pointless while passtrhough is active. IMO we should reject all cue and overview interaction (except actions in hotcue popup) and deactivate CUE blining. That would be consistent with the blocked waveform. There we should remove the dragging cursor there and add a passthrough overlay to avoid confusion.

OTOH allowing to seek without actually processing the deck buffer is a lot of work since that requires to rework places in EngineBuffer for example which would complicate the code and only be helpful for the (IMO edge) use case of preparing a deck while passtrough is active. Not worth it IMO.
What do you think?

@uklotzde
Copy link
Contributor

uklotzde commented Jun 7, 2022

@ronso0 Okay to close in favor of #4789?

@ronso0
Copy link
Member Author

ronso0 commented Jun 7, 2022

Sure.
I'll move the UX discussion to Zulip.

@ronso0 ronso0 closed this Jun 7, 2022
@ronso0 ronso0 deleted the passthrough-track-load-2.3 branch June 7, 2022 19:42
@ronso0 ronso0 changed the title EngineBuffer: avoid waveform crash when loading a track with active passthrough EngineBuffer: avoid std::bad_alloc() in WaveformRenderBeat::draw() when loading a track with active passthrough Jun 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants