Skip to content

fix: tighten VideoTrackRenderer Widget #695

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 19 commits into
base: main
Choose a base branch
from

Conversation

holzgeist
Copy link
Contributor

@holzgeist holzgeist commented Jan 28, 2025

This allows external users to e.g. apply rounded edges. Without this patch, VideoTrackRenderer will expand to its available space with no way (known to me..) for parent widgets to know how large the video actually is.

depends on flutter-webrtc/flutter-webrtc#1754 (and flutter-webrtc/webrtc-interface#30)

@holzgeist
Copy link
Contributor Author

tests fail due to required changes in flutter-webrtc and webrtc-interface not being merged yet

@holzgeist
Copy link
Contributor Author

I just realized that I should probably move the sized box up the chain to _VideoTrackRendererState.build to also include web..

@cloudwebrtc
Copy link
Contributor

This PR will be merged after the new flutter-webrtc is released

@cloudwebrtc
Copy link
Contributor

cloudwebrtc commented Feb 7, 2025

I tested this PR and found that it broke the fit setting in VideoTrackRenderer.

when I publish a video track from the portrait viewfinder on a mobile

Original code
RTCVideoViewObjectFitContain

image

RTCVideoViewObjectFitCover
image

Modification from this PR

RTCVideoViewObjectFitContain
image

RTCVideoViewObjectFitCover
image

As shown in the screenshot, PR broke the centering of widgets and the rendering of the RTCVideoViewObjectFitCover mode

@holzgeist
Copy link
Contributor Author

oh right, I didn't test with the cover option. All my sizing should be ignored in cover option as the widget will fill up all available space anyway. I'll push a fix soon

This allows external users to e.g. apply rounded edges.
Without this patch, VideoTrackRenderer will expand to its available
space with no way for parent widgets to know how large the video
actually is
also, cover all paths by moving up sizing logic to build function
@holzgeist holzgeist force-pushed the video-size-constraint branch from 18c5f03 to 32b8b0a Compare February 10, 2025 11:00
@holzgeist holzgeist force-pushed the video-size-constraint branch 2 times, most recently from a722a19 to 8daa6b0 Compare May 7, 2025 14:29
@cloudwebrtc cloudwebrtc closed this Jul 1, 2025
@holzgeist
Copy link
Contributor Author

Hi @cloudwebrtc

what's the reason for closing this? I added the fix you requested. (Though now I see that I didn't ping you about it..)

@cloudwebrtc
Copy link
Contributor

cloudwebrtc commented Jul 1, 2025

I think this PR breaks the existing layout, this is the main branch layout
image
image

This branch causes the position of the video widgets to shift, the video view is always docked in the upper left corner.
image
image

You can try dragging the window and changing the video size, and the video should always be displayed in the center.

@holzgeist
Copy link
Contributor Author

oh, sorry about that! I did test both options with my app, but not with the example app. I'll fix it and ping you here to reopen the PR (if I get it to work :) )

@holzgeist
Copy link
Contributor Author

holzgeist commented Jul 1, 2025

It works with the example app now (tested on native linux and web).

The initial issue was that flutter-webrtc wraps the whole view in a center, preventing any "insights" into the actual size. My PR "removes" that Center by wrapping in a LayoutBuilder and SizedBox.
Our app makes use of the original size to add rounded edges and then wraps in a center again.
The example app (and very likely other users) don't do this, so I introduced a autoCenter parameter to wrap the result in a center. Not for cover fit though, it doesn't matter in that case as all the available space is filled anyway

@holzgeist
Copy link
Contributor Author

@cloudwebrtc should I open another PR or can you reopen this one? It looks like I can't reopen it

@cloudwebrtc cloudwebrtc reopened this Jul 1, 2025
@holzgeist
Copy link
Contributor Author

The build failure for android does not seem to be connected to my changes, the PR should be ready for review now

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.

2 participants