Page MenuHome

Fix T78835: Ghosting audio after using undo
ClosedPublic

Authored by Sergey Sharybin (sergey) on Jul 30 2020, 3:47 PM.

Details

Summary

The root of the issue comes to the fact that sub-data pointers were
used to match strips before/after copy-on-write. The undo system might
re-use sub-data pointers after re-allocating them, making it so that,
for example, pointer used by sound strip is later re-used by video
strip.

This fix takes an advantage of recently introduced per-sequence UUID
and uses it to match sequences before/after copy-on-write.

The code is committed to the temp-T78835 branch, to make it easier
to apply and test.

After this part is done the same UUID approach will be used for the
modifiers and pose channels. The ID UUID can also be switched to use
the new code (is stronger typed, uses more bits, and is good to use
same primitives for everything).

Diff Detail

Repository
rB Blender
Branch
temp-T78835 (branched from master)
Build Status
Buildable 9270
Build 9270: arc lint + arc unit

Event Timeline

Sergey Sharybin (sergey) requested review of this revision.Jul 30 2020, 3:47 PM
Sergey Sharybin (sergey) created this revision.
Brecht Van Lommel (brecht) requested changes to this revision.Jul 30 2020, 4:29 PM
Brecht Van Lommel (brecht) added inline comments.
source/blender/blenkernel/BKE_lib_id.h
130–142

LIB_ID_CREATE_NO_MAIN is used in other places to detect depsgraph copies. I don't think we need another flag?

The comment for that flag could be updated to explain it's not only for localize, but also depsgraph copies.

source/blender/blenlib/BLI_session_uuid.h
33

CHeck -> Check

source/blender/blenloader/intern/readfile.c
6517

I would move this earlier, at least before functions BKE_sequencer_proxy_set, in case future BKE_sequencer function use the UUID.

This revision now requires changes to proceed.Jul 30 2020, 4:29 PM
source/blender/blenkernel/BKE_lib_id.h
130–142

From the commit message: Depsgraph: Use explicit flag that ID is to copied for copy-on-write

I do not like the implicit semantic meaning of LIB_ID_CREATE_NO_MAIN. To me it seems that we should move away from check LIB_ID_CREATE_NO_MAIN and use LIB_ID_COPY_ON_WRITE instead. This could happen separately (as in, separate cleanup, or discussion) though.

I ended up to need LIB_ID_COPY_KEEP_SESSION_UUID for the clipboard, so for now i can ditch LIB_ID_COPY_ON_WRITE out and pass LIB_ID_COPY_KEEP_SESSION_UUID via flags from deg_eval_copy_on_write.cc. Is this something you'd be happy with?

  • Cleanup: Spelling in comment
  • Make sure sequence's UUID is always initialized on read
  • Remove recently introduced explicit flags
  • Cleanup: Warning in release builds
This revision is now accepted and ready to land.Jul 30 2020, 5:05 PM