Fix #141084: Sound glitches in render with equalizer

Ultimately, the issue was caused by updating sound sequence handle in
`AUD_SequenceEntry_setSound` on each new frame regardless if the
modifier data has changed.

This commit fixes the issue by implementing a means for modifiers to
check, if their parameters or inputs are changed.
This is done by storing these parameters in `StripModifierDataRuntime`
struct, that is shared between all modifier types. This is not ideal,
but it significantly simplifies dependency graph runtime store/restore
code.
Function `strip_update_sound_modifiers` passes boolean `needs_update`
to strip stack update functions. If any needs to be updated, it sets
value of `needs_update` to true allowing following update functions to
skip parameter checking to speed up the process.

Original code updated sound sequence handle twice. Once by function
`BKE_sound_update_scene_sound` then by `strip_update_sound_modifiers`.
If sound modifier is used, only `strip_update_sound_modifiers` needs to
be called, so there is condition to decide which one of these functions
is called.

Also fixes #139605

Pull Request: https://projects.blender.org/blender/blender/pulls/141595
This commit is contained in:
Richard Antalik
2025-07-18 16:34:09 +02:00
committed by Richard Antalik
parent ef89c75382
commit 0396316fa4
13 changed files with 294 additions and 71 deletions

View File

@@ -27,7 +27,7 @@
/* Blender file format version. */
#define BLENDER_FILE_VERSION BLENDER_VERSION
#define BLENDER_FILE_SUBVERSION 38
#define BLENDER_FILE_SUBVERSION 39
/* Minimum Blender version that supports reading file written with the current
* version. Older Blender versions will test this and cancel loading the file, showing a warning to

View File

@@ -47,6 +47,8 @@
#include "BLO_read_write.hh"
#include "SEQ_iterator.hh"
#include "SEQ_modifier.hh"
#include "SEQ_sequencer.hh"
#include "readfile.hh"
@@ -1499,6 +1501,21 @@ void blo_do_versions_500(FileData * /*fd*/, Library * /*lib*/, Main *bmain)
FOREACH_NODETREE_END;
}
if (!MAIN_VERSION_FILE_ATLEAST(bmain, 500, 39)) {
LISTBASE_FOREACH (Scene *, scene, &bmain->scenes) {
Editing *ed = seq::editing_get(scene);
if (ed != nullptr) {
seq::for_each_callback(&ed->seqbase, [](Strip *strip) -> bool {
LISTBASE_FOREACH (StripModifierData *, smd, &strip->modifiers) {
seq::modifier_persistent_uid_init(*strip, *smd);
}
return true;
});
}
}
}
/**
* Always bump subversion in BKE_blender_version.h when adding versioning
* code here, and wrap it inside a MAIN_VERSION_FILE_ATLEAST check.

View File

@@ -14,6 +14,46 @@
namespace blender::deg {
StripModifierDataBackup::StripModifierDataBackup()
{
reset();
}
void StripModifierDataBackup::reset()
{
sound_in = nullptr;
sound_out = nullptr;
last_buf = nullptr;
}
void StripModifierDataBackup::init_from_modifier(StripModifierData *smd)
{
if (smd->type == seqModifierType_SoundEqualizer) {
sound_in = smd->runtime.last_sound_in;
sound_out = smd->runtime.last_sound_out;
last_buf = smd->runtime.last_buf;
smd->runtime.last_sound_in = nullptr;
smd->runtime.last_sound_out = nullptr;
smd->runtime.last_buf = nullptr;
}
}
void StripModifierDataBackup::restore_to_modifier(StripModifierData *smd)
{
if (smd->type == seqModifierType_SoundEqualizer) {
smd->runtime.last_sound_in = sound_in;
smd->runtime.last_sound_out = sound_out;
smd->runtime.last_buf = last_buf;
}
reset();
}
bool StripModifierDataBackup::isEmpty() const
{
return sound_in == nullptr && sound_out == nullptr && last_buf == nullptr;
}
StripBackup::StripBackup(const Depsgraph * /*depsgraph*/)
{
reset();
@@ -23,6 +63,7 @@ void StripBackup::reset()
{
scene_sound = nullptr;
BLI_listbase_clear(&anims);
modifiers.clear();
}
void StripBackup::init_from_strip(Strip *strip)
@@ -30,6 +71,14 @@ void StripBackup::init_from_strip(Strip *strip)
scene_sound = strip->scene_sound;
anims = strip->anims;
LISTBASE_FOREACH (StripModifierData *, smd, &strip->modifiers) {
StripModifierDataBackup mod_backup;
mod_backup.init_from_modifier(smd);
if (!mod_backup.isEmpty()) {
modifiers.add(smd->persistent_uid, mod_backup);
}
}
strip->scene_sound = nullptr;
BLI_listbase_clear(&strip->anims);
}
@@ -38,12 +87,20 @@ void StripBackup::restore_to_strip(Strip *strip)
{
strip->scene_sound = scene_sound;
strip->anims = anims;
LISTBASE_FOREACH (StripModifierData *, smd, &strip->modifiers) {
std::optional<StripModifierDataBackup> backup = modifiers.pop_try(smd->persistent_uid);
if (backup.has_value()) {
backup->restore_to_modifier(smd);
}
}
reset();
}
bool StripBackup::isEmpty() const
{
return (scene_sound == nullptr) && BLI_listbase_is_empty(&anims);
return (scene_sound == nullptr) && BLI_listbase_is_empty(&anims) && modifiers.is_empty();
}
} // namespace blender::deg

