search

mixxxdj / mixxx

Mixxx is Free DJ software that gives you everything you need to perform live mixes.
http://mixxx.org
Other
4.52k stars 1.28k forks source link

AutoDJ don't fallback on crossfade time when intro or outro is not set #11648

Closed Bacadam closed 10 months ago

Bacadam commented 1 year ago

Bug Description

When using AutoDJ in either Full Intro + Outro or Fade at Outro Start, I think the behaviour is not really what it should.

I quote the tooltip :

Full Intro + Outro: Play the full intro and outro. Use the intro or outro length as the crossfade time, whichever is shorter. If no intro or outro are marked, use the selected crossfade time.

Fade at Outro Start: Start crossfading at the outro start. If the outro is longer than the intro, cut off the end of the outro. Use the intro or outro length as the crossfade time, whichever is shorter. If no intro or outro are marked, use the selected crossfade time.

So in both cases, if intro OR outro is not set, AutoDJ should fallback on crossfade time.

But in reality, that's the case only if both outro AND intro are not set. If intro is set, but not outro, then the intro length will be the crosfade time. If outro is set, but not intro, then the outro length will be the crosfade time.

That's not good when you have a song with a long outro, like fade-out, and a transition to a song that starts immediately (no intro). The crossfade time in this scenario is your long outro time, with a long period where both songs and their beats overlap.

I think crossfade setting should represent a maximum transition time when either intro or outro is missing. See the diagrams below,

- is part of a track outside the outro/intro,
o is part of the outro
i is part of the intro
One of the above character represent 1 second

| marks the boundaries of the transition

These examples assume a 5 seconds crossfade setting

Full Intro + Outro, long outro, no intro:
------ooo|ooooo|
         |-----|------

Full Intro + Outro, short outro, no intro:
---------|ooo|
         |---|------

Full Intro + Outro, long intro, no outro:
---------|-----|
         |iiiii|iii---

Full Intro + Outro, short intro, no outro:
---------|---|
         |iii|------

Full Intro + Outro, no outro and no intro:
---------|-----|
         |-----|------

Full Intro + Outro, both outro and intro:
------oo|oooooooo|
        |iiiiiiii|------

Fade at Outro Start, long outro, no intro:
---------|ooooo|ooo
         |-----|------

Fade at Outro Start, short outro, no intro:
---------|ooo|
         |---|------

Fade at Outro Start, long intro, no outro:
---------|-----|
         |iiiii|iii---

Fade at Outro Start, short intro, no outro:
---------|---|
         |iii|--------

Fade at Outro Start, no outro and no intro:
---------|-----|
         |-----|------

Fade at Outro Start, both outro and intro:
------|oooooooo|oo
      |iiiiiiii|------

Version

2.4-beta

OS

Debian 12

daschuer commented 1 year ago

I can confirm that the tool-tip do to reflect the current behavior correctly.

Your arguments of changing the behavior are reasonable and the proposed behavior will work for me. Is there a good reason to keep the old behavior as alternative?

It this still a bug fix for 2.4 or a new behavior that needs to go to 2.5.

What do others think?

ronso0 commented 1 year ago

If users expect the behaviour as described in the tooltip, it's a 2.4 bugfix, no?

daschuer commented 1 year ago

Do you mean a 2.3 bugfix? For me personally it would be OK. The issue is only that it is a significant change and we may argue that the tootip is wrong.

What do you think about that idea?

@ikr83 Does this work in your case?

ikr83 commented 1 year ago

Perhaps there should be two different modes: "Full intro + outro" with Bacadam's request, and "Intro x Outro"

ikr83 commented 1 year ago

In my case, I always fill in the outros for both input and output, but only fill in intros for input, because I want my intros to start at the beginning of the outro and stop at the end of the outro consistently. Therefore, the crossfade is always ignored, which is what I want.

ikr83 commented 1 year ago 

Intro x Outro [Outro + Intro Start only NO CROSSFADE TIME]:
---------|ooooo|-----
      ---i-----|----- where intro duration = 0 become intro duration = outro duration        

Intro x Outro [Outro + Intro End only NO CROSSFADE TIME]:
---------|ooooo|---
      ---|-----i------ where intro duration = 0 become intro duration = outro duration  

Intro x Outro [Outro Start only + Intro NO CROSSFADE TIME]:
---------o-----|----   where outro duration = 0 become outro duration = intro duration  
      ---|iiiii|------- 

