Skip to content

Release detached seek frame when the FFmpeg video loop exits - #91

Closed
Blackspirits wants to merge 2 commits into
upl/review-base-f67bfrom
fix/ffmpeg-last-dropped-frame-release-f67b
Closed

Blackspirits wants to merge 2 commits into
upl/review-base-f67bfrom
fix/ffmpeg-last-dropped-frame-release-f67b

Conversation

@Blackspirits

@Blackspirits Blackspirits commented Sep 13, 2026

Copy link
Copy Markdown
Owner

Summary

During seek handling the video decoder may keep one converted frame in lastDropped while it advances from the key frame to the requested target. That frame is deliberately detached from VideoFrameQueue: it is neither queued nor returned to the pool yet.

If the session closes, the video loop exits, or an exception is thrown before a frame at/after the target is presented, lastDropped falls out of scope without being returned. VideoFrame owns unmanaged memory allocated with Marshal.AllocHGlobal, and it has no finalizer, so this path leaks native memory.

This change:

  • moves lastDropped to the video-loop lifetime scope;
  • returns it through VideoFrameQueue.Return from finally;
  • preserves the normal paths where it is returned or pushed and set to null;
  • relies on VideoFrameQueue.Return to dispose it immediately when the queue has already been closed.

Validation

  • based on upstream f67b07f5b143fa47c87255ca1c3ed7adb3bd7142
  • diff is limited to FfmpegPlayer.cs
  • VideoFrame owns an unmanaged HGlobal and has no finalizer
  • Session.Dispose closes _videoFrames before joining the video thread, so returning a detached frame during loop teardown disposes it safely
  • full solution restore passed
  • full solution build passed
  • full solution test suite passed on the first run
  • UI suite: 5,150 passed, 9 skipped, 0 failed (5,159 total)
  • retry step was not used
  • validation run: #34772887525

AI assistance: ChatGPT was used to audit FFmpeg frame ownership across seek, fallback and teardown paths.

@Blackspirits Blackspirits left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Independent adversarial re-check: lastDropped is a detached VideoFrame owning unmanaged HGlobal memory. Normal presentation/serial-change paths null or return it, but teardown/exception could previously leave it unreachable. Moving ownership to the loop scope and returning it in finally is compatible with queue shutdown because VideoFrameQueue.Return disposes frames after Close. Full CI #34772887525 passed restore, build and the complete suite on the first run; UITests reported 5,150 passed, 9 skipped and 0 failed. Retry was not used. No blocker identified. Keep draft; no merge performed.

Copy link
Copy Markdown
Owner Author

Superseded by current-base draft PR #94. Upstream changed FfmpegPlayer and enlarged VideoFrame allocations after this draft; #94 reapplies the same ownership fix on c77c4b4 and has full green CI. Closing the older-base draft to avoid competing fixes; no merge performed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant