VSE: Add sound strip retiming support #105072
No reviewers
Labels
No Label
Interest
Alembic
Interest
Animation & Rigging
Interest
Asset Browser
Interest
Asset Browser Project Overview
Interest
Audio
Interest
Automated Testing
Interest
Blender Asset Bundle
Interest
BlendFile
Interest
Collada
Interest
Compatibility
Interest
Compositing
Interest
Core
Interest
Cycles
Interest
Dependency Graph
Interest
Development Management
Interest
EEVEE
Interest
EEVEE & Viewport
Interest
Freestyle
Interest
Geometry Nodes
Interest
Grease Pencil
Interest
ID Management
Interest
Images & Movies
Interest
Import Export
Interest
Line Art
Interest
Masking
Interest
Metal
Interest
Modeling
Interest
Modifiers
Interest
Motion Tracking
Interest
Nodes & Physics
Interest
OpenGL
Interest
Overlay
Interest
Overrides
Interest
Performance
Interest
Physics
Interest
Pipeline, Assets & IO
Interest
Platforms, Builds & Tests
Interest
Python API
Interest
Render & Cycles
Interest
Render Pipeline
Interest
Sculpt, Paint & Texture
Interest
Text Editor
Interest
Translations
Interest
Triaging
Interest
Undo
Interest
USD
Interest
User Interface
Interest
UV Editing
Interest
VFX & Video
Interest
Video Sequencer
Interest
Virtual Reality
Interest
Vulkan
Interest
Wayland
Interest
Workbench
Interest: X11
Legacy
Blender 2.8 Project
Legacy
Milestone 1: Basic, Local Asset Browser
Legacy
OpenGL Error
Meta
Good First Issue
Meta
Papercut
Meta
Retrospective
Meta
Security
Module
Animation & Rigging
Module
Core
Module
Development Management
Module
EEVEE & Viewport
Module
Grease Pencil
Module
Modeling
Module
Nodes & Physics
Module
Pipeline, Assets & IO
Module
Platforms, Builds & Tests
Module
Python API
Module
Render & Cycles
Module
Sculpt, Paint & Texture
Module
Triaging
Module
User Interface
Module
VFX & Video
Platform
FreeBSD
Platform
Linux
Platform
macOS
Platform
Windows
Priority
High
Priority
Low
Priority
Normal
Priority
Unbreak Now!
Status
Archived
Status
Confirmed
Status
Duplicate
Status
Needs Info from Developers
Status
Needs Information from User
Status
Needs Triage
Status
Resolved
Type
Bug
Type
Design
Type
Known Issue
Type
Patch
Type
Report
Type
To Do
No Milestone
No project
No Assignees
4 Participants
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: blender/blender#105072
Loading…
Reference in New Issue
No description provided.
Delete Branch "iss/blender:retiming-sound-2"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
This patch contains changes needed for retiming sound strips.
BKE_sound_set_scene_sound_pitch()
is replaced byBKE_sound_set_scene_sound_pitch_constant_range()
which uses newAudaspace interface to set pitch in bulk.
This is done in
SEQ_retiming_sound_animation_data_set()
where retimedsections are created for each strip. When strip is inside of meta
strip(s), the retimed sections of meta and actual strip are split where
they intersect and pitch is multiplied where they overlap. Each section
will have pitch value that is provided to audaspace.
Waveform overlay now represents retimed audio accurately.
Ref: #100337
Versioning for animated pitch is not present, because resulting data is unusable (whole animation is baked, so changing retiming is basically impossible). It would also result in precision issues which results in non uniform sound pitch in some cases.
Changes in audaspace are meged to upstram.
@blender-bot package
Package build started. Download here when ready.
Hey, sorry for taking some time to review this, I've been quite busy recently. I have looked over the code changes now, primarily in audaspace and added corresponding comments. I also briefly looked over the Blender changes, which look fine to the extent that I can judge them, maybe @Sergey wants to take a closer look there.
@ -167,1 +167,4 @@
AUD_API void AUD_SequenceEntry_setAnimationData_constant_range(AUD_SequenceEntry* entry, AUD_AnimateablePropertyType type, int frame_start, int frame_end, float* data)
{
AnimateableProperty* prop = (*entry)->getAnimProperty(static_cast<AnimateablePropertyType>(type));
Audaspace has its own coding style as you surely noticed, since you're following most of it. Please make sure that indentation happens with tabs everywhere. I just recently introduced a clang-format file in upstream that you can find here: https://github.com/audaspace/audaspace/blob/master/.clang-format. Please only apply it to the changes though, since not all of audaspace is following it perfectly and I wouldn't want to make this patch verbose because of it (see the clang-format-diff script or https://offlinemark.com/2021/04/02/surgical-formatting-with-git-clang-format/ for example).
While we're at it: I would appreciate if you also create a pull request for the audaspace changes upstream (https://github.com/audaspace/audaspace/pulls)!
@ -115,0 +118,4 @@
* \param position_start The start position in the animation in frames.
* \param position_end The end position in the animation in frames.
*/
void write_range(const float* data, int position_start, int position_end);
Please rename
write_range
similarly to the other API functions that call it, so that its also clear from the name that it writes the same data into the range and doesn't get the full data for the range in the parameters. I.e. it repeats the data found in the parameter rather than expecting a bigger block of data that may differ all the way from start to end.@ -64,2 +64,4 @@
double m_skip;
/// The FPS of the scene.
float m_fps;
Likely a bug: Rather than having the fps duplicated here, please add a
std::shared_ptr<SequenceData>
and get the fps from there. This way, it will be updated whenSequence::setFPS
is called which happens when the FPS in blender are changed!@ -245,3 +244,1 @@
if(seekpos < 0)
seekpos = 0;
seekpos += m_entry->m_skip;
float seek_frame = (position - m_entry->m_begin) * m_entry->m_fps;
Is there a specific reason you are using floats here (
seek_frame
andtarget_frame
) now rather than doubles (as was used forseekpos
before)? I think it's still critical for accuracy of long sequences (e.g. full streams of movies at something like 90 minutes).@ -248,0 +246,4 @@
seek_frame = 0;
seek_frame += m_entry->m_skip * m_entry->m_fps;
AnimateableProperty* pitch_property = m_entry->getAnimProperty(AP_PITCH);
Just a note: since this only considers the
AP_PITCH
property and not the doppler effect happening with 3D audio, this will only seek correctly for pitch animation not for doppler animated sounds. But that's ok, considering the 3D audio case is even more complicated and expensive to compute. So in Blender VSE sequences work better then, but 3D speakers don't.@ -248,0 +251,4 @@
float target_frame = 0;
if(pitch_property != nullptr)
{
for(int i = 0; i < seek_frame; i++)
Since this is just an integer loop, the fractal part of
seek_frame
is not considered. For more accurate seeking, I recommend to still consider it though.@neXyon I have marked all issues as resolved, not sure if my inline responses were submitted - seems they are not, so will ask here:
Shall we continue with review here and then I submit PR to audaspace repo?
You wanted to rename
write_range
, but wasn't very specific, so I assumed you meantwrite_constant_range
.I've added two name change suggestions, just fixing their styling.
I think the audaspace changes are ready, so there won't be much to continue reviewing, when you create a PR upstream :)
@ -71,0 +72,4 @@
* \param frame_end End of the frame range.
* \param data The data to write.
*/
AUD_API void AUD_SequenceEntry_setAnimationData_constant_range(AUD_SequenceEntry* entry, AUD_AnimateablePropertyType type, int frame_start, int frame_end, float* data);
AUD_SequenceEntry_setConstantRangeAnimationData
@ -115,0 +118,4 @@
* \param position_start The start position in the animation in frames.
* \param position_end The end position in the animation in frames.
*/
void write_constant_range(const float* data, int position_start, int position_end);
writeConstantRange
Patch seems nice and small :)
Some cosmetic feedback for now.
@ -248,0 +283,4 @@
class RetimingRangeData {
public:
std::vector<RetimingRange> ranges;
Any specific reason to use
std::vector
instead ofblender::Vector
?Unless there is really goo reason (like, interfacing with an external library) we'd better be consistently using our own primitives.
@ -248,0 +303,4 @@
RetimingRangeData &operator*=(const RetimingRangeData rhs)
{
if (ranges.size() == 0) {
if (!ranges.empty())
(is_empty
for blender C++ primitives).Also, consider early output, to unindent the not-so-trivial-logic below.
@blender-bot package
Package build started. Download here when ready.
Something seems odd in the current buildbot build:
@blender-bot package
Package build started. Download here when ready.
Oops, forgot to include this fix. I assume I fixed both issues, but if not please clarify how to reproduce first one.
Not sure what's going on with those failed regression tests in the build, probably just branch needs to be updated to the latest main state.
From my testing the retiming works as expected now.
@blender-bot build