Skip to content
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

[video_player_avplay] Automatically rotates video player based on device orientation #817

Merged
merged 1 commit into from
Feb 12, 2025

Conversation

JSUYA
Copy link
Member

@JSUYA JSUYA commented Feb 11, 2025

We already provide rotate API for video player,
but it seems like a bug that the video player's video image doesn't rotate according to the device orientation. So we should automatically change this inside video_player. When user call the API, the video image may rotate, but when the device's orientation changes, the rotation may change again.

@JSUYA
Copy link
Member Author

JSUYA commented Feb 11, 2025

@xiaowei-guan
The service team asked me about this issue. They want to handle the device that needs to support portrait mode inside the player, not in Guide. Can you please review this approach?
I'm worried that calling MediaQuery.orientationOf(context) will have a performance impact as build() is called repeatedly while the video player is playing.

@xiaowei-guan
Copy link
Contributor

The app side must call setState after initialized:

_controller.initialize().then((_) => setState()));

@JSUYA
Copy link
Member Author

JSUYA commented Feb 11, 2025

I waiting for #814 merge

@xiaowei-guan
Copy link
Contributor

I waiting for #814 merge

Done

…ice orientation

We already provide rotate API for video player,
but it seems like a bug that the video player's video image doesn't rotate according to the device orientation.
So we should automatically change this inside video_player.
When user call the API, the video image may rotate,
but when the device's orientation changes, the rotation may change again.
@JSUYA JSUYA force-pushed the video_player_avplay/rotate_auto branch from b98dd97 to e5ea2c1 Compare February 11, 2025 09:11
@JSUYA
Copy link
Member Author

JSUYA commented Feb 11, 2025

Rebased PR

@xiaowei-guan
Copy link
Contributor

@xiaowei-guan The service team asked me about this issue. They want to handle the device that needs to support portrait mode inside the player, not in Guide. Can you please review this approach? I'm worried that calling MediaQuery.orientationOf(context) will have a performance impact as build() is called repeatedly while the video player is playing.

If we want to reduce the call times of MediaQuery.orientationOf, We can also use this:

OrientationBuilder(
        builder: (BuildContext context, Orientation orientation)

@JSUYA
Copy link
Member Author

JSUYA commented Feb 12, 2025

@xiaowei-guan The service team asked me about this issue. They want to handle the device that needs to support portrait mode inside the player, not in Guide. Can you please review this approach? I'm worried that calling MediaQuery.orientationOf(context) will have a performance impact as build() is called repeatedly while the video player is playing.

If we want to reduce the call times of MediaQuery.orientationOf, We can also use this:

OrientationBuilder(
        builder: (BuildContext context, Orientation orientation)

When I tested it, the build() calls from calling MediaQuery.orientationOf and OrientationBuilder both had the same call count. This is probably an effect of the prograssbar UI.

When we provided a guide about setDisplayRotate(), we did so by using OrientationBuilder().
However, the Orientation of OrientationBuilder doesn't change because it calculates the Orientation based on the size of the parent Container that contains it. (The ratio of the video screen are still the same.) That's why i had use MediaQuery.orientationOf.

@JSUYA JSUYA merged commit 582c2d3 into flutter-tizen:master Feb 12, 2025
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants