Stop reporting playback on a clip that ended during the play call - #75
Open
danielchalmers wants to merge 1 commit into
Open
Stop reporting playback on a clip that ended during the play call#75danielchalmers wants to merge 1 commit into
danielchalmers wants to merge 1 commit into
Conversation
PlayAsync sets IsPlaying once its play operation completes, with nothing checking whether the clip is still running by then. A clip that reaches its end while that operation is in flight raises Ended, whose handler correctly clears IsPlaying, and then the play call sets it straight back. The transport is left claiming to play a clip parked on its last frame, and nothing clears it until the user presses something else. Found by instrumenting an end-of-clip measurement that stalled once in eight runs, identically on 3.10.4 and 3.11.3, so this is pre-existing rather than something the upgrade introduced. The IsPlaying trace makes the ordering plain: --- iteration 0 start (playing=True) --- 2ms seeked to 00:00:13.8 1318ms IsPlaying=False pos=00:00:13.8 (Ended fired, handler cleared it) 8019ms IsPlaying=True pos=00:00:15 (play call returned and set it back) The window is not exotic: play pressed near the end of a clip, or a play queued behind an open that is still joining secondary cameras, which is what took eight seconds here. A counter bumped by the Ended handler lets the play call tell that the clip finished underneath it and leave the settled state alone. FakeCameraPlayer gains a PlayCallback hook, mirroring the SeekCallback it already had for interleaving an action mid-call. Two tests cover it: one ends the clip from inside the play call and asserts the controller does not report playback, one asserts an ordinary resume still does. Verified the first fails without the guard. 376 tests green.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
PlayAsyncsetsIsPlayingonce its play operation completes, with nothing checking whether the clip is still running by then. A clip that reaches its end while that operation is in flight raisesEnded, whose handler correctly clearsIsPlaying, and then the play call sets it straight back. The transport is left claiming to play a clip parked on its last frame, and nothing clears it until you press something else.Found while instrumenting an end-of-clip measurement during the Flyleaf 9.0 upgrade (#74). It stalled once in eight runs, identically on FlyleafLib 3.10.4 and 3.11.3, so this is pre-existing and unrelated to that upgrade, which is why it was held back into its own PR. The
IsPlayingtrace makes the ordering plain:The window is not exotic. Play pressed near the end of a clip hits it, and so does a play queued behind an open that is still joining secondary cameras, which is what took eight seconds in the trace above.
The fix
A counter bumped by the
Endedhandler lets the play call tell that the clip finished underneath it and leave the settled state alone.FakeCameraPlayergains aPlayCallbackhook, mirroring theSeekCallbackit already had for interleaving an action mid-call.Two tests: one ends the clip from inside the play call and asserts the controller does not report playback, one asserts an ordinary resume still does. I verified the first fails without the guard, with
Shouldly.ShouldAssertException : controller.IsPlaying.374 tests green,
dotnet format --verify-no-changesclean.