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

GSoC 2020: Mixxx Macros [Moved] #2989

Closed
wants to merge 185 commits into from
Closed

GSoC 2020: Mixxx Macros [Moved] #2989

wants to merge 185 commits into from

Conversation

xeruf
Copy link
Contributor

@xeruf xeruf commented Aug 4, 2020

This is my main contribution for Google Summer of Code 2020. 1af3280 is the last commit that is officially part of GSoC.

To test it, please refer to the Control Specification and have a look at the usage instructions in the blog post.

Overview

This Pull Request is a continuation of #2873 and is supported by other Pull Requests of mine, such as #2980 (for database tests) and #2969 (for the MacroManager, which has been disbanded in the late stages of the project). I also improved some documentation on classes while trying to understand them as seen and merged in #2885 #2925 #2997. Other Pull Requests of mine in the time period of GSoC are often tangientally related contributions that helped me get a better understanding of Mixxx as a whole.

For technical documentation, design and a detailed work log visit the project wiki page.

Features

Core

  • Recording
  • Database Storage
  • Playback
    • Looping
  • Channel-specific Control Objects

GUI

  • Waveform dimming when running (and potentially also when recording)
  • Skin Integration
    • Integrate for Testing (in LateNight)
    • Expose controls properly (similar to WTrackProperty, look into how it's done for cues)
    • Proper visuals

Fixes

Future considerations

  • Controller Integration
  • Serato Ex- and Import

xeruf added 30 commits August 4, 2020 12:09
* Avoid copy constructors in MacroManager constructor
* Use ChannelHandle instead of plain int
* Convert interleaved stereo positions to frame positions for Macro saving
It is done that way in the whole codebase, and the previous version
created nasty bugs when the local copy went out of scope.
@xeruf xeruf requested a review from uklotzde January 8, 2021 11:15
@xeruf
Copy link
Contributor Author

xeruf commented Jan 8, 2021

Looks good again. Also in terms of features and behavior I would deem the core mergable now, so I will go into separating it.

Would also appreciate a review from @ronso0 concerning the skin stuff :)

@xeruf
Copy link
Contributor Author

xeruf commented Jan 8, 2021

Should I remove the skin stuff from this PR or should I split it up and keep this PR as a part of history, sort of?

@ronso0
Copy link
Member

ronso0 commented Jan 9, 2021

So I gave this a try (with the GUI controls only) and like to share my thoughts on the UX and GUI.

  1. it seems a macro is replayed only after the 2nd click on the macro button, even though it changes to Pause icon after the 1st click.

  2. I think toggling macro a macro off should be possible with left click, there's no need to bind it to less accessible right click.
    Edit Hmm..no: clicking the button again while a macro is running restarts the macro, so I sugest to add an individual Stop button.

  3. Macros are bound to tracks, so I think there's no need to show the track title in the macro box. Omitting that also allows to shrink the macro box to the minimum, like small samplers.
    To clarify that macro items belong to the deck above it's sufficient IMO to group them in a container, like effects in effect units.

@ronso0
Copy link
Member

ronso0 commented Jan 9, 2021

Should I remove the skin stuff from this PR or should I split it up and keep this PR as a part of history, sort of?

I think that should be included here to allow using it without controllers even when the styling isn't complete.
Later on we can add proper styles and a special WMacroTitle widget that allows naming macros. I guess that would combine a WLabel and a QLineEdit that is enabled a) on single click (show 'text' mouse when hovering) or b) with a dedicated Edit button.

@xeruf
Copy link
Contributor Author

xeruf commented Jan 11, 2021

Should I remove the skin stuff from this PR or should I split it up and keep this PR as a part of history, sort of?

I think that should be included here to allow using it without controllers even when the styling isn't complete.

My point is that I want to merge the core functionality ASAP without any GUI first to avoid further conflicts and piling up of diffs. So I need to separate them out somehow. Since this PR already contains tons of comments, I think it would be good to move the GUI stuff to a new PR based on this one.

Later on we can add proper styles and a special WMacroTitle widget that allows naming macros. I guess that would combine a WLabel and a QLineEdit that is enabled a) on single click (show 'text' mouse when hovering) or b) with a dedicated Edit button.

Yes sounds good - similar to the text fields in the library.

@xeruf
Copy link
Contributor Author

xeruf commented Jan 11, 2021

1. it seems a macro is replayed only after the 2nd click on the macro button, even though it changes to Pause icon after the 1st click.

Oh right, the pause icon is not really adequate here. Please read the control specification linked at the top to understand the behavior.

2. I think toggling macro a macro off should be possible with left click, there's no need to bind it to less accessible right click.
   **Edit** Hmm..no: clicking the button again while a macro is running restarts the macro, so I sugest to add an individual Stop button.

Stopping a Macro is not a common use-case - think of it more like an "abort". Usually you enable it and let it do its thing. That's why that functionality doesn't need to be very accessible.
With the current behavior repeated invocations allow it to act somewhat like a hotcue as well.

3. Macros are bound to tracks, so I think there's no need to show the track title in the macro box. Omitting that also allows to shrink the macro box to the minimum, like small samplers.
   To clarify that macro items belong to the deck above it's sufficient IMO to group them in a container, like effects in effect units.

Yeah, that title was there only for testing.

@xeruf
Copy link
Contributor Author

xeruf commented Mar 17, 2021

I am very sorry that this is now laying dormant for so long. Unfortunately, university started back up right after GSoC, so I wasn't able to bring it to a close then. The biggest issue is that I cannot simply take a spare hour to polish this up, as I have to work myself into the code again, so I will have to allocate multiple days at some point.

Nevertheless, the core is basically finished, so I would then move all the GUI stuff out of this PR to finally merge this and make the controls already available for controller mapping while integrating the visuals later.

@Be-ing
Copy link
Contributor

Be-ing commented Mar 17, 2021

I would be fine merging the backend of this even if we are uncertain about the ControlObject APIs. IMO we don't need all those details figured out until we make a GUI and making the GUI will help us figure out the requirements.

@github-actions
Copy link

This PR is marked as stale because it has been open 90 days with no activity.

@github-actions github-actions bot added the stale Stale issues that haven't been updated for a long time. label Jun 16, 2021
@xeruf
Copy link
Contributor Author

xeruf commented Nov 16, 2021

Alright, I am back hoping to make some progress as I have some spare time this week, considering I would like to use this feature myself, too :)

@xeruf
Copy link
Contributor Author

xeruf commented Nov 16, 2021

One little test still failing, otherwise looks fine

@github-actions github-actions bot removed the stale Stale issues that haven't been updated for a long time. label Nov 17, 2021
@xeruf xeruf force-pushed the macros branch 2 times, most recently from 21c8816 to 58a96f2 Compare November 17, 2021 11:13
@xeruf xeruf force-pushed the macros branch 3 times, most recently from e0f2705 to 0044c43 Compare November 17, 2021 18:16
@xeruf xeruf mentioned this pull request Nov 17, 2021
@xeruf
Copy link
Contributor Author

xeruf commented Nov 18, 2021

Superceded by other PRs now.

@xeruf xeruf closed this Nov 18, 2021
@xeruf xeruf changed the title Macros GSoC 2020: Mixxx Macros [Moved] Nov 18, 2021
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.

None yet

8 participants