Page MenuHome

VSE: Only update sound strip length when it is expexted
Needs ReviewPublic

Authored by Richard Antalik (ISS) on Jan 5 2021, 4:53 AM.

Details

Summary

Function sequencer_refresh_sound_length_recursive() can change strip
length, which can cause strips to overlap and break strip position.

This is prevented by ensuring right handle position is fixed.

Strip length should only change when user changes source file manually.
SEQ_sound_update_for_new_sound_file() function was added that will
update strips using specific sound and strip length is changed only if
strip has no offset.

Added docs to destinguish purpose of these functions.

Diff Detail

Repository
rB Blender
Branch
fix-snd-bounds (branched from master)
Build Status
Buildable 11985
Build 11985: arc lint + arc unit

Event Timeline

Richard Antalik (ISS) requested review of this revision.Jan 5 2021, 4:53 AM
Richard Antalik (ISS) created this revision.
Richard Antalik (ISS) planned changes to this revision.Jan 5 2021, 5:44 AM

I am looking into Strip length should only change when user changes source file manually part and it may make more sense to make both changes in one patch.

  • Add function SEQ_sound_update_for_new_sound_file() to update length when changing source manually
Richard Antalik (ISS) retitled this revision from VSE: Don't change sound strip length on update to VSE: Only update sound strip length when it is expexted.Jan 5 2021, 6:36 AM
Richard Antalik (ISS) edited the summary of this revision. (Show Details)

Strip length should only change when user changes source file manually this makes sense to me. Where is it violated currently? How to reproduce the "violation"?

Strip length should only change when user changes source file manually this makes sense to me. Where is it violated currently? How to reproduce the "violation"?

  1. Add sound to VSE
  2. Change file to another one with different length.
    • Strip length is not updated.
  3. Change FPS to same value (from 24 to 24), This will run SEQ_sound_update_length()
    • Strip length will change.

The behavior should be backwards I would say.

I think the actual issue goes somewhat deeper into inconsistency between audio, video, the way how VSE reacts to input and FPS change.

To my knowledge, changing source of video strips will not change their duration. This makes sense from the following point of view: one might replace media with higher/lower resolution, or replace media from PNG to EXR for HDR for final renders. It will be undesirable if changing media will change strip duration. Instead, the strip should maintain its position, cuts and such.

When adding first video into the sequencer it will set scene FPS, so that the video is played back properly. After that changing scene FPS will not change strips. Adding another videos (with possibly different FPS) will not change scene's FPS. Instead, all the changes will cause to perceived speed change in the video. This behavior sounds reasonable to me.

The audio seems to be handled differently. in the current state of VSE: VSE always tries to map audio strip duration so that n o matter which FPS you use in scene the audio speed does not change. This opens all sort of the issues which lead to need of strip length adjustment. And this is something I'm not really sure is a good design decision, as it leads to possible strips intersection and makes it harder to alternate between different versions of audio sample.

@Francesco Siddi (fsiddi), please share your experience w.r.t changing media source and strip length.

  • Fix passing too many arguments
  • Fix passing too many args again......

Not sure what's the state of this patch, but the expectations for framerate adjustment are:

  • if the first media added does not match the current file settings, Blender should offer to adapt (and should not do that quietly as it does now - that's for a separate design task)
  • changing file framerate should not affect the timeline at all. If the media in the timeline does not exactly match the file, it's expected to have a certain loss of quality

Not sure what's the state of this patch, but the expectations for framerate adjustment are:

It should be ready to commit.
I rebased it because user faced this problem - made project at wrong FPS by accident, now he is stuck. So I made a build so he can recover his work.

  • if the first media added does not match the current file settings, Blender should offer to adapt (and should not do that quietly as it does now - that's for a separate design task)
  • changing file framerate should not affect the timeline at all. If the media in the timeline does not exactly match the file, it's expected to have a certain loss of quality

Thanks for clarification, I agree with this approach.