Page MenuHome

Scale & stretch & cut sound waveform
AbandonedPublic

Authored by Richard Antalik (ISS) on Jun 21 2018, 9:07 PM.
Tokens
"Evil Spooky Haunted Tree" token, awarded by Funkster."Love" token, awarded by tintwotin."Love" token, awarded by vklidu."Love" token, awarded by davidmcsween.

Details

Reviewers
None
Group Reviewers
Video Sequencer
Summary

this patch will:

  • scale waveform by volume.
  • show if waveform "clips", rendering clipping samples in red color
  • stretch waveform so it corresponds to set pitch
  • cut operator will offset start of sped up sound seq, so it seems as it is continuous after cutting
  • playing of sped up sound now should start as it's set by playhead / when seeking / scrubbing

because:

  • often sound in movie is so silent, that waveform is barely visible
  • If you set volume too high, it may lead to clipping
  • If pitch is <> 1, you can not see what you are cutting so waveform is useless/misleading
  • cutting of sped up sound will result in offset sound
  • playing of sped up sound may result in offset sound

side effects:

  • sequence slide operator will move right handle, when moving left handle and pitch is less than 1. Otherwise it should behave normally
  • setting pitch to more than original may result in negative seq->endofs in order to not change length of sequence
  • animated pitch may cause waveform to be offset. To make this precise, you will have to read curve / driver when drawing waveform.
  • cutting sequence with pitch < 1 will result in unprecise cut, as we can not make cut in between frames.
  • if sequence with changed pitch is saved and then opened with older blender, it has changed pitch and to fix it back, sound has to be reloaded.

Diff Detail

Event Timeline

Richard Antalik (ISS) retitled this revision from Scale & stretch sound waveform to Scale & stretch & cut sound waveform.Jul 4 2018, 9:04 PM
Richard Antalik (ISS) edited the summary of this revision. (Show Details)
Richard Antalik (ISS) updated this revision to Diff 11350.

added fetures discussed in T50843 with @Joerg Mueller (nexyon) :

cut operator will offset start of sped up sound seq, so it seems as it is continuous.

some notes to update:

  • it looks like when cutting sound, there may be a little error of +/- 1 frame. I will have to track this down
  • I am using seq->pitch to calculate offsets even in sequences that are not sound. I guess, that seq->pitch is 1 by default. Or should I check if I am working with sound?
  • modifying sped up seq. by grabbing a handle is not OK. Should it behave like it did until this patch?
  • this patch links pitch slider with fn, that calculates length.

The last point is quite problematic, because endofs of seq are not pitch compensated, so changing pitch may result in negative length. See comment in code - rna_sequencer.c
If length is not linked to pitch change, user will have to adjust length manually after pitch change.

Hey, thanks for the update! I tried it and it works really nice for strips with constant pitch. However it doesn't work for animated pitch at all. I think we should go a middle way and don't touch the strip length when the pitch is changed at all. Otherwise we get serious problems with animated pitch strips. The code for cutting and drawing would work fine, but we would have to remove the other changes I guess. Can you try strips with animated pitch? The goal is that the strip start and end don't change during playback.

Hello,
To disable this behavior, you can comment line 698 in rna_sequencer.c - do_sequence_frame_change_update(scene, seq);
But it is glitchy.

Technically it may be possible. My opinion is that animated pitch should be our last problem - is that really a feature, that is used that much?

Thing is that cut and basically all length-changing operators needs correct length of sequence. I can get length before actually cutting and then use original length as endofs.
But to me this seems, that most of the time user will undo manually what will be done by code. (Also I was thinking about patching speed effect to allow controlling 1 video + audio sequence)

I left commented code in the patch (rna_sequencer.c), that would fix end of sequence relative to audio / timeline. But I would need a backup variable that is persistent at least through session. Before I was using some unused var in sequence, but if I can do this with per session variable there will be no compatibility issue.

Just a side question - Are you interested in reviewing my other VSE patches? Because I have some, that are waiting for Sergey Sharybin and Campbell Barton, and I guess that they are busy doing other things.

Hello,

Hi again!

To disable this behavior, you can comment line 698 in rna_sequencer.c - do_sequence_frame_change_update(scene, seq);
But it is glitchy.
Technically it may be possible. My opinion is that animated pitch should be our last problem - is that really a feature, that is used that much?

