Page MenuHome
Differential D7595 Diff 24433 source/blender/blenkernel/intern/sequencer.c

Changeset View

Standalone View

View Options

source/blender/blenkernel/intern/sequencer.c

Show First 20 Lines • Show All 241 Lines • ▼ Show 20 Linesstatic void BKE_sequence_free_ex(Scene *scene,
if (seq->strip) { if (seq->strip) {
seq_free_strip(seq->strip); seq_free_strip(seq->strip);
} }
BKE_sequence_free_anim(seq); BKE_sequence_free_anim(seq);
if (seq->type & SEQ_TYPE_EFFECT) { if (seq->type & SEQ_TYPE_EFFECT) {
struct SeqEffectHandle sh = BKE_sequence_get_effect(seq); struct SeqEffectHandle sh = BKE_sequence_get_effect(seq);
bool do_user = do_id_user;
/* No ID is "responsible" for BLF font cleanup, so we need to do it here. */
if (scene && seq->type == SEQ_TYPE_TEXT && (scene->id.tag & LIB_TAG_NO_USER_REFCOUNT) == 0) {
BKE_sequencer_blf_font_unload(seq->effectdata);
}
sh.free(seq, do_id_user); sh.free(seq, do_id_user);
} }
sergeyUnsubmitted
Done
Inline Actions

It is very weird to override do_user behavior here.

If it is to prevent CoW versions of strips from freeing vfont it can happen more explicitly by storing an ownership flag which will indicate whether strip shares the font with someone else or whether it owns it.

sergey: It is very weird to override `do_user` behavior here. If it is to prevent CoW versions of…
ISSAuthorUnsubmitted
Done
Inline Actions

I don't have problem with VFont - that is fine to handle.
Problem is FontBLF, because you have to manage it's usercount, and also it is global, so if you don't do it correctly, they will be unremovable.

The issue here is, that I can't tell what is happening here. Is this CoW update / undo / loading new .blend file?
Only in case of CoW update I shouldn't modify usercount. Ideally I would say you should be able to check for these events?

I should double check this code because there BKE_sequencer_cache_destruct(scene); always runs before this.
But my point stands mostly I think.

ISS: I don't have problem with VFont - that is fine to handle. Problem is FontBLF, because you have…
brechtUnsubmitted
Done
Inline Actions

I don't understand why ID user counting is involved here.

What I would expect is that every sequence strip that has a text_blf_id holds a reference. If it is copied the reference count is increased. If it is freed the reference count is decreased. It shouldn't matter if that sequence strip is original, CoW or anything else?

brecht: I don't understand why ID user counting is involved here. What I would expect is that every…
ISSAuthorUnsubmitted
Done
Inline Actions

I will increase usercount of FontBLF only if it is already loaded when copying strip and remove CoW exception.

ISS: I will increase usercount of `FontBLF` only if it is already loaded when copying strip and…
if (seq->sound && do_id_user) { if (seq->sound && do_id_user) {
id_us_min(((ID *)seq->sound)); id_us_min(((ID *)seq->sound));
} }
if (seq->stereo3d_format) { if (seq->stereo3d_format) {
MEM_freeN(seq->stereo3d_format); MEM_freeN(seq->stereo3d_format);
} }
▲ Show 20 Lines • Show All 242 Lines • ▼ Show 20 Linesvoid BKE_sequencer_editing_free(Scene *scene, const bool do_id_user)
BKE_sequencer_prefetch_free(scene); BKE_sequencer_prefetch_free(scene);
BKE_sequencer_cache_destruct(scene); BKE_sequencer_cache_destruct(scene);
SEQ_BEGIN (ed, seq) { SEQ_BEGIN (ed, seq) {
/* handle cache freeing above */ /* handle cache freeing above */
BKE_sequence_free_ex(scene, seq, false, do_id_user, false); BKE_sequence_free_ex(scene, seq, false, do_id_user, false);
} }
SEQ_END; SEQ_END;
sergeyUnsubmitted
Done
Inline Actions

Unrelated.

sergey: Unrelated.
BLI_freelistN(&ed->metastack); BLI_freelistN(&ed->metastack);
MEM_freeN(ed); MEM_freeN(ed);
scene->ed = NULL; scene->ed = NULL;
} }
/*********************** Sequencer color space functions *************************/ /*********************** Sequencer color space functions *************************/
▲ Show 20 Lines • Show All 5,179 Lines • ▼ Show 20 Lines if ((flag & LIB_ID_CREATE_NO_USER_REFCOUNT) == 0) {
id_us_plus((ID *)seqn->sound); id_us_plus((ID *)seqn->sound);
} }
} }
else if (seq->type == SEQ_TYPE_IMAGE) { else if (seq->type == SEQ_TYPE_IMAGE) {
seqn->strip->stripdata = MEM_dupallocN(seq->strip->stripdata); seqn->strip->stripdata = MEM_dupallocN(seq->strip->stripdata);
} }
else if (seq->type & SEQ_TYPE_EFFECT) { else if (seq->type & SEQ_TYPE_EFFECT) {
struct SeqEffectHandle sh; struct SeqEffectHandle sh;
/* BKE_sequence_get_effect may change state of seq->flag, which is already copied. As a result
* effect may be initialized twice. So copy seq->flag once again here. */
sh = BKE_sequence_get_effect(seq); sh = BKE_sequence_get_effect(seq);
seqn->flag = seq->flag;
if (sh.copy) { if (sh.copy) {
sh.copy(seq, seqn, flag); sh.copy(seqn, seq, flag);
} }
seqn->strip->stripdata = NULL; seqn->strip->stripdata = NULL;
} }
else { else {
/* sequence type not handled in duplicate! Expect a crash now... */ /* sequence type not handled in duplicate! Expect a crash now... */
BLI_assert(0); BLI_assert(0);
} }
▲ Show 20 Lines • Show All 247 Lines • Show Last 20 Lines