[audio_core] Fix audio effects timing.
Fix a sign error which was causing us to compensate for delay
introduced by audio effects in the wrong direction, effectively
shifting the audio out by 2x the amount introduced by the effects,
instead of canceling it out as it should have.
Tests: Verified timing on scope using pulse-gen test app with a GPIO
reference.
Change-Id: Ib63a2d410fccf818bc2703f357b8d7d0e978c08f
Reviewed-on: https://fuchsia-review.googlesource.com/c/fuchsia/+/378087
Commit-Queue: Martin Puryear <mpuryear@google.com>
Reviewed-by: Dustin Green <dustingreen@google.com>
Reviewed-by: Tim Detwiler <tjdetwiler@google.com>
Testability-Review: Tim Detwiler <tjdetwiler@google.com>
diff --git a/src/media/audio/audio_core/effects_stage.cc b/src/media/audio/audio_core/effects_stage.cc
index f927149..60ad6ac 100644
--- a/src/media/audio/audio_core/effects_stage.cc
+++ b/src/media/audio/audio_core/effects_stage.cc
@@ -113,11 +113,15 @@
auto snapshot = source_->ReferenceClockToFractionalFrames();
// Update our timeline function to include the latency introduced by these effects.
+ //
+ // Our effects shift incoming audio into the future by "delay_frames".
+ // So input frame[N] corresponds to output frame[N + delay_frames].
int64_t delay_frames = effects_processor_->delay_frames();
- auto delay_frac_frames = FractionalFrames<int64_t>(-delay_frames);
- auto delay_function = TimelineFunction(delay_frac_frames.raw_value(), 0, TimelineRate(1, 1));
- snapshot.timeline_function =
- TimelineFunction::Compose(delay_function, snapshot.timeline_function);
+ auto delay_frac_frames = FractionalFrames<int64_t>(delay_frames);
+
+ auto source_frac_frame_to_dest_frac_frame =
+ TimelineFunction(delay_frac_frames.raw_value(), 0, TimelineRate(1, 1));
+ snapshot.timeline_function = source_frac_frame_to_dest_frac_frame * snapshot.timeline_function;
return snapshot;
}
diff --git a/src/media/audio/audio_core/effects_stage_unittest.cc b/src/media/audio/audio_core/effects_stage_unittest.cc
index 97819b7..df5cf21 100644
--- a/src/media/audio/audio_core/effects_stage_unittest.cc
+++ b/src/media/audio/audio_core/effects_stage_unittest.cc
@@ -182,17 +182,20 @@
});
auto effects_stage = EffectsStage::Create(effects, stream);
- // Since our effect is introducing 13 frames of latency, the frame at time 0 is now actually frame
- // -13, and the actual frame 0 will occur once it makes its way through the effect.
- auto timeline_function = effects_stage->ReferenceClockToFractionalFrames().timeline_function;
- EXPECT_EQ(FractionalFrames<int64_t>::FromRaw(timeline_function.Apply(0)),
- FractionalFrames<int64_t>(-13));
+ // Since our effect introduces 13 frames of latency, the incoming source frame at time 0 can only
+ // emerge from the effect in output frame 13.
+ // Conversely, output frame 0 was produced based on the source frame at time -13.
+ auto ref_clock_to_output_frac_frame =
+ effects_stage->ReferenceClockToFractionalFrames().timeline_function;
+ EXPECT_EQ(FractionalFrames<int64_t>::FromRaw(ref_clock_to_output_frac_frame.Apply(0)),
+ FractionalFrames<int64_t>(13));
- // Similarly, at the time we'd normally expect frame 13, we should instead be seeing frame 0. Use
- // a fuzzy compare to allow for slight rounding errors.
- int64_t frame_13_time = (zx::sec(13).to_nsecs()) / kDefaultFormat.frames_per_second();
+ // Similarly, at the time we produce output frame 0, we had to draw upon the source frame from
+ // time -13. Use a fuzzy compare to allow for slight rounding errors.
+ int64_t frame_13_time = (zx::sec(-13).to_nsecs()) / kDefaultFormat.frames_per_second();
auto frame_13_frac_frames =
- FractionalFrames<int64_t>::FromRaw(timeline_function.Apply(frame_13_time)).Absolute();
+ FractionalFrames<int64_t>::FromRaw(ref_clock_to_output_frac_frame.Apply(frame_13_time))
+ .Absolute();
EXPECT_LE(frame_13_frac_frames.raw_value(), 1);
}