I don't have usage statistics of the feature. I'd love for it not to exist, because it would get rid of a lot of problems, but it goes against Blender's "animate everything" philosophy. I guess it's not for me to decide though.

Thing is that cut and basically all length-changing operators needs correct length of sequence. I can get length before actually cutting and then use original length as endofs.
But to me this seems, that most of the time user will undo manually what will be done by code. (Also I was thinking about patching speed effect to allow controlling 1 video + audio sequence)
I left commented code in the patch (rna_sequencer.c), that would fix end of sequence relative to audio / timeline. But I would need a backup variable that is persistent at least through session. Before I was using some unused var in sequence, but if I can do this with per session variable there will be no compatibility issue.

The problem I see with the length of the strip changing when for example the pitch changes is that this might mess up the whole arrangement of the strips. Check what happens if you put a strip just behind an audio strip and then lower the audio strip's pitch. The strip jumps to some free space, that might be even off screen and the users will wonder what happened. Turning the pitch back up won't make it come back. So I think the start and end of the strip should only be changed with explicit actions of the user doing so, e.g. grabbing the handles or cutting.

Just a side question - Are you interested in reviewing my other VSE patches? Because I have some, that are waiting for Sergey Sharybin and Campbell Barton, and I guess that they are busy doing other things.

To be honest, you probably know a lot more about the VSE code at this point than I do by just working on this patch alone and I don't have the time right now to dive into it. So I can't really judge whether your code is actually correct/bug free and this patch will have to run through one of these two or another proficient VSE developer anyway. I personally would also add more checks and run your changes only if it's a sound strip, just to be on the safe side and at least not break other strips' functionality in case there is a bug. IMHO the sequencer code is a real mess and needs to be cleanly rewritten from scratch as I've been pleading for since a few years already. I'm amazed by your toughness to deal with it.

Richard Antalik (ISS) edited the summary of this revision. (Show Details)Jul 8 2018, 12:26 PM
Richard Antalik (ISS) updated this revision to Diff 11375.

Now "strips" should be fixed to the timeline when changing pitch,
fix sequence slide operator, at least to the point that it is usable,
fix playback position when playing not from start.

@Joerg Mueller (nexyon) , Thank you a lot for guidance, even if you can not help with complete review, even pointing to a general idea helps. Right now, it seems that no one does have time.

Joerg Mueller (nexyon) added a comment.EditedJul 8 2018, 4:13 PM

Hey! Have you tried this with pitch animated sound? It still doesn't work. I've also found a few other bugs and might even find more if I keep digging.

I think the problem is that you are attacking the problem here with the wrong approach. You are trying to fit everything into the reference of having strips with pitch and then changing the sequence's/strip's value at every point according to that. I think the better approach would be to keep everything as is and only add changes where necessary, leaving the corresponding variables (startofs, len, etc.) of the sequence/strip as they are.

One of these special occasions where you want to alter the variables based on the pitch is the second half of a strip during a cut. Another occasion might be when moving the left handle of a strip. Do you know any other? It's basically whenever the user explicitly edits the start of a strip - these changes would need to be pitch compensated.

Since there are problems with the length/end of a strip when the pitch is changed as you suggested, I'd like to be able to move the handles of a strip outside of their original range as we can do with movie strips. One example: if you add a sound strip and decrease the pitch to 0.5, the strip would need to be twice as long for the full length. Without any modification this is currently not possible. However if I add a movie strip, I can easily grab its end handle and move it beyond the original length, which is displayed as a grey part of the strip. We could allow the same for sound strips to fix the problem.

Lastly, there's the rendering/drawing code that displays the strip. Here your code works fine already, so I don't see any issues there!

Hi,
I currently know about 3 issues:
Cutting of strip leaves gaps sometimes and strip can "jump" channel when changing/animating pitch.
Now I noticed, that when I have startofs, it slowly shrinks, when changing pitch.

I tried to test this as much as possible. If you found any other bug please let me know, how to reproduce it. I will return to this probably next weekend to finish this up.

Hey! Have you tried this with pitch animated sound? It still doesn't work. I've also found a few other bugs and might even find more if I keep digging.
I think the problem is that you are attacking the problem here with the wrong approach. You are trying to fit everything into the reference of having strips with pitch and then changing the sequence's/strip's value at every point according to that. I think the better approach would be to keep everything as is and only add changes where necessary, leaving the corresponding variables (startofs, len, etc.) of the sequence/strip as they are.

