From b2660dbd2b26c4c491edd3745e0d35477b46fff0 Mon Sep 17 00:00:00 2001 From: Richard Antalik Date: Fri, 7 Feb 2025 22:41:18 +0100 Subject: [PATCH] Fix: Improve retiming speed get/set code precision Use float for intermediate values when calculating speed or timeline frames. More details on precision improvement itself: In some cases, setting strip speed to say 70% would result in actual speed being set to say 69.5%. With this change, the number would be closer or equal to 70%. There few tangentially related changes: - `SEQ_retiming_key_speed_get` was changed so both functions work in same time domain. - `SEQ_retiming_key_speed_set` now expects speed as float factor instead of percentage. This is consistent with get function return value. - Improve variable names, for better code readability. Pull Request: https://projects.blender.org/blender/blender/pulls/131782 --- .../space_sequencer/sequencer_retiming.cc | 4 +-- .../sequencer/intern/strip_retiming.cc | 32 ++++++++----------- 2 files changed, 15 insertions(+), 21 deletions(-) diff --git a/source/blender/editors/space_sequencer/sequencer_retiming.cc b/source/blender/editors/space_sequencer/sequencer_retiming.cc index cb5fd642079..1f306035628 100644 --- a/source/blender/editors/space_sequencer/sequencer_retiming.cc +++ b/source/blender/editors/space_sequencer/sequencer_retiming.cc @@ -654,7 +654,7 @@ static int strip_speed_set_exec(bContext *C, const wmOperator *op) continue; } /* TODO: it would be nice to multiply speed with complex retiming by a factor. */ - SEQ_retiming_key_speed_set(scene, strip, key, RNA_float_get(op->ptr, "speed"), false); + SEQ_retiming_key_speed_set(scene, strip, key, RNA_float_get(op->ptr, "speed") / 100.0f, false); ListBase *seqbase = SEQ_active_seqbase_get(SEQ_editing_get(scene)); if (SEQ_transform_test_overlap(scene, seqbase, strip)) { @@ -679,7 +679,7 @@ static int segment_speed_set_exec(const bContext *C, SEQ_retiming_key_speed_set(scene, item.value, item.key, - RNA_float_get(op->ptr, "speed"), + RNA_float_get(op->ptr, "speed") / 100.0f, RNA_boolean_get(op->ptr, "keep_retiming")); if (SEQ_transform_test_overlap(scene, seqbase, item.value)) { diff --git a/source/blender/sequencer/intern/strip_retiming.cc b/source/blender/sequencer/intern/strip_retiming.cc index d070df8bc73..d81a2e7dd86 100644 --- a/source/blender/sequencer/intern/strip_retiming.cc +++ b/source/blender/sequencer/intern/strip_retiming.cc @@ -781,17 +781,12 @@ float SEQ_retiming_key_speed_get(const Strip *strip, const SeqRetimingKey *key) } const SeqRetimingKey *key_prev = key - 1; - const int frame_index_max = strip->len - 1; - const int frame_retimed_prev = round_fl_to_int(key_prev->retiming_factor * frame_index_max); - const int frame_index_prev = key_prev->strip_frame_index; - const int frame_retimed = round_fl_to_int(key->retiming_factor * frame_index_max); - const int frame_index = key->strip_frame_index; - - const int fragment_length_retimed = frame_retimed - frame_retimed_prev; - const int fragment_length_original = frame_index - frame_index_prev; - - const float speed = float(fragment_length_retimed) / float(fragment_length_original); + const float frame_index_start = round_fl_to_int(key_prev->retiming_factor * frame_index_max); + const float frame_index_end = round_fl_to_int(key->retiming_factor * frame_index_max); + const float segment_content_frame_count = frame_index_end - frame_index_start; + const float segment_length = key->strip_frame_index - key_prev->strip_frame_index; + const float speed = segment_content_frame_count / segment_length; return speed; } @@ -803,19 +798,18 @@ void SEQ_retiming_key_speed_set( } const SeqRetimingKey *key_prev = key - 1; - const float speed_fac = 100.0f / speed; const int frame_index_max = strip->len - 1; - const int frame_retimed_prev = round_fl_to_int(key_prev->retiming_factor * frame_index_max); - const int frame_retimed = round_fl_to_int(key->retiming_factor * frame_index_max); + const float frame_index_prev = round_fl_to_int(key_prev->retiming_factor * frame_index_max); + const float frame_index = round_fl_to_int(key->retiming_factor * frame_index_max); - const int segment_duration = (frame_retimed - frame_retimed_prev) / - SEQ_time_media_playback_rate_factor_get(scene, strip); - const int new_duration = segment_duration * speed_fac; + const float segment_timeline_duration = (frame_index - frame_index_prev) / + SEQ_time_media_playback_rate_factor_get(scene, strip); + const float new_timeline_duration = segment_timeline_duration / speed; - const int orig_timeline_frame = SEQ_retiming_key_timeline_frame_get(scene, strip, key); - const int new_timeline_frame = SEQ_retiming_key_timeline_frame_get(scene, strip, key_prev) + - new_duration; + const float orig_timeline_frame = SEQ_retiming_key_timeline_frame_get(scene, strip, key); + const float new_timeline_frame = std::round( + SEQ_retiming_key_timeline_frame_get(scene, strip, key_prev) + new_timeline_duration); SEQ_retiming_key_timeline_frame_set(scene, strip, key, new_timeline_frame);