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

add fade_in_length() and fade_out_length() methods to audioregion #592

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

Houston4444
Copy link
Contributor

Hi.
This has been discussed with Robin on IRC, this PR adds fade_in_length() and fade_out_length() methods to audioregion.

The goal is to give access to theses values from a Lua script.

I also give a script that makes a new playlist in selected tracks with only audible regions. That means it cuts or removes regions if they are below other ones (except during their fade of course). The new playlist is named track_name.MAIN because I attempt to do another script to copy selected regions to the track_name.MAIN playlist.
The idea came making music, with many takes on a guitar track playlist, and always have to move many regions to get a silence at a moment. This script allows to have a very clean playlist easier to edit with exactly the same audible contents.

The script has no undo method because I didn't find any way to remove a playlist.
Cheers.

…give access to theses values from Lua script. Give a script example that makes a new playlist in selected tracks with only audible regions.
@Houston4444
Copy link
Contributor Author

mmmh, all things considered I probably made a mistake.
my fade_in_length() and fade_out_length() methods returns a double, then I use math.floor() Lua method in my script to convert it in int.
Length here is a number of samples, so the C++ method should directly return an integer, don't you think ?

@pauldavisthefirst
Copy link
Contributor

yes, these methods should return samplecnt_t not double

@Houston4444
Copy link
Contributor Author

Ok that is done.
I don't know your politic about embedded Lua scripts, this script copies current playlist of selected tracks, renames the new playlists to track_name.MAIN, and clean their contents to only audible regions. This way another script can easily copy selected regions to playlist named track_name.MAIN. I want to try this workflow.

Maybe you will prefer that the script only clear the playlist of selected regions, and this action could be undo.

@pauldavisthefirst
Copy link
Contributor

I was going to rebase + merge this, but there's a merge conflict even after fixing the rebase conflict. If you don't mind fixing that up against master, I'll be happy to merge it.

@Houston4444
Copy link
Contributor Author

conflict fixed !

Copy link
Member

@x42 x42 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NIce very elaborate example script too!

Few comments inline

samplecnt_t
AudioRegion::fade_in_length ()
{
samplecnt_t fade_in_length = (samplecnt_t) _fade_in->when(false);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why use a temporary variable instead of return _fade_in->when(false)?

ControlList::when (false) returns the position of the last point, which is only the length if ControlList::when(true) == 0.

Either name the function fade_in_end_position(), or subtract _fader_in->when(true).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why use a temporary variable instead of return _fade_in->when(false)?

TBH, I can't remember, indeed it's useless.

I didn't know it is possible that fade in could start before or after the region start. Is it really possible ? I don't see how to do it in the GUI. The script doesn't manage this possibility.

AudioRegion::fade_out_length ()
{
samplecnt_t fade_out_length = (samplecnt_t) _fade_out->when(false);
return fade_out_length;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see above comment for AudioRegion::fade_in_length()

-- s : A___________D

-- worst case, compared region rg is above r,
-- rg starts after and finish before r.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

whitespace (should use tabs for indent)

Comment on lines +66 to +72
if ra:fade_in_active() then
r_no_cut_before = r_front + ra:fade_in_length() + 64
end

if ra:fade_out_active() then
r_no_cut_after = r_end - ra:fade_out_length() - 64
end
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens if the region is shorter than 128 samples?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

regions shorter than 128 samples are not copied to the new playlist, because an isolated region with a such length could only be noise. But at this point of the script we just check from when we could cut the compared regions.

@pauldavisthefirst
Copy link
Contributor

Any plans to polish this up for use in 7.0?

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

Successfully merging this pull request may close these issues.

3 participants