Intro x Outro [Outro End only + Intro NO CROSSFADE TIME]:
---------|-----o------ where outro duration = 0 become outro duration = intro duration
         |iiiii|------

Intro x Outro [Outro Start only + Intro Start Only with To to T = CROSSFADE TIME]:
---------o-----|------ where outro duration = 0 become outro duration = CROSSFADE TIME
         i-----|------ where intro duration = 0 become outro duration = CROSSFADE TIME
     T-----T       CROSSFADE TIME

Intro x Outro [Outro End only + Intro End Only with To to T = CROSSFADE TIME]:
---------|-----o------ where outro duration = 0 become outro duration = CROSSFADE TIME
         |-----i------ where intro duration = 0 become outro duration = CROSSFADE TIME
     T-----T       CROSSFADE TIME

Intro x Outro [Outro End only + Intro Start Only with To to T = CROSSFADE TIME]:
---------|-----o------ where outro duration = 0 become outro duration = CROSSFADE TIME
         i-----|------ where intro duration = 0 become outro duration = CROSSFADE TIME
     T-----T       CROSSFADE TIME

Intro x Outro [Outro Start only + Intro End Only with To to T = CROSSFADE TIME]:
---------o-----|------ where outro duration = 0 become outro duration = CROSSFADE TIME
         |-----i------ where intro duration = 0 become outro duration = CROSSFADE TIME
     T-----T       CROSSFADE TIME    

Intro x Outro [No Intro + No Outro with T to T = CROSSFADE TIME]:
---------T-----|
         |-----T------

Intro x Outro [both outro and intro (intro>outro) NO CROSSFADE TIME]:
--------|ooooo|---      Align start markers
     ---|iiiii|iii------

Intro x Outro [both outro and intro (outro>intro) NO CROSSFADE TIME]:
-----ooo|ooooo|---  Align end markers (currently, I dislike it because the outro begins without any control)
     ---|iiiii|------   It is exactly why I never use End Marker on Intro (see first case)
ikr83 commented 1 year ago 
Intro x Outro [both outro and intro (outro>intro) NO CROSSFADE TIME]:
--------|ooooo|ooo---   Align Start markers would be better in my case
     ---|iiiii|------
ikr83 commented 1 year ago

@Bacadam in your case, Intro x Outro with the alignment of the start markers can resolve your issue if you have a little intro and a long outro...

--------|oo|oooooo---   Align Start markers 
     ---|ii|------
ronso0 commented 1 year ago

Do you mean a 2.3 bugfix? For me personally it would be OK. The issue is only that it is a significant change and we may argue that the tootip is wrong.

Yes, I mean 2.3 but I don't expect another 2.3 bugfix release just because of this when the 2.4 release is scheduled for August. So even if it is a behavioral change it's a fix, not a feature.

I'm not in favor of yet another fade mode, I think this will complicate the situation for users. Let's just make the behaviour match the description.

Bacadam commented 1 year ago

We could keep both behaviors in existing modes depending on cross-fade value:

Bacadam commented 1 year ago

@Bacadam in your case, Intro x Outro with the alignment of the start markers can resolve your issue if you have a little intro and a long outro...

--------|oo|oooooo--- Align Start markers 
     ---|ii|------

Yes but that needs me to add short intros on songs that don't really have it (e.g. when the singer starts singing at the very first seconds).

daschuer commented 1 year ago

For reference: All this was developed in #2103

During the development we have removed the Qick Transition mode that uses into or outro and shotes the longer of both. https://github.com/mixxxdj/mixxx/pull/2103#issuecomment-520190611

Some other ideas have been discussed: https://github.com/mixxxdj/mixxx/pull/2103#issuecomment-515795803 https://github.com/mixxxdj/mixxx/pull/2103#issuecomment-520167859

daschuer commented 1 year ago

We could keep both behaviors in existing modes depending on cross-fade value ...

This does not work, because one mode needs to work for any combination of intro and outro definition somehow reasonable. With Transition Time = 0 we have no longer a fall back value for the cases where no other transition time can be calculated.

daschuer commented 1 year ago

Let me summarize:

The missing use-case is:

On the other hand we have the demand:

daschuer commented 1 year ago

@ikr83 Your proposal looks interesting. Let's have a look when what setting can will be used:

So I think the we can adjust the current "Fade at Outro Start" towards your proposal.

Than introduce a new mode that works like @Bacadam has opposed, matching the original description.

Question? Do which of both modes should existing users migrated?

