-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Auto dj cross fader center #13628
Auto dj cross fader center #13628
Conversation
pre-commit is complaining. You may test it by adding a space at the end of the affected lines that are shown following "Details" above. If everything works you need to commit twice. |
@davidlmorris took the liberty to fix it for you. make sure you |
I wonder if it's worth adding this option to the controls in the GUI. I mean: is this something you'd want to toggle quickly while mixxxing? without having to go to the preferences |
I would say no. It is what I originally set out to do, but basically, this setting could/would apply to all AutoDj options, so we would end up with 8 rather than 4, which would make that GUI setting too busy and too confusing. I have an idea though for my 'next change' which would require a much bigger effort to dig through and understand the code: Add more 'unseen' cue points, being the first and last (and here I'm just guessing - this needs to be thought out) say: -6db (from ref), - 10db, -20db and maybe others. Then have an AutoDJ option that works like the 'skip silence' option, except that it picks up the timing from, for example, the -6db point (selectable in options), called 'Selectable Fade' or 'Radio Fade' so that you can get much better crossfades (or radio fades) on songs where you don't have to add start and end clue markers. This way a fade-out would be merged better with a cold start. So a cold (abrupt) end would transition perfectly (and tightly) into a cold opening. Naturally which cue marker you used would be selectable in AuduDJ preferences. (I am assuming that adding cue marks would not be a huge pain or overhead- given they would need to be checked and loaded with each song - and I presume updated in the DB ... lots here I don't know about that I (or someone) would need to check). |
@Swiftb0y Thankyou! Brilliant! (I have done the pull. But until I am confident I won't break things further, will abstain from any other git shinanigans.) |
What I meant was: have this option in the Auto DJ controls row, too. (this does not require duplicating all transistion modes) |
I am in doubt that this option suits for every transition mode. For instance if you have defined an into of a track you may not want to start it immediatly at full volume. In the other hands there are tracks that start with a significant sound like a signature vocal. that shall not be fadet in, because it is silenced at the beginning. In addition I want not fade in Track started at 0:00, because the usually have their own "intro". Sometimes I fade the previous track out to make the signature sound of the new track even more notable. So It is a case by case decision. So we may define these rules:
What is your use case? How we can distinguish the intro types? |
Thanks, that's excactly why I proposed to have the option in the skins, not only in the preferences.
I don't think we should assume the track has no intro/outro just because users didn't configure it. |
Yes, I can confirm that. On the other hand I like to be able to set the behavior track by track, like we do with the intros. My rational was that: If a track has an optionsl intro set, it is assumed to faded in. We have no other option yet. In addition I think the default should be no fade in. |
From a radio perspective, there is almost NO 'use case' for when you would want to fade a track in. (Ok there is one: when you fade an instrumental in from the middle so you can time a cold ending to the top of the hour where you have time pips like BBC World Service or a syndicated news bulletin... but this isn't an AutoDj thing.) I've never worked in or seen a radio studio with a 'crossfader'. It just isn't (wasn't?) a thing. Every announcer I've seen starts the track at (or near full volume) talking over the intro, or running a sweeper, or doing nothing as required. On some public and community radio stations I have heard the occasional fade-in, but it sounds unprofessional and messy and was probably a mistake. Perhaps this is different in other parts of the world? So from my perspective in my use case, I would have the option checked all the time. I suspect this would be true for most people using it this way. So if a song has an intro specified (and the previous has an outro or not) I would still want to start it at full volume. (In my case when I use the intro/outro I use it as an indicator for when it is safe to voice over music... which still mostly works for AutoDJ) To be clear this is not a full solution to the problem of having the ability to do a 'full radio mix' (more on that later), but at the risk of a pun, it does take us halfway there. I have enormous respect for those that are using Mixxx (and the rest) as musical instruments, manually matching beats, and fading bits and pieces in and out to create something new. That is a skill set I just don't have. And of course, the requirements there are very different. In those cases, I can't imagine AutoDJ is used at all, and a cross-fader might be used a lot. But perhaps Electro and House music lends itself to a fade-in and so for those having that as an option would be useful. Not my area of expertise (at all!). I have no objection to it being part of the UI bar for AutoDJ, but right now, my suggestion would be to include it as the option is proposed. And then as a separate pull request if someone needs it have it as an option of the AutoDJ UI 'bar'. I would be keen to have the feature included as soon as possible, rather than stalled out trying to make it perfect. After all, there is simply no option at the moment that allows a track to start at full volume in AutoDj. |
I might add, that I use the 'Skip Silence' with the cross-over time set to 5 seconds currently. This works with the center option as well as you would expect (and a whole lot better than a fade-in). Even using 'Full Track' and adding silence would work better for me since you are hearing the full track as the creator intended (like an LP or CD) without a fade-in. |
I can confirm.
We are in a good progress to make it perfect. I like rather include only your use case as a special transition than increase the complexity of the Auto DJ by making it a separate option. So it looks like we already have a conclusion .. Skip Silence is the same as the two intro/outro modes in case no intros and outros are defined. Do you have some defined? My guess is if a track has defined an outro, you want to use it, right? If not, you like the 5 second fade out as described. This is "Fade At Outro Start". Which has currently the IMHO buggy behaviour that it fades a track into lyrics or such if you have a long outro and no intro defined. So I think this can be fixed by your mode. In case the is no intro, start with full volume at sound start. I don't think there is any use case for an uncontrolled long intro in that case. Does it work for you? |
I do. While most work as advertised (as you stated), occasionally I marked some from the last vocal with a very extended outro, mostly so I have a mark for starting an extended talk break over the tail. Unfortunately, this sometimes makes the crossfade seem a bit messy, making the song seem like it has been cut short - which it has, so I just leave it on skip silence. I have tested the change against all or the options, and they all seem to work. So to be clear this works for every type of AutoDj transition currently, and I would be against limiting it to any particular one. So I would be keen to have the change (as it is) included as soon as possible. The only reason that I would not propose making it a default, is that I don't think that changes that 'break expectations' or 'break behavior' should be introduced as a default, even if I suspect most people will end up using it that way. |
The issue I have with this extra toggle, that this is clearly a "per track" option. You can either have club tracks with an intro you like to fade in and a track that starts with vocals that should be started with full volume. I want a feature that allows unattended mixes as well. I am not afraid to break a feature in favor of it. We will learn if this was good or bad during the beta period and may change the implementation. So how about doing both, add another skip silence option with only fade out and alter the "Fade At Outro Start" feature as described? |
I thought AutDJ was really only for unattended mixes (although I also use it as a 'current' playlist), so a per-track option is not a 'live' option unless you have gone through each song on the playlist and set this up, right? Or have I misunderstood what you are suggesting? This doesn't mean I would be against another separate option. My solution is just a quick fix for the problem of tracks not starting at full volume when using AutoDj, regardless of what setting you are using. One I would be very keen to see implemented ASAP. |
Auto DJ shall be usable for complet unattanded mixing but also allow seamless interaction with a real DJ.
Yes, right. For many of my track I have only set the cue point, used as load cue. Some have also a difined intro and outro.
The issue with the extra option for all transition is that it is more than a quick fix for your particular use case, but it still does not help for my use case. It may limit use for future improvments because some useres will get used to this option and we may hasitate to remove it. |
Proposal: Later or in addition, we may also add the workaround for my use case. |
You say that like it is a bad thing?
Why limit this? It is after all a setting in preferences, that doesn't affect extant behavior at all. If you want less, then don't set the check box. If you want more... then create a new feature that can be implemented later, right? What problem are we trying to prevent here? |
Yes, I mean it actually: https://www.youtube.com/watch?v=glZ1C-Yu5tw&t=206s
because it breaks the concept of full auto, for the intro/outro related modes. |
How? Just don't set the checkbox (like it is now). Are we trying to do too many things here? |
I watched this. I did find him a tad hyperbolic. It is an argument of usability and functionality over the complexity of development. These things are, of course, a trade-off. In my view, not an argument for an incomplete user experience. However, this isn't my software - it is yours or your team's. If you think this is a bad proposal, then I understand that you won't include it, even if I don't understand the reasoning. Do let me know, since I would like to adapt it for my purposes (since I do think Mixxx is great software), and I would like to fork a more stable version than the nightly build. |
Oh sorry, it was not my intention to reject your work. It was probably also a bad idea to post the video, which shows the issue exaggerated. I highly appreciate your work and the fader trick you demonstrate here is really a missing feature. |
Let's summe up:
I am happy to merge your demand. That's why I have proposed to go in small steps and only solve your problem, which is for my understanding covered with ""Skip Silence, fade out". Once this is merged, we can build on that in any direction. |
Thanks. (I was trying to reduce complexity, while not messing with other user's expectations.) For the most part "Skip Silence, fade out" is at least good enough for now (and certainly better than nothing!). I can see situations where I might not be skipping silence (dinner party background et al) or using the into and outro, but "Skip Silence" is certainly my most pressing requirement. I have a further more detailed idea which I hinted at above... which would require a much bigger effort to dig through and understand the code, the biggest part probably is to add more 'unseen' cue points, being the first and last -6db (from ref), - 10db, -20db and maybe others (I would need to play around and hear the results). What would you see as the biggest problem with adding hidden cue points like the existing 60db point? (e.g. Database reformat?) |
One other thing worth mentioning is the potential unintended consequence for someone who has set (under preferences Mixer) the Crossfader Curve to Constant power. |
I can confirm this. In a new test I just did.
My demand for this PR is to turn you option in to a new transition mode as discussed. Any other changes can be done in follow up PRs.
I have currently other priorities, like releasing 2.4.2. Sorry.
I have tested your original PR and did a review here without testing. I have probably misread your code. |
@@ -566,6 +568,8 @@ AutoDJProcessor::AutoDJError AutoDJProcessor::toggleAutoDJ(bool enable) { | |||
setCrossfader(1.0); | |||
} | |||
} | |||
m_crossfaderStartCenter = m_pConfig->getValue<bool>(ConfigKey(kConfigKey, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
m_crossfaderStartCenter should be also initialized in the constructor.
Currently we need to restart auto DJ to switch the modes. When it becomes a transition mode, it can be already changes on the fly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As far as I can tell AutoDj is instantiated just once at the program start, so I've made the change even if I think is a tad redundant for completeness. "Currently we need to restart auto DJ to switch the modes.", is true, like a lot of changes made in options.
While I have made the changes I mentioned above to my code. I won't be making any further commits until I understand git better and have understood the implications of other changes, some of which as I said before I am uncomfortable with. (I've been playing with it this morning, and it seems to make even cold cuts sound 'ok-ish' [as in so, so much better] with a 4-second crossfade time - so I'll modify version 2.4 for my use in the meantime, and maybe 2.5 when it comes out). |
Ok, I pushed this to my repository... forgetting that it was connected to this pull... hopefully, this is not an 'ooops' situation again. These are changes made or suggested above. |
Can you also re-introduce the the new transition mode rather than the preferences option? Than this is immediately merge-able IMHO. |
As "demanded": I've left in the global m_crossfaderStartCenter and supporting code in Not included: is a much much better transition, which (I assume) I should do as a separate pull request, that creates some hidden cue points, which correspond to the first and last sound in the track at specific volumes, and then uses those to start the fade out, up to a maximum set by the user in the spin box. This means that cold endings and cold starts can mesh perfectly, and fade-outs happen sensibly. Other than a few gremlins which I "think" I have sorted out - but will test more, I think it now does a better job "mixing tracks" than I do especially on cold endings! |
Build Ubuntu 22.04 trips up when I let a case fall through (deliberately) on a switch statement.
Is there some warning suppression I can/should apply here? |
I've added this (C++17 option) :
to see if it will pass. |
Thank you. |
I can enter that command, but I confess this will be 'cargo cult' scripting at its worst since I don't know git enough to really understand what you are asking, and won't know what to do if it breaks. (I've been using the GitHub Windows app.) For that reason, I suggest you do it. |
…l Volume". As an option to set the Crossfader to the center as AutoDj starts a crossfade so the track starts at full volume.
b8b4f88
to
fad5b9d
Compare
Update auto dj fade mode tooltip. Nice catch. I managed to miss that one. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All green now. Thank you for this nice new mode.
My pleasure. And, thank you. I'll create another branch (soon... a few things on the go) and upload the Pull Request for the 'feature request' AutoDj: New transition mode 'Radio Laneway' #13716 |
I like this new mode a lot. It sounds more "natural". |
I hadn't especially considered it, but it would be trivial to implement. I can do this if you like. I'll basically just add (yet another) transition mode, called "Fade At Outro Start, Start Center", using the same code (effectively). I'll probably do that as a separate pull (for practice), and then do the 'Radio laneway crossfade" one as another pull. Feel free to 'assign' me to those jobs if you file an issue. |
This #11648 |
Ok I will file a new issue. Yet another transition mode does not work for me. It shall auto detect from the intro and intros if starting at center is desired or not. |
cf Yet another for my oblique humour. From #11648 and "It shall auto detect from the intro and intros if starting at center is desired or not", sounds like a non-trivial change from "Fade At Outro Star" as it stands (I was expecting to just add another case to the switch statement with one line). I think this might be best left to after you get a chance to see #13716 in action since that (just) might solve your issue. |
In the tool tips can we change |
Fixed in e2abcf9 |
[An attempt to fix my understanding of git, by creating a new branch to fix code format 'issues'.]
edit(ronso0) original PR is #13619
edit (davidlmorris) This is now a new AutoDj transition option 'Skip Silence Start Center', that works exactly like the 'skip silence' transition and takes the same time, except that it now moves the cross-fader to the centre at the start of the transition, so that the next (the 'to' deck) track will start at full volume.
Note: this might lead to a sudden reduction in the volume of the playing deck if you have set the Crossfader Curve to Constant power. See Preferences->Mixer and make sure 'Slow fade/Fast cut (additive)' is set (And move the fader to the position somewhere close to where the 'Logarithmic chart' looks like a triangle).