It may seem, that I am referencing sequences by pitch, but as long as pitch is 1 most of the changes I did will not be taken into account. I will probably wrap these offsets in if condition, so we can be sure to not distort behavior of other sequence types.

One of these special occasions where you want to alter the variables based on the pitch is the second half of a strip during a cut. Another occasion might be when moving the left handle of a strip. Do you know any other? It's basically whenever the user explicitly edits the start of a strip - these changes would need to be pitch compensated.

I do that only when cutting and when moving handle. Then I change endofs, when changing pitch.

Since there are problems with the length/end of a strip when the pitch is changed as you suggested, I'd like to be able to move the handles of a strip outside of their original range as we can do with movie strips. One example: if you add a sound strip and decrease the pitch to 0.5, the strip would need to be twice as long for the full length. Without any modification this is currently not possible. However if I add a movie strip, I can easily grab its end handle and move it beyond the original length, which is displayed as a grey part of the strip. We could allow the same for sound strips to fix the problem.

You can do it now, just set the pitch first. But limits could be removed easily if they aren't necessary.

Lastly, there's the rendering/drawing code that displays the strip. Here your code works fine already, so I don't see any issues there!

Good, at least something :)

Hi,
I currently know about 3 issues:
Cutting of strip leaves gaps sometimes and strip can "jump" channel when changing/animating pitch.
Now I noticed, that when I have startofs, it slowly shrinks, when changing pitch.

That's what I meant by not changing the variables (startofs, endofs, etc.) during actions that are not specifically changing them. Eg. a pitch change doesn't change them, but cutting does. Changing those variables during non-explicit changing is exactly what causes all the bugs.

I tried to test this as much as possible. If you found any other bug please let me know, how to reproduce it. I will return to this probably next weekend to finish this up.

Other bugs I found: loading old files and cutting their strips doesn't work properly. Pitch animation still doesn't work.

Richard Antalik (ISS) edited the summary of this revision. (Show Details)Jul 28 2018, 6:03 PM
Richard Antalik (ISS) updated this revision to Diff 11475.

Fixed audio glitching when pitch is animated, also sequence position / length is stable now. I was not able to reproduce these 1 frame offsets that I remember seeing last time.

As I looked at this after some time, I must say, that this is an ugly hack so, I agree with you Joerg, this(VSE) has to be rewritten... But in the meantime, this patch can be useful.

Hey, sorry for the slow responses, I'm quite busy at work at the moment. Can you please update the patch? Currently it doesn't apply.

rebase on top of latest master

Hey,

I tried the patch. Animated strips work now, but I still have trouble with strips loaded from a file that was created with vanilla blender.

When this is fixed, you should let Sergey Sharybin and Campbell Barton have a look at this patch. I'd be ok with applying, but only if one of them has a look and approves it too.

Cheers

Hi,

I can "convert" old files, by adjusting offsets when loading.
When I load files blend file saved by blender compiled with this diff, it's much worse - there can be sequences with 0 length
I can "convert" offsets back when saving.

well this is already ugly so why not :)

Well, ideally you would use the offsets as they were used before. But if not, you don't have to convert before storing, since we're not forward compatible, but we have to be backward compatible, that's what do_versions() is for.

I tested it in your custom build and it's a much welcome improvement! How about updating the scale when volume != 1.0? I use .wav audio samples and soundtracks that are normalized by default, and I lower their volume in the VSE. But the waveform still doesn't reflect that. Would that improvement be possible? It'd be much welcome for me ๐Ÿ˜„

My understanding of backward and forward compatibility was kinda reversed - from application point of view, to support newer files.
So if this is not the case, I will consider different approach here

Nathan, Ok I will update this and add scaling up only as an option. My reasoning is in summary - I value visibility more than acuracy in this case.
I was also thinking about applying logarithmic scale, because well, that's how we percieve the sound volume, but my math-fu is poor :)

Richard Antalik (ISS) edited the summary of this revision. (Show Details)Sep 4 2018, 8:38 PM
Richard Antalik (ISS) updated this revision to Diff 11775.

Convert old files to new format - triggered by manipulation with sequence individually.
This is done in BKE_sequence_calc_sound_disp() instead of do_versions()

If some madman decides to apply this to master, I can move conversion, but this patch should be very temporary...