Since this Bug involves User setting migration, it is a important 2.4.0 candidate. Even though we will have no translations for the new mode. I will have a look how complex such a fix would be.

ikr83 commented 1 year ago

Hello,

As a reminder, I don't mark the end of intros. To generalize my initial intent, it would be a mode in which when 1/4 marker is missing, it automatically aligns itself (in time) with the corresponding marker of the transition song. Therefore, we would have an alignment of the tracks, either from both starting markers up to the single ending marker, or from the single starting marker to the two ending markers.

daschuer commented 1 year ago

I think that works already with "Fade at Outro Start". Adjust the default intro start to a downbeat and add a new Outro start at a downbeat as well.

You may also use the "Load at Cue point" and adjust that to a down beat (that is my workflow)

Can you confirm that?

Can you also test #11830 sorrowly? It should implement your suggestions from above.

ikr83 commented 1 year ago

Hi, I can confirm than "Fade at Outro Start" works now (mixxx-2.3.5-7-gbccd6c7e2d) as well as "Full Intro + Outro" worked in the previous version (mixxx-2.3.4-win64).

daschuer commented 1 year ago

Thank you for confirming. We have recently released the 2.3.6 Mixxx. Now we are working on the 2.4.0 release. Can you build an test https://github.com/mixxxdj/mixxx/pull/11830 as well? Maybe we can get that fixed before the release.

ikr83 commented 1 year ago

Firstly, I installed 2.3.6 and it works fine.

Secondly I build the 11830. Here is the process.

Run x64 Native Tools Command Prompt for VS 2019

cd c:/users/david/Desktop git clone https://github.com/mixxxdj/mixxx.git (It "git clone" 2.5 version - I don't know how to "git clone" 2.4 version.) cd mixxx/tools windows_buildenv.bat

correction

11830 mixxx\src\library\autodj\autodjprocessor.cpp

cd ../build cmake .. cmake --build . cpack -G WIX

I installed and tried 11830 (mixxx-2.5-alpha-73-gdfde28dbe4-modified) It works.

ronso0 commented 1 year ago

@ikr83 You built the main branch, which currently is 2.5-alpha. However, the fix you should test has not been merged to a Mixxx branch, it's a Pull Request, #11830. You can download a Windows installer of that PR following the instructions at https://github.com/mixxxdj/mixxx/wiki/Testing --> "GitHub pull requests"

ikr83 commented 1 year ago

Thanks !!! I downloaded, installed and tested mixxx-2.4-beta-89-g06b6692094.msi and it's ok.

daschuer commented 1 year ago

Thank you. Does that meet you expectations as described above? Any additional thought how to proceed with @Bacadam demand?

ikr83 commented 1 year ago

I'm not sure I understand our interactions over the past few days, so I will clarify my recent answers. Initially, I had an issue with transitions in the version 2.3.5, which did not exist in previous versions. The version mixxx-2.3.5-7-gbccd6c7e2d fixed this problem. Version 2.3.6 incorporated the fix, and so did version mixxx-2.4-beta-89-g06b6692094. To do the tests in the past few days, I maintained my initial markers at a ratio of 3 out of 4. A track includes the intro start marker and two outro markers spaced less than 2 seconds apart. I alternated between the "Full Intro + Outro" and "Fade at Outro Start" modes. I also alternated between a 1-second transition, a 2-second transition, and a 20-second transition. In all six cases, I achieved the same result that I was expecting. The duration was the one between the two exit markers.

daschuer commented 1 year ago

https://github.com/mixxxdj/mixxx/pull/11830 aims to implement your suggestions from: https://github.com/mixxxdj/mixxx/issues/11648#issuecomment-1587945933

It changes: Auto-DJ: Don't adopt last sound as outro end, when user has explicit deleted it Use transition time instead if possible, else fall back to last sound.

Auto-DJ: Don't adopt first sound as intro start, when user has explicit deleted it. Use transition time instead if possible, else fall back to first sound.

daschuer commented 1 year ago

Can you confirm these changes?

ikr83 commented 1 year ago

Thanks! Yes, I can see these changes !! It's huge and better !! :)

Bacadam commented 1 year ago

Actually, #11830 is also a solution for my use case. On songs that have real outros (I mean part of the song that can overlap with intro of next song), I can set outro start and outro end. But on songs that just have fade-out endings I can just define the outro start. In this case, with your patch, the transition time is used. I have tested it with some songs I use, it works great. Thanks!

ronso0 commented 10 months ago

fixed by #11830