Import saved loops from Serato Tags

[Holzhaus]

Based on: #2526, #2667

This also imports saved loops from the Serato metadata tags. The will be stored as cues of type CueType::Loop. Since hotcues and loops are separate lists in Serato, loop indices are shifted by 8 (which is the maximum number of hotcue in Serato). This means that the first loop has hotcue number 8 instead of 0.

[Be-ing]

How does Mixxx currently behave with multiple LOOP type cues? Does it just ignore them except for the first one?

[Holzhaus]

It ignores them completely for some reason. But I can see them when I run a Select statement on the mixxxdj manually. I didn't investigate it further.

[Be-ing]

I think that is what should happen until we support merge #2194.

[Holzhaus]

Possible, but I expected one of them to show up as the last used loop.

[Be-ing]

Oh, yeah that isn't good. I think Mixxx should use the lowest indexed loop.

[Be-ing]

That could be implemented in a separate PR.

[Swiftb0y]

This means that the first loop has hotcue number 8 instead of 0.

Even though we have support for more than eight hotcues, the skins usually only show the first eight maximum, making the loopcues inaccessible from the skins, right?

[Be-ing]

They will still show on the waveforms and be accessible from controllers. For 2.4 we may consider adding a way to switch between pages of hotcues, which would be similar enough to how Serato switches between regular hotcues and loops.

[Holzhaus]

Yes. But I guess that most Serato users have a Serato controller with a dedicated loop mode that can be used.

[Be-ing]

Regardless, only the first loop is usable currently because we have no GUI nor ControlObjects for managing multiple loops. This PR imports the rest of the loops to avoid data loss so they will be useable in 2.4.

[Holzhaus]

I don't know how Mixxx saves the last used loop though. Might be possible that it just deletes all loop cues before it does that. We'll need to look into that.

[Be-ing]

Loading of loop cues is a mess. Strangely it is handled by BaseTrackPlayerImpl::loadTrack, not LoopingControl. There is a loop there which breaks after the first found loop, as I expected.

[Be-ing]

BaseTrackPlayerImpl::unloadTrack does not break after the first found LOOP cue though, so it saves the last LOOP cue. I think inserting a break; there is all that is needed.

[Holzhaus]

#2678

I also check if a hotcue number is set. We don't want to overwrite our precious saved loops by volatile "most recent loops".

[Holzhaus]

I applied the beta blocker tag because I split this off from #2526 to keep the PR small.

[Be-ing]

I think we should merge #2678 before this.

[Holzhaus]

I resolved the merge conflicts and added support for using saved loops as hotcues for now. Please test.

Here is a sample file:
https://transfer.sh/unO9x/hotcues-and-loops.flac
(Got the track from Soundcloud: Perséphone - Retro Funky (SUNDANCE remix), CC-BY 3.0 licensed, https://soundcloud.com/sundancemusic/pers-phone-retro-funky)

[daschuer]

Some minor comments.

[daschuer]

can we DEBUG_ASSERT(!"unknown cue type") or such?

[Holzhaus]

I don't think so. The loop goes over all cue points regardless of type. I think the other types (Beginning, AudibleSound, etc.) are supposed to be silently ignored.

[daschuer]

LGTM, @Be-ing: merge?

[Be-ing]

No, we need to think through this carefully.

[Be-ing]

I'm unclear what the plan is now between this and #2678. Splitting these into separate PRs has fragmented the discussion. What are you proposing to do with regard to Mixxx's single loop? Using the first numbered loop cue? So the first loop cue is used both for the single unnumbered loop and a hotcue?

[Holzhaus]

What are you proposing to do with regard to Mixxx's single loop? Using the first numbered loop cue? So the first loop cue is used both for the single unnumbered loop and a hotcue?

Yes. If no restored loop from the last session exists, it makes sense to restore the saved loop with the lowest hotcue number instead. As soon as the track is unloaded, it stored as a regular "restored loop" without a hotcue number (the actual saved loop cue will remain unchanged).

[Be-ing]

If restored loop from the last session exists, it makes sense to restore the saved loop with the lowest hotcue number instead.

Why? I would only expect the unnumbered loop to be loaded from an imported numbered loop if there was no unnumbered loop saved.

[Holzhaus]

Sorry, I was answering on my phone and apparently missed to add a "no". You are right, this is exactly what I have in mind. I edited my previous comment.

[Be-ing]

Okay, this seems like a reasonable hack. I'll give it a test with that example file.

[Be-ing]

Okay, I tested and this LGTM aside from some minor improvements to the comments. This seems to work fine without #2678, so what is the plan for #2678? Close it without merging?

[Holzhaus]

No, we still need #2678. Without it, saved loops might be overwritten because it just uses the last loop cue that is found to save the restored loop. Also, Mixxx will always restore the first loop cue in the list which might have a hotcue number - even another loop cue without a hotcue number exists.

[Be-ing]

Thank you!