这是indexloc提供的服务，不要输入任何密码
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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Jul 31, 2025
Merged

fix: tighten VideoTrackRenderer Widget #695

merged 1 commit into from
Jul 31, 2025

Conversation

@holzgeist
Copy link
Contributor

@holzgeist holzgeist commented Jan 28, 2025 •
edited
Loading

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

holzgeist commented Jan 28, 2025

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

@holzgeist
Copy link
Contributor Author

holzgeist commented Jan 29, 2025

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

cloudwebrtc commented Feb 5, 2025

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

@cloudwebrtc
Copy link
Contributor

cloudwebrtc commented Feb 7, 2025 •
edited
Loading

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

holzgeist commented Feb 10, 2025

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

@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

holzgeist commented Jul 1, 2025

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 •
edited
Loading

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

holzgeist commented Jul 1, 2025

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 •
edited
Loading

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

holzgeist commented Jul 1, 2025

@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

holzgeist commented Jul 2, 2025

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

@holzgeist
fix: tighten VideoTrackRenderer Widget
8fc1c84 
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
@holzgeist holzgeist force-pushed the video-size-constraint branch from 47bb716 to 8fc1c84 Compare July 8, 2025 10:00
@holzgeist
Copy link
Contributor Author

holzgeist commented Jul 28, 2025

Hi @cloudwebrtc
Is there something that's missing in the PR or is it in your queue?
No hurries though if you don't need anything from my side :)

cloudwebrtc
cloudwebrtc approved these changes Jul 31, 2025
View reviewed changes
Copy link
Contributor

@cloudwebrtc cloudwebrtc left a comment

Choose a reason for hiding this comment

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

tested on linux desktop, it seems works fine.

@cloudwebrtc cloudwebrtc merged commit 804eaf0 into livekit:main Jul 31, 2025
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@cloudwebrtc cloudwebrtc cloudwebrtc approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@holzgeist @cloudwebrtc