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] Add isLive property for Android, iOS and Web. #28903

Merged
merged 2 commits into from
May 20, 2024

Conversation

JustJoostNL
Copy link
Contributor

@JustJoostNL JustJoostNL commented May 15, 2024

Why

Expo-video doesn't have any way to check if a video is a livestream.

How

I added the isLive property on all platforms. For Web and iOS it checks if the duration is infinite, which means the video is a live stream. For Android it uses isCurrentMediaItemLive.

Test Plan

Tested using bare-expo.

Checklist

@expo-bot expo-bot added bot: suggestions ExpoBot has some suggestions bot: passed checks ExpoBot has nothing to complain about and removed bot: suggestions ExpoBot has some suggestions labels May 15, 2024
@@ -137,6 +139,7 @@ class VideoPlayer(context: Context, appContext: AppContext, source: VideoSource?
}
if (playbackState == Player.STATE_READY) {
Copy link
Contributor

Choose a reason for hiding this comment

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

shouldn't this have some sort of undefined state while the media item is loading? Are we making it explicit that you should not check this while the state is not ready?

Copy link
Member

Choose a reason for hiding this comment

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

@aleqsio I don't know if adding a special state while the media is loading isn't a bit overkill. I think we can just set it to false.

Copy link
Contributor

@aleqsio aleqsio left a comment

Choose a reason for hiding this comment

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

LGTM code wise, @behenate do you think it makes sense in the package?

Copy link
Member

@behenate behenate left a comment

Choose a reason for hiding this comment

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

@aleqsio I think it's a good addition, especially for people who will be creating their own UI controls on the JS side

@JustJoostNL Thanks for another contribution!

@tsapeta tsapeta merged commit df15cec into expo:main May 20, 2024
35 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bot: passed checks ExpoBot has nothing to complain about
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants