PlayHead: divide by zero on the audio thread when looping is turned on during playback

Open Beginner friendly
  • Is something wrong?
#482 0 comments 0 reactions 0 assignees View on GitHub

Maintainers usually reply within 1 day

Nobody has claimed this yet.

Assessment

Difficulty
2/5
Estimated time
1-3 hours
Newbie friendliness
76/100
Issue type
Bug
Clarity
Clearly specified
Activity status
Active
Tech stack
cpp
Domain
audio-video-rtc, backend

Research direction

Start in PlayHead::setLoopRange and PlayHead::linearPositionToLoopPosition, named in the issue, and trace how PlayHeadState::update reads the loop range during playback. Check for existing PlayHead tests and add coverage for enabling and disabling looping with an empty range; run the relevant test. Done means the audio thread cannot pass a zero-length range to modulo.

Written by the indexing model from the issue text.

Description

graph::PlayHead can divide by zero on the audio thread when looping is switched on during playback.

Stack: juce::negativeAwareModulo ← PlayHead::linearPositionToLoopPosition ← PlayHead::referenceSamplePositionToTimelinePosition ← PlayHeadState::update.

Cause: PlayHead::setLoopRange sets looping = loop before timelinePlayRange.store (loopRange). Turning the loop off stores an empty range (TransportControl: setLoopTimes (false, {})), so when it's turned back on the audio thread can briefly see looping == true with that zero-length range, and negativeAwareModulo divides by its length. The reverse can happen too: a reader holding looping == true from just before it's turned off, then reading the empty range.

Fix we're carrying (seen rarely, as a crash under load on Windows):

 inline void PlayHead::setLoopRange (bool loop, juce::Range<int64_t> loopRange, bool updatePosition)
 ...
-        looping = loop;
-        timelinePlayRange.store (loopRange);
+        if (loop)
+        {
+            timelinePlayRange.store (loopRange);
+            looping = true;
+        }
+        else
+        {
+            looping = false;
+            timelinePlayRange.store (loopRange);
+        }

 inline int64_t PlayHead::linearPositionToLoopPosition (int64_t position, juce::Range<int64_t> loopRange)
 {
+    if (loopRange.getLength() <= 0)
+        return position;
+
     const auto loopStart = loopRange.getStart();

The guard alone is enough to stop the crash, since the two atomics can't be read together; the ordering just makes it rarer. Seen on 38d5a27.

Dominant language
C++
Stars
1.5k
Forks
222
Avg merge
8h 7m
Merged PRs (30d)
17

Getting set up

This project ships no dev container, Dockerfile or contributing guide, so setting up is up to you: start from its README, and see our first-contribution guide for the general steps.

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from Tracktion/tracktion_engine

All issues in Tracktion/tracktion_engine

Similar issues

More C++ issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.