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

Introduce Picture in Picture #87

Merged
merged 20 commits into from
Dec 22, 2023
Merged

Introduce Picture in Picture #87

merged 20 commits into from
Dec 22, 2023

Conversation

zigavehovec
Copy link
Contributor

@zigavehovec zigavehovec commented Dec 19, 2023

Description

This PR introduces Picture in Picture handling on Android including a sample usage.

Prework PRs:
#85
#86

Changes

  • Add and use the FlutterPictureInPictureHandler
  • Relay PiP related events
  • Basic PiP sample

Tests

Checklist (for PR submitters and reviewers)

  • 🗒 CHANGELOG.md entry for new/changed features, bug fixes or important code changes
  • 🧪 Tests added and/or updated
  • 📢 New public API is fully documented

@zigavehovec zigavehovec self-assigned this Dec 19, 2023
@zigavehovec zigavehovec marked this pull request as ready for review December 21, 2023 15:17
Copy link
Contributor

@strangesource strangesource left a comment

Choose a reason for hiding this comment

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

Nice work!

Comment on lines 144 to 146
private val isPictureInPictureSupported: Boolean
get() = Build.VERSION.SDK_INT >= Build.VERSION_CODES.O

Copy link
Contributor

Choose a reason for hiding this comment

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

NIT: could be a top level function in the file to make it clear that it does not need any state from the view.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

example/lib/pages/picture_in_picture.dart Outdated Show resolved Hide resolved
Copy link
Collaborator

@hawk23 hawk23 left a comment

Choose a reason for hiding this comment

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

Nice work 💪

  • I see that the PictureInPictureAvailabilityChanged event is not implemented for Flutter. As it's Android only, do you think it would make sense to expose it?
  • As discussed on Slack, when _player.loadSourceConfig(_sourceConfig); is commented out in the PiP sample (no source loaded into the player), PiP does not work anymore on my personal Android device. The floating window does not appear and the player view just goes to Fullscreen mode.

example/lib/pages/picture_in_picture.dart Show resolved Hide resolved
@zigavehovec
Copy link
Contributor Author

I see that the PictureInPictureAvailabilityChanged event is not implemented for Flutter. As it's Android only, do you think it would make sense to expose it?

I actually missed this event. I'll create a follow up ticket and we can tackle this in another PR 🙂

As discussed on Slack, when _player.loadSourceConfig(_sourceConfig); is commented out in the PiP sample (no source loaded into the player), PiP does not work anymore on my personal Android device. The floating window does not appear and the player view just goes to Fullscreen mode.

I looked into this and found some limitation in the native SDK DefaultPictureInPictureHandler. We discussed it with @strangesource and decided for a workaround until we improve the situation on the native side. edce49e

Base automatically changed from PFL-83/add-configuration-change-handling to main December 22, 2023 13:20
@zigavehovec zigavehovec merged commit ab51814 into main Dec 22, 2023
3 checks passed
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.

3 participants