modules/mixer: update T tap once in prepare_frame

It's problematic/racy to always be dereferencing the tap pointer
to access it's current value.

Running checkers:fill_module=mixer on a 2c/4t (n_cpus=4) machine
was quite effective at crashing in mixer::render_fragment()
due to dereferencing a NULL snapshot fragment.

The "tween" T value is being used to indicate when interpolation
of the snapshots is necessary.  But repeatedly re-reading the T
value via the tap pointer would race with the driving tap.  In
the case of checkers, which is a threaded module, the mixer
contexts are all rendering in parallel at the same path so
they're sharing a single pipe for their T taps.

This situation of sets of contexts sharing a single path,
resulting in their taps all landing on a single pipe, is still
largely up in the air and might be actively prevented in the
future.  But until then, it can be made far less crashy and
problematic by just being more careful about dereferencing the
tap to access its latest value just once at the prepare frame
stage, storing it in the local T variable in the context.  Then
all the render_fragment() accesses can at least find a stable
value in the context from prepare-to-render, so at least the
snapshots are there when they should be according to the T value
etc.

Something like this will probably need to be done regardless of
what happens with the context sets sharing the same path.  Since
when a given tap isn't the driver, it still has to take care to
just grab the updated value once for the frame...  The tap API
can't really automagically do that single update of the local
variable when passenger though, since it's been deliberately kept
devoid of all the type-specific accessor junk (ptr/elems etc are
all void **/void* in the tap api side).  Hence why this commit is
just ad-hoc updating the local variable in the else branch; it's
best positioned to do so directly.  But there still needs to be
more consideration for thread-safety.

1 files changed, 16 insertions, 7 deletions

diff --git a/src/modules/mixer/mixer.c b/src/modules/mixer/mixer.c
index 7de91e6..efa7de3 100644
--- a/src/modules/mixer/mixer.c
+++ b/src/modules/mixer/mixer.c
@@ -1,3 +1,4 @@
+#include <assert.h>
 #include <math.h>
 #include <stdlib.h>
 #include <string.h>
@@ -110,10 +111,12 @@ static void mixer_prepare_frame(til_module_context_t *context, til_stream_t *str
 
 	if (!til_stream_tap_context(stream, context, NULL, &ctxt->taps.T))
 		*ctxt->T = cosf(ticks * .001f) * .5f + .5f;
+	else /* we're not driving the tap, so let's update our local copy just once */
+		ctxt->vars.T = *ctxt->T; /* FIXME: taps need synchronization/thread-safe details fleshed out / atomics */
 
 	switch (((mixer_setup_t *)context->setup)->style) {
 	case MIXER_STYLE_FLICKER:
-		if (randf(&context->seed) < *ctxt->T)
+		if (randf(&context->seed) < ctxt->vars.T)
 			i = 1;
 		else
 			i = 0;
@@ -121,20 +124,23 @@ static void mixer_prepare_frame(til_module_context_t *context, til_stream_t *str
 		til_module_render(ctxt->inputs[i].module_ctxt, stream, ticks, &fragment);
 		break;
 
-	case MIXER_STYLE_FADE:
-		if (*ctxt->T < 1.f) {
+	case MIXER_STYLE_FADE: {
+		float	T = ctxt->vars.T;
+
+		if (T < 1.f) {
 			til_module_render(ctxt->inputs[0].module_ctxt, stream, ticks, &fragment);
 
-			if (*ctxt->T > 0.f)
+			if (T > 0.f)
 				ctxt->snapshots[0] = til_fb_fragment_snapshot(&fragment, 0);
 		}
 
-		if (*ctxt->T > 0.f) {
+		if (T > 0.f) {
 			til_module_render(ctxt->inputs[1].module_ctxt, stream, ticks, &fragment);
-			if (*ctxt->T < 1.f)
+			if (T < 1.f)
 				ctxt->snapshots[1] = til_fb_fragment_snapshot(&fragment, 0);
 		}
 		break;
+	}
 
 	default:
 		assert(0);
@@ -189,11 +195,14 @@ static void mixer_render_fragment(til_module_context_t *context, til_stream_t *s
 		break;
 
 	case MIXER_STYLE_FADE: {
-		float	T = *ctxt->T;
+		float	T = ctxt->vars.T;
 
 		if (T <= 0.f || T  >= 1.f)
 			break;
 
+		assert(ctxt->snapshots[0]);
+		assert(ctxt->snapshots[1]);
+
 		/* for the tweens, we already have snapshots sitting in ctxt->snapshots[],
 		 * which we now interpolate the pixels out of in parallel
 		 */