Page MenuHome

Fix T70845: Crash when switching scene of scene strip.
ClosedPublic

Authored by Richard Antalik (ISS) on Oct 21 2019, 1:15 AM.

Details

Summary

Crash on assert in sound_verify_evaluated_id()

Use DEG_relations_tag_update() to re-evaluate sounds

Diff Detail

Repository
rB Blender

Event Timeline

Sergey Sharybin (sergey) requested changes to this revision.Oct 21 2019, 10:32 AM
Sergey Sharybin (sergey) added inline comments.
source/blender/makesrna/intern/rna_sequencer.c
166

How's ID_RECALC_SEQUENCER_STRIPS relevant here?
This comment states something which is not clear from the context, it should give some more context information to be useful.

Tagging is cheap, so you can as well do ID_RECALC_AUDIO | ID_RECALC_SEQUENCER_STRIPS to avoid relying on a specifics of how rna_Sequence_invalidate_raw_update works.

167

This tag is to be done if old scene also was using audio.
Simpler and more reliable is to just to always tag for ID_RECALC_AUDIO | ID_RECALC_SEQUENCER_STRIPS.

170–171

Relations update doesn't necessarily evaluate anything, making the comment irrelevant/wrong.

The proper reasoning here is that the scene which used to be used by the strip is to be removed from the depsgraph and the new one is to be added. But stating such things in all usages of DEG_relations_tag_update()will be more of unnecessary over-documentation.

2237

The convention is to use _update suffix for update callbacks. Would be something like: rna_Sequence_strip_scene_update

This revision now requires changes to proceed.Oct 21 2019, 10:32 AM
Richard Antalik (ISS) marked 4 inline comments as done.
  • Address points from review.
source/blender/makesrna/intern/rna_sequencer.c
166

My reasoning was, that sure, tagging is cheap, but it can trigger recalc, which is not. So I wanted to tag ID_RECALC_AUDIO only when I expect audio to be actually used(reproduced)

Perhaps it's better to tag ID_RECALC_AUDIO | ID_RECALC_SEQUENCER_STRIPS in this case, as this func is run only when switching scenes.

170–171

I left this comment here to validate my assumption.

So if I understand you correctly, does it mean, that we should call DEG_relations_tag_update() each time any ID datablock usage has been changed?

This revision is now accepted and ready to land.Oct 22 2019, 2:16 PM