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

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

Closed
Bacadam opened this issue Jun 11, 2023 · 29 comments
Closed

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

Bacadam opened this issue Jun 11, 2023 · 29 comments

Comments

@Bacadam
Copy link
Contributor

Bacadam commented Jun 11, 2023

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
Copy link
Member

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?

@daschuer daschuer added this to the 2.4.0 milestone Jun 11, 2023
@ronso0 ronso0 added the autodj label Jun 12, 2023
@ronso0
Copy link
Member

ronso0 commented Jun 12, 2023

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

@daschuer
Copy link
Member

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
Copy link

ikr83 commented Jun 12, 2023

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

@ikr83
Copy link

ikr83 commented Jun 12, 2023

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
Copy link

ikr83 commented Jun 12, 2023

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
Copy link

ikr83 commented Jun 12, 2023

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
Copy link

ikr83 commented Jun 12, 2023

@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
Copy link
Member

ronso0 commented Jun 12, 2023

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
Copy link
Contributor Author

Bacadam commented Jun 13, 2023

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

  • If the cross-fade is 0 seconds, it is ignored and either intro or outro length is used (like in @ikr83 first diagrams)
  • If the cross-fade is positive, it acts as a maximum transition time when intro or outro is missing, like I suggested
  • I can't think of any use case of a negative value using intro and outro cues, but I might be wrong. So negative could be treated as 0.

@Bacadam
Copy link
Contributor Author

Bacadam commented Jun 13, 2023

@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
Copy link
Member

daschuer commented Aug 13, 2023

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.
#2103 (comment)

Some other ideas have been discussed:
#2103 (comment)
#2103 (comment)

@daschuer
Copy link
Member

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
Copy link
Member

Let me summarize:

The missing use-case is:

  • Transition from song with a undesired long outros, to song with a not marked, but short intro.
    -> assume short intros by default

On the other hand we have the demand:

  • Transitions should be controlled by the outro only. Intro end is only used as a deadline for some tracks with short intros.
    -> assume long intros by default.

@daschuer
Copy link
Member

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

  • You want a reliable Transition Start at Outro Start
  • You want to set Outro start with no duration. Currently the track end is adopted as outro end, which can be considered as a bug IMHO.
  • You want to set an Intro End with no duration. Currently the track is loaded at the audible sound start point instead. This can be also considered as a bug.

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
Copy link

ikr83 commented Aug 14, 2023

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
Copy link
Member

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
Copy link

ikr83 commented Aug 23, 2023

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
Copy link
Member

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 #11830 as well? Maybe we can get that fixed before the release.

@ikr83
Copy link

ikr83 commented Aug 23, 2023

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
Copy link
Member

ronso0 commented Aug 23, 2023

@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
Copy link

ikr83 commented Aug 23, 2023

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

@daschuer
Copy link
Member

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

@ikr83
Copy link

ikr83 commented Aug 23, 2023

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
Copy link
Member

#11830 aims to implement your suggestions from:
#11648 (comment)

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
Copy link
Member

Can you confirm these changes?

@ikr83
Copy link

ikr83 commented Aug 23, 2023

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

@Bacadam
Copy link
Contributor Author

Bacadam commented Sep 3, 2023

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
Copy link
Member

ronso0 commented Jan 3, 2024

fixed by #11830

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants