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

Add getThumbnail APIs to Player and Source #225

Merged
merged 4 commits into from
Sep 7, 2023

Conversation

rolandkakonyi
Copy link
Contributor

@rolandkakonyi rolandkakonyi commented Sep 6, 2023

Description

This PR extends the Player and Source APIs to allow fetching thumbnail information.

Changes

Introduced:

  • Player.getThumbnail(): to fetch thumbnail information for the current source
  • Source.getThumbnail(): to fetch thumbnail information for the given source

Checklist

  • 🗒 CHANGELOG entry

@rolandkakonyi rolandkakonyi self-assigned this Sep 6, 2023
@rolandkakonyi rolandkakonyi changed the title feat(thumbnail): add getThumbnail APIs Add getThumbnail APIs to Player and Source Sep 6, 2023
@rolandkakonyi rolandkakonyi marked this pull request as ready for review September 6, 2023 14:23
@rolandkakonyi rolandkakonyi requested review from matamegger and zigavehovec and removed request for matamegger September 6, 2023 14:24
Copy link
Contributor

@zigavehovec zigavehovec 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 👍

CHANGELOG.md Outdated Show resolved Hide resolved
@ReactMethod
fun getThumbnail(nativeId: NativeId, time: Double, promise: Promise) {
uiManager()?.addUIBlock {
promise.resolve(JsonConverter.fromThumbnail(players[nativeId]?.source?.getThumbnail(time)))
Copy link
Contributor

Choose a reason for hiding this comment

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

Player.getThumbnail has been deprecated in favour of Source.getThumbnail on Android. From my perspective I would be in favour of only wrapping Source.getThumbnail in this PR, but I'm not sure what the state is on iOS, just wanted to point this out.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@zigavehovec I know about the deprecation on Android but I went for providing both as our Source API is not that rich in the React Native SDK yet. Also we don't support playlist, so only one Source is available at a time.

@rolandkakonyi rolandkakonyi merged commit 28f0d75 into development Sep 7, 2023
@rolandkakonyi rolandkakonyi deleted the add-thumbnail-API branch September 7, 2023 13:54
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