View File

@@ -10,12 +10,31 @@
#include "DNA_listBase.h"
#include "BLI_map.hh"
struct Strip;
struct StripModifierData;
namespace blender::deg {
struct Depsgraph;
class StripModifierDataBackup {
public:
StripModifierDataBackup();
void reset();
void init_from_modifier(StripModifierData *smd);
void restore_to_modifier(StripModifierData *smd);
bool isEmpty() const;
void *sound_in;
void *sound_out;
float *last_buf;
};
/* Backup of a single strip. */
class StripBackup {
public:
@@ -30,6 +49,7 @@ class StripBackup {
void *scene_sound;
ListBase anims;
Map<int, StripModifierDataBackup> modifiers;
};
} // namespace blender::deg

View File

@@ -42,7 +42,8 @@ static wmOperatorStatus strip_modifier_add_exec(bContext *C, wmOperator *op)
Strip *strip = seq::select_active_get(scene);
int type = RNA_enum_get(op->ptr, "type");
seq::modifier_new(strip, nullptr, type);
StripModifierData *smd = seq::modifier_new(strip, nullptr, type);
seq::modifier_persistent_uid_init(*strip, *smd);
seq::relations_invalidate_cache(scene, strip);
WM_event_add_notifier(C, NC_SCENE | ND_SEQUENCER, scene);
@@ -241,52 +242,52 @@ enum {
static wmOperatorStatus strip_modifier_copy_exec(bContext *C, wmOperator *op)
{
Scene *scene = CTX_data_sequencer_scene(C);
Editing *ed = scene->ed;
Strip *strip = seq::select_active_get(scene);
Strip *active_strip = seq::select_active_get(scene);
const int type = RNA_enum_get(op->ptr, "type");
if (!strip || !strip->modifiers.first) {
if (!active_strip || !active_strip->modifiers.first) {
return OPERATOR_CANCELLED;
}
int isSound = ELEM(strip->type, STRIP_TYPE_SOUND_RAM);
int isSound = ELEM(active_strip->type, STRIP_TYPE_SOUND_RAM);
LISTBASE_FOREACH (Strip *, strip_iter, seq::active_seqbase_get(ed)) {
if (strip_iter->flag & SELECT) {
if (strip_iter == strip) {
continue;
}
int strip_iter_is_sound = ELEM(strip_iter->type, STRIP_TYPE_SOUND_RAM);
/* If original is sound, only copy to "sound" strips
* If original is not sound, only copy to "not sound" strips
*/
if (isSound != strip_iter_is_sound) {
continue;
}
VectorSet<Strip *> selected = selected_strips_from_context(C);
selected.remove(active_strip);
if (type == SEQ_MODIFIER_COPY_REPLACE) {
if (strip_iter->modifiers.first) {
StripModifierData *smd_tmp,
*smd = static_cast<StripModifierData *>(strip_iter->modifiers.first);
while (smd) {
smd_tmp = smd->next;
BLI_remlink(&strip_iter->modifiers, smd);
seq::modifier_free(smd);
smd = smd_tmp;
}
BLI_listbase_clear(&strip_iter->modifiers);
for (Strip *strip_iter : selected) {
int strip_iter_is_sound = ELEM(strip_iter->type, STRIP_TYPE_SOUND_RAM);
/* If original is sound, only copy to "sound" strips
* If original is not sound, only copy to "not sound" strips
*/
if (isSound != strip_iter_is_sound) {
continue;
}
if (type == SEQ_MODIFIER_COPY_REPLACE) {
if (strip_iter->modifiers.first) {
StripModifierData *smd_tmp,
*smd = static_cast<StripModifierData *>(strip_iter->modifiers.first);
while (smd) {
smd_tmp = smd->next;
BLI_remlink(&strip_iter->modifiers, smd);
seq::modifier_free(smd);
smd = smd_tmp;
}
BLI_listbase_clear(&strip_iter->modifiers);
}
}
seq::modifier_list_copy(strip_iter, strip);
LISTBASE_FOREACH (StripModifierData *, smd, &active_strip->modifiers) {
StripModifierData *smd_new = seq::modifier_copy(*strip_iter, smd);
seq::modifier_persistent_uid_init(*strip_iter, *smd_new);
}
}
if (ELEM(strip->type, STRIP_TYPE_SOUND_RAM)) {
if (ELEM(active_strip->type, STRIP_TYPE_SOUND_RAM)) {
DEG_id_tag_update(&scene->id, ID_RECALC_SEQUENCER_STRIPS | ID_RECALC_AUDIO);
}
else {
seq::relations_invalidate_cache(scene, strip);
seq::relations_invalidate_cache(scene, active_strip);
}
WM_event_add_notifier(C, NC_SCENE | ND_SEQUENCER, scene);

View File

@@ -513,6 +513,18 @@ typedef struct ColorMixVars {
/** \name Strip Modifiers
* \{ */
typedef struct StripModifierDataRuntime {
/* Reference parameters for optimizing updates. Sound modifiers can store parameters, sound
* inputs and outputs. When all existing parameters do match new ones, the update can be skipped
* and old sound handle may be returned. This is to prevent audio glitches, see #141595 */
float *last_buf; /* Equalizer frequency/volume curve buffer */
/* Reference sound handles (may be used by any sound modifier). */
void *last_sound_in;
void *last_sound_out;
} StripModifierDataRuntime;
typedef struct StripModifierData {
struct StripModifierData *next, *prev;
int type, flag;
@@ -524,6 +536,11 @@ typedef struct StripModifierData {
struct Strip *mask_strip;
struct Mask *mask_id;
int persistent_uid;
char _pad[4];
StripModifierDataRuntime runtime;
} StripModifierData;
typedef struct ColorBalanceModifierData {

View File

@@ -1495,6 +1495,7 @@ static StripModifierData *rna_Strip_modifier_new(
StripModifierData *smd;
smd = blender::seq::modifier_new(strip, name, type);
blender::seq::modifier_persistent_uid_init(*strip, *smd);
blender::seq::relations_invalidate_cache(scene, strip);

View File

@@ -58,10 +58,12 @@ void modifier_apply_stack(const RenderData *context,
const Strip *strip,
ImBuf *ibuf,
int timeline_frame);
StripModifierData *modifier_copy(Strip &strip_dst, StripModifierData *mod_src);
void modifier_list_copy(Strip *strip_new, Strip *strip);
int sequence_supports_modifiers(Strip *strip);
void modifier_blend_write(BlendWriter *writer, ListBase *modbase);
void modifier_blend_read_data(BlendDataReader *reader, ListBase *lb);
void modifier_persistent_uid_init(const Strip &strip, StripModifierData &smd);
} // namespace blender::seq

View File

@@ -23,7 +23,7 @@ namespace blender::seq {
struct SoundModifierWorkerInfo {
int type;
void *(*recreator)(Strip *strip, StripModifierData *smd, void *sound);
void *(*recreator)(Strip *strip, StripModifierData *smd, void *sound, bool &needs_update);
};
#define SOUND_EQUALIZER_DEFAULT_MIN_FREQ 30.0
@@ -41,12 +41,18 @@ EQCurveMappingData *sound_equalizer_add(SoundEqualizerModifierData *semd, float
void sound_blend_write(BlendWriter *writer, ListBase *soundbase);
void sound_blend_read_data(BlendDataReader *reader, ListBase *lb);
void *sound_modifier_recreator(Strip *strip, StripModifierData *smd, void *sound);
void *sound_modifier_recreator(Strip *strip,
StripModifierData *smd,
void *sound,
bool &needs_update);
void sound_equalizermodifier_init_data(StripModifierData *smd);
void sound_equalizermodifier_free(StripModifierData *smd);
void sound_equalizermodifier_copy_data(StripModifierData *target, StripModifierData *smd);
void *sound_equalizermodifier_recreator(Strip *strip, StripModifierData *smd, void *sound);
void *sound_equalizermodifier_recreator(Strip *strip,
StripModifierData *smd,
void *sound,
bool &needs_update);
void sound_equalizermodifier_set_graphs(SoundEqualizerModifierData *semd, int number);
const SoundModifierWorkerInfo *sound_modifier_worker_info_get(int type);
EQCurveMappingData *sound_equalizermodifier_add_graph(SoundEqualizerModifierData *semd,

View File

@@ -11,9 +11,12 @@
#include <cstring>
#include "BLI_array.hh"
#include "BLI_hash.hh"
#include "BLI_listbase.h"
#include "BLI_math_geom.h"
#include "BLI_math_vector.hh"
#include "BLI_rand.hh"
#include "BLI_set.hh"
#include "BLI_string.h"
#include "BLI_string_utils.hh"
#include "BLI_task.hh"
@@ -37,12 +40,57 @@
#include "BLO_read_write.hh"
#include "modifier.hh"
#include "render.hh"
namespace blender::seq {
/* -------------------------------------------------------------------- */
static bool modifier_has_persistent_uid(const Strip &strip, int uid)
{
LISTBASE_FOREACH (StripModifierData *, smd, &strip.modifiers) {
if (smd->persistent_uid == uid) {
return true;
}
}
return false;
}
void modifier_persistent_uid_init(const Strip &strip, StripModifierData &smd)
{
uint64_t hash = blender::get_default_hash(blender::StringRef(smd.name));
blender::RandomNumberGenerator rng{uint32_t(hash)};
while (true) {
const int new_uid = rng.get_int32();
if (new_uid <= 0) {
continue;
}
if (modifier_has_persistent_uid(strip, new_uid)) {
continue;
}
smd.persistent_uid = new_uid;
break;
}
}
bool modifier_persistent_uids_are_valid(const Strip &strip)
{
Set<int> uids;
int modifiers_num = 0;
LISTBASE_FOREACH (StripModifierData *, smd, &strip.modifiers) {
if (smd->persistent_uid <= 0) {
return false;
}
uids.add(smd->persistent_uid);
modifiers_num++;
}
if (uids.size() != modifiers_num) {
return false;
}
return true;
}
static float4 load_pixel_premul(const uchar *ptr)
{
float4 res;
@@ -1341,25 +1389,29 @@ void modifier_apply_stack(const RenderData *context,
}
}
StripModifierData *modifier_copy(Strip &strip_dst, StripModifierData *mod_src)
{
const StripModifierTypeInfo *smti = modifier_type_info_get(mod_src->type);
StripModifierData *mod_new = static_cast<StripModifierData *>(MEM_dupallocN(mod_src));
if (smti && smti->copy_data) {
smti->copy_data(mod_new, mod_src);
}
BLI_addtail(&strip_dst.modifiers, mod_new);
BLI_uniquename(&strip_dst.modifiers,
mod_new,
"Strip Modifier",
'.',
offsetof(StripModifierData, name),
sizeof(StripModifierData::name));
return mod_new;
}
void modifier_list_copy(Strip *strip_new, Strip *strip)
{
LISTBASE_FOREACH (StripModifierData *, smd, &strip->modifiers) {
StripModifierData *smdn;
const StripModifierTypeInfo *smti = modifier_type_info_get(smd->type);
smdn = static_cast<StripModifierData *>(MEM_dupallocN(smd));
if (smti && smti->copy_data) {
smti->copy_data(smdn, smd);
}
BLI_addtail(&strip_new->modifiers, smdn);
BLI_uniquename(&strip_new->modifiers,
smdn,
"Strip Modifier",
'.',
offsetof(StripModifierData, name),
sizeof(StripModifierData::name));
modifier_copy(*strip_new, smd);
}
}

View File

@@ -0,0 +1,17 @@
/* SPDX-FileCopyrightText: 2025 Blender Authors
*
* SPDX-License-Identifier: GPL-2.0-or-later */
#pragma once
/** \file
* \ingroup sequencer
*/
struct Strip;
namespace blender::seq {
bool modifier_persistent_uids_are_valid(const Strip &strip);
} // namespace blender::seq

View File

@@ -59,6 +59,7 @@
#include "cache/final_image_cache.hh"
#include "cache/intra_frame_cache.hh"
#include "cache/source_image_cache.hh"
#include "modifier.hh"
#include "prefetch.hh"
#include "sequencer.hh"
#include "utils.hh"
@@ -548,6 +549,7 @@ static Strip *strip_duplicate(const Scene *scene_src,
modifier_list_copy(strip_new, strip);
}
BLI_assert(modifier_persistent_uids_are_valid(*strip));
if (is_strip_connected(strip)) {
BLI_listbase_clear(&strip_new->connections);
@@ -1026,14 +1028,16 @@ static void strip_update_sound_properties(const Scene *scene, const Strip *strip
static void strip_update_sound_modifiers(Strip *strip)
{
void *sound_handle = strip->sound->playback_handle;
if (!BLI_listbase_is_empty(&strip->modifiers)) {
LISTBASE_FOREACH (StripModifierData *, smd, &strip->modifiers) {
sound_handle = sound_modifier_recreator(strip, smd, sound_handle);
}
bool needs_update = false;
LISTBASE_FOREACH (StripModifierData *, smd, &strip->modifiers) {
sound_handle = sound_modifier_recreator(strip, smd, sound_handle, needs_update);
}
/* Assign modified sound back to `strip`. */
BKE_sound_update_sequence_handle(strip->scene_sound, sound_handle);
if (needs_update) {
/* Assign modified sound back to `strip`. */
BKE_sound_update_sequence_handle(strip->scene_sound, sound_handle);
}
}
static bool must_update_strip_sound(Scene *scene, Strip *strip)
@@ -1047,9 +1051,16 @@ static void seq_update_sound_strips(Scene *scene, Strip *strip)
if (strip->sound == nullptr || !must_update_strip_sound(scene, strip)) {
return;
}
/* Ensure strip is playing correct sound. */
BKE_sound_update_scene_sound(strip->scene_sound, strip->sound);
strip_update_sound_modifiers(strip);
if (BLI_listbase_is_empty(&strip->modifiers)) {
/* Just use playback handle from sound ID. */
BKE_sound_update_scene_sound(strip->scene_sound, strip->sound);
}
else {
/* Use Playback handle from sound ID as input for modifier stack. */
strip_update_sound_modifiers(strip);
}
}
static bool scene_sequencer_is_used(const Scene *scene, ListBase *seqbase)

View File

@@ -245,6 +245,9 @@ void sound_equalizermodifier_free(StripModifierData *smd)
MEM_freeN(eqcmd);
}
BLI_listbase_clear(&semd->graphics);
if (smd->runtime.last_buf) {
MEM_freeN(smd->runtime.last_buf);
}
}
void sound_equalizermodifier_copy_data(StripModifierData *target, StripModifierData *smd)
@@ -264,7 +267,10 @@ void sound_equalizermodifier_copy_data(StripModifierData *target, StripModifierD
}
}
void *sound_equalizermodifier_recreator(Strip *strip, StripModifierData *smd, void *sound)
void *sound_equalizermodifier_recreator(Strip *strip,
StripModifierData *smd,
void *sound_in,
bool &needs_update)
{
#ifdef WITH_CONVOLUTION
UNUSED_VARS(strip);
@@ -273,7 +279,7 @@ void *sound_equalizermodifier_recreator(Strip *strip, StripModifierData *smd, vo
/* No equalizer definition. */
if (BLI_listbase_is_empty(&semd->graphics)) {
return sound;
return sound_in;
}
float *buf = MEM_calloc_arrayN<float>(SOUND_EQUALIZER_SIZE_DEFINITION, "eqrecreator");
@@ -311,15 +317,28 @@ void *sound_equalizermodifier_recreator(Strip *strip, StripModifierData *smd, vo
}
}
AUD_Sound *equ = AUD_Sound_equalize(sound,
buf,
SOUND_EQUALIZER_SIZE_DEFINITION,
SOUND_EQUALIZER_DEFAULT_MAX_FREQ,
SOUND_EQUALIZER_SIZE_CONVERSION);
/* Only make new sound when necessary. It is faster and it prevents audio glitches. */
if (!needs_update && smd->runtime.last_sound_in == sound_in &&
smd->runtime.last_buf != nullptr &&
std::memcmp(buf, smd->runtime.last_buf, SOUND_EQUALIZER_SIZE_DEFINITION) == 0)
{
MEM_freeN(buf);
return smd->runtime.last_sound_out;
}
MEM_freeN(buf);
AUD_Sound *sound_out = AUD_Sound_equalize(sound_in,
buf,
SOUND_EQUALIZER_SIZE_DEFINITION,
SOUND_EQUALIZER_DEFAULT_MAX_FREQ,
SOUND_EQUALIZER_SIZE_CONVERSION);
return equ;
needs_update = true;
smd->runtime.last_buf = buf;
smd->runtime.last_sound_in = sound_in;
smd->runtime.last_sound_out = sound_out;
return sound_out;
#else
UNUSED_VARS(strip, smd, sound);
return nullptr;
@@ -336,12 +355,15 @@ const SoundModifierWorkerInfo *sound_modifier_worker_info_get(int type)
return nullptr;
}
void *sound_modifier_recreator(Strip *strip, StripModifierData *smd, void *sound)
void *sound_modifier_recreator(Strip *strip,
StripModifierData *smd,
void *sound,
bool &needs_update)
{
if (!(smd->flag & SEQUENCE_MODIFIER_MUTE)) {
const SoundModifierWorkerInfo *smwi = sound_modifier_worker_info_get(smd->type);
return smwi->recreator(strip, smd, sound);
return smwi->recreator(strip, smd, sound, needs_update);
}
return sound;
}