Fix a "black screen flash" which occurs sometimes when restarting a track#111
Fix a "black screen flash" which occurs sometimes when restarting a track#111
Conversation
This was being set as part of #96 to try to avoid a longer gray flash, but inadvertently this being reset on destroy meant that if a track processor was reused with multiple cycles of init/destroy, it would get preserved even though all the mediapipe setup logic that was happening during the gray flash wouldn't get rerun.
…hen disconnecting
…essorWrapper pipeline This always happens at the end of processing a stream
…that a restart is imminent
… this.destroy using a different value
Ensure you can't destroy a ProcessorWrapper when it is already destroyed.
🦋 Changeset detectedLatest commit: 367d72a The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
…en restarting the processor wrapper
1egoman
changed the title
|
|
||
| <div id="initial-mode-wrapper" style="display: none;"> | ||
| <span class="text-secondary">Initial mode:</span> | ||
| <select class="custom-select" id="initial-mode-select" style="width: 200px;"> | ||
| <option value="disabled">Disabled</option> | ||
| <option value="background-blur">Blur</option> | ||
| <option value="virtual-background">Virtual Background</option> | ||
| </select> | ||
| </div> |
There was a problem hiding this comment.
I added this UI control in the example to change the initial state the background processor is added in:
|
|
||
| type ProcessorWrapperLifecycleState = 'idle' | 'initializing' | 'running' | 'media-exhausted' | 'destroying' | 'destroyed'; | ||
|
|
||
| export interface ProcessorWrapperOptions { |
There was a problem hiding this comment.
The ProcessorWrapper had a lot of implicit state checking logic that made it hard to follow, so I refactored it to be a big state machine instead.
lukasIO
left a comment
lukasIO
left a comment
There was a problem hiding this comment.
thanks for digging into this!
| if (symbol && this.symbol !== symbol) { | ||
| // If the symbol is provided, we only destroy if it matches the current symbol | ||
| /** Called if the media pipeline no longer can read frames to process from the source media */ | ||
| private async handleMediaExhausted() { |
|
Should we land this PR ? |
|
@xianshijing-lk I was going to merge this last week but noticed that there was a bug in the new example code this change introduces only in firefox where buttons are disabled / not clickable sometimes when they should be. I haven't gotten a chance to dig into why this is happening yet. |
…ndling / finally state setting logic
|
^ Ok, worked around the issue with the above commit! |
3 participants
Summary
@lukasIO recently noticed that on 3920c38, there was a visual artifact visible when a track processor was enabled and the track was restarted (one thing that "flip video" button does). Example below:
Screen.Recording.2025-10-09.at.10.10.14.AM.mov
I dug into it and it ended up being an unintended side effect of #96 - the
this.isFirstFramevalue was being reset on restarts even though the initialization was already done, leading to an errant delay on the first frame that shouldn't happen.Untitled.mov
Note: the above is in chrome, but was able to replicate the same behavior on chrome, firefox, and safari.
Technical Deep Dive
This was where I started: #107 (comment). When a track is restarted that has a track processor attached via
setProcessor, it looks like a few things happen in this order:.restart()is called), which then ...transformer.destroy()...transformer.init().Unfortunately this has some problems:
initthendestroyis not always the same as arestartFirst, calling
init/destroywhen performing aProcessorWrapperrestart means it is impossible to distinguish between calling init and then calling destroy manually or a restart which does these in quick succession. TheBackgroundTransformerneeds to setthis.isFirstFrametotruein a processor wrapper restart scenario but this doesn't need to happen in non processor wrapper restart scenarios.So, to distinguish between the two cases, I added an options object which is passed to
TrackTransformer.destroythat has an optionwillProcessorRestartindicating if this destroy is part of a restart. This allows theTrackTransformerto adequitely distinguish between the two cases.Note that there is a concept of a "transformer restart", but I can't quite get this to do what I want. In theory it seems like maybe I could proxy this processor wrapper signal downwards, but I couldn't get it to work after a few hours of playing around locally so I gave up. Also, doing this might be somewhat of a breaking change for any user developed custom track transformers, given that there could be custom logic in
destroy/initthatrestartisn't handling properly if the code path that a processor level restart takes were to be switched, so I'm also hesitant to make that change just generally.destroybeing called more than onceIn the above list, item 3 (
... callstransformer.destroy()...) can actually happen MULTIPLE times, due to a longstanding issue. Lukas worked around this here, but the workaround just patches the problem by blocking execution, it doesn't actually solve anything. This means when a "processor wrapper restart" happens, a second destroy call is fired which has the wrongwillProcessorRestartvalue set, resulting in weird behavior.It turns out this "destroy being called multiple times" issue was because the stream processor path (but not the fallback path) was calling
this.destroyafter it ran out of frames. So, calling destroy manually would close the processor stream, which then would cause it to terminate, which would then callthis.destroyAGAIN! So instead, break updestroyintodestroyandcleanup(which just cleans up the media pipeline objects, but doesn't actually officially proxy thedestroydown to the transformer), and only callthis.cleanupwhen a media pipeline runs out of frames.