Skip to content

Fix video render mode switching - #1135

Merged
hiroshihorie merged 3 commits into
mainfrom
hiroshi/video-render-mode-switch
Jul 20, 2026
Merged

hiroshihorie merged 3 commits into
mainfrom
hiroshi/video-render-mode-switch

Conversation

@hiroshihorie

Copy link
Copy Markdown
Member

Summary

Switching renderMode on a live VideoTrackRenderer (texture to platformView or back) previously misbehaved on iOS, and would have on macOS after #1116:

  • platformView to texture crashed during build. _renderer still held the RTCVideoPlatformViewController, which then failed the as rtc.RTCVideoRenderer cast.
  • texture to platformView leaked the old texture renderer. It stayed attached to srcObject, kept consuming decoded frames and held its GPU texture.
  • initState adopted a texture-type cachedRenderer even in platform view mode.

Fix

didUpdateWidget now resets the renderer when the effective render mode changes, with ownership respected:

  • Platform view controllers are only detached, never disposed here. They are created and disposed by RTCVideoPlatFormView, so disposing them from this widget raced its own dispose (the controller's _disposed guard is set only after two awaits, so both dispose calls could pass it).
  • A caller-provided cachedRenderer is never disposed on a mode switch, so it can be re-adopted when switching back.
  • Self-created texture renderers are disposed according to autoDisposeRenderer, matching unmount behavior.

Testing

  • flutter analyze and dart format clean.

🤖 Generated with Claude Code

Switching renderMode on a live VideoTrackRenderer previously crashed
(platformView to texture cast error) or leaked the old texture renderer
(texture to platformView). Reset the renderer on mode change, respecting
ownership: platform view controllers are disposed by RTCVideoPlatFormView
and caller-provided cachedRenderers are never disposed here.

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no potential bugs to report.

View in Devin Review to see 1 additional finding.

Open in Devin Review

devin-ai-integration[bot]

This comment was marked as resolved.

The renderer was adopted under the old widget's configuration, so
checking identity against the incoming widget.cachedRenderer could
wrongly dispose a caller-owned renderer when the caller swaps both
renderMode and cachedRenderer in the same update.
@hiroshihorie
hiroshihorie merged commit f7530d3 into main Jul 20, 2026
15 checks passed
@hiroshihorie
hiroshihorie deleted the hiroshi/video-render-mode-switch branch July 20, 2026 17:02
hiroshihorie added a commit that referenced this pull request Jul 28, 2026
Changes the default `renderMode` from `texture` to `auto`. For now
`auto` resolves to texture rendering on all platforms, so behavior is
unchanged. This lets the SDK change how `auto` resolves in a future
release without an API break.

Platform view support and related fixes from the original diff already
landed via #1116 and #1135.
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