Conversation
… render NodeRenderContext waits for the graph's leaf nodes to become ready before rendering each block, which is how it waits for proxies and rendered sources to be generated. That wait had no bound, so a WaveNode whose source file had been deleted or moved never became ready and the render retried forever: Renderer::renderToFile never returned and no error was reported. The wait is now bounded by Renderer::Parameters::sourceReadyTimeout (10s by default) in both the audio and MIDI paths. The timer is reset for as long as any proxy or render job is running, so waits for something the engine is actually generating stay unbounded. On timeout the partial file is deleted and the render finishes with an error message, so renderToFile returns false. fixes #416 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #434 +/- ##
===========================================
+ Coverage 59.81% 59.84% +0.03%
===========================================
Files 568 568
Lines 79964 80011 +47
Branches 12381 12386 +5
===========================================
+ Hits 47828 47881 +53
+ Misses 32136 32130 -6 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
I'm not sure about this... I think the existing Is this not what's happening? |
|
🤖 Good challenge - I checked, and that isn't what's happening, because
static inline HashCode getAudioFileHash (const juce::File& file) noexcept
{
return file.getFullPathName().hashCode64();
}so Where it actually sticks, confirmed by instrumenting the run:
So the node never reports ready, Why I didn't just bail on a missing fileThe obvious version of what you're describing - return true when the file doesn't exist or its info is invalid - breaks the proxy wait, because that wait uses exactly the same two signals. That's why I bounded the wait instead: it doesn't need to tell the two cases apart, and it covers every leaf node rather than just If you'd rather it rendered silenceI agree that's the nicer user-facing behaviour, and I think it can be done properly rather than by inference. At node-build time we already know whether a file is expected to be generated ( Happy to redo it that way, either replacing the timeout or keeping it as a backstop for the other leaf node types. Which would you prefer? |
|
It would be much better the second way as some proxy files can take longer that 10s to generate. |
…iming out Replaces the Renderer::Parameters::sourceReadyTimeout bound from the previous commit, which capped every wait for a leaf node and so could also cut short a proxy that legitimately takes longer than the timeout to generate. Instead, decide at node-build time whether the playback file is ever going to appear. AudioClipBase::isGeneratingPlaybackFile() reports whether the clip is generating it (a proxy, or a source render it should attempt), mirroring the conditions in beginRenderingNewProxyIfNeeded so it's true before the async job has actually started. createNodeForAudioClip now returns no node when the playback file is invalid and nothing is going to produce it, alongside the existing bail for a null playback file. WaveNode::isReadyToProcess keeps its current meaning, so waits for proxies stay unbounded, and a deleted or moved source is simply left out of the graph and renders as silence rather than stalling the render. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
🤖 Done - reworked to the node-build-time approach, timeout removed. What's there now
bool AudioClipBase::isGeneratingPlaybackFile()
{
// A source the clip renders itself (an EditClip, say) only exists once that render
// has run, and a failed render sets lastRenderJobFailed so we stop claiming it's coming
if (shouldAttemptRender() && ! getAudioFile().isValid())
return true;
if (! canUseProxy())
return false;
// Proxies are generated by the timer started in beginRenderingNewProxyIfNeeded, so the
// job may not exist yet - go by whether one is needed rather than whether one is running
return usesTimeStretchedProxy() || getAudioFile().getInfo().needsCachedProxy;
}It mirrors the conditions in
// Trigger proxy render if it needs it
clip.beginRenderingNewProxyIfNeeded();
// A playback file that can't be read and that the clip isn't going to generate will never
// appear - the source has been deleted, moved or is unreadable. Building a node for it would
// leave a leaf that never reports itself ready, which makes an offline render wait forever
if (! playFile.isValid() && ! clip.isGeneratingPlaybackFile())
return {};
Test
One residual caseIf a proxy or source render is started and then fails, the node has already been built and |
Summary
Rendering an Edit that references a clip whose source file has been deleted or moved never returned -
Renderer::renderToFilespun indefinitely with no error. The wait for clip sources is now bounded and reported as a render failure.Root cause
NodeRenderContext::renderNextBlockchecks that every leaf node in the graph is ready before rendering a block, which is how a render waits for proxies and rendered sources to be generated:Returning
falsemakesRenderTask::runJobreturnjobNeedsRunningAgain, and the drivers (Renderer::renderToFile'swhile (task->runJob() == jobNeedsRunningAgain) {}andEditRenderer's render thread) simply call it again. There was no bound on that retry.For a clip whose file is gone,
WaveNode::isReadyToProcess()never returns true.AudioFileCache::getOrCreateCachedFilestill hands out aCachedFile(the memory-mapped format manager matches on extension, not existence), socreateReadersucceeds, but the cachedAudioFileInfohas a sample rate of 0, soupdateFileSampleRate()fails on every call.WaveNodealready bails out for anAudioFilewith a null hash ("this will never return a valid reader and we should just bail") but has no equivalent for a file that has gone missing after the fact. The MIDI path (NodeRenderContext::renderMidi) had the same unboundedwhile (! leafNodesReady())loop.The fix
The wait is bounded rather than special-cased per node type, so it covers every leaf node, not just
WaveNode:Renderer::Parameters::sourceReadyTimeout(10 seconds by default; zero or negative restores the old unbounded wait).NodeRenderContexttracks how long it has been waiting for leaf nodes. The timer is reset for as long as any proxy or render job is running, so legitimately long waits for something the engine is actually generating (timestretch proxies,EditCliprenders, freeze files) are still unbounded - only stalls where nothing is being generated count down.RenderTask::errorMessage, so the job reports as finished andRenderer::renderToFilereturnsfalse. The MIDI path returns the same message.AudioProxyGenerator::isAnyProxyBeingGenerated()to support the reset check alongsideRenderManager::getNumJobs().Regression test
Renderer: missing source file fails the render instead of retrying foreverintracktion_Renderer.test.cppinserts a wave clip, deletes the source from under the Edit, then renders with a 1 secondsourceReadyTimeout. It asserts the render finishes, reports an error naming the source file, and leaves no output file behind. With the fix reverted it fails (the render never calls back inside a 30 second bound); with the fix it passes in ~1 second. FullTestRunnersuite: 370/370 pass.Notes
The
*** Cache misslogging the reporter saw is a separate symptom of the same missing file (AudioFileCache::Reader::readcan't map a section of a file that isn't there); the render itself stalls before reaching it in the case reproduced here. This PR doesn't change the cache's behaviour.BREAKING-CHANGES.mdhas an entry, since an app that generates a clip's source file through a mechanism the engine doesn't know about will now see such a render fail after the timeout rather than wait.Fixes #416
🤖 Generated with Claude Code