Page MenuHome

Fix T75686 Animating scene audio volume doesn't work
ClosedPublic

Authored by Sybren A. Stüvel (sybren) on Apr 14 2020, 7:23 PM.

Details

Summary

Scene audio volume changes require the scene to be tagged with DEG_id_tag_update(&scene->id, ID_RECALC_AUDIO_VOLUME) (see BKE_scene_update_sound()). Tagging happens in the RNA update function rna_Scene_volume_update(), but that function is not called by the animation system. As a result, animated volume changes are not sent to the audio system.

To resolve this, I've added a depsgraph operation node, which a callback that sets this tag. It depends on the animation evaluation node when the volume is actually animated, as per the AUDIO_VOLUME_ANIMATED flag.

Diff Detail

Repository
rB Blender

Event Timeline

Sybren A. Stüvel (sybren) requested review of this revision.Apr 14 2020, 7:23 PM
Sybren A. Stüvel (sybren) created this revision.
Sergey Sharybin (sergey) requested changes to this revision.Apr 15 2020, 9:39 AM

Animation system must not be using update(). No tagging is supposed to happen from set() callback.
Doing dependency graph tags during evaluation must not be happening (or at a very least, you must not design area which relies on the fact that those recalc flags will be handled at all).

source/blender/makesrna/intern/rna_scene.c
1954–1956 ↗(On Diff #23737)

set functions must not do any sort of tagging.

This revision now requires changes to proceed.Apr 15 2020, 9:39 AM

@Sergey Sharybin (sergey) the "Audio Mute" checkbox can't be animated through the GUI at the moment, but an FCurve can still be created for the RNA path via Python. We haven't had a bug report about this, so it's just me doing an extra bit of work to make the volume & mute both animatable. If you're okay with the volume part of this patch, I can commit just that and close T75686.

Please move discussion to a single place, so I don't need to explain things twice, or put different parts of information to different parts of developer.b.o.

If you're okay with the volume part of this patch,

I am not okay. The solution is wrong. So I suggest moving discussion to the actual task. Especially since I've already written explanation there.

  • Added a new OperationNode for setting the audio volume.
  • Removed tagging for update in rna_Scene_use_audio_set because it breaks the law.
  • Removed formatting change
Sergey Sharybin (sergey) requested changes to this revision.Apr 17 2020, 4:00 PM
Sergey Sharybin (sergey) added inline comments.
source/blender/blenkernel/intern/scene.c
1286

You can't be using DEG_id_tag_update during evaluation.
Write the recalc flag directly: scene->id.recalc |= ID_RECALC_AUDIO_VOLUME.

This revision now requires changes to proceed.Apr 17 2020, 4:00 PM
Sybren A. Stüvel (sybren) edited the summary of this revision. (Show Details)
  • Update with scene->id.recalc |= ID_RECALC_AUDIO_VOLUME directly.

This should do it.
Assuming you've tested and it works in practice :)

This revision is now accepted and ready to land.Apr 17 2020, 4:14 PM

Assuming you've tested and it works in practice :)

I have, and it does! The reporter of T75686 was kind enough to include the music from Tears of Steel in the test file, so it was even pleasant to work on :)