Picture-in-Picture Fix #151

arthurgeron · 2023-08-10T17:01:17Z

Picture-in-picture now optional and disabled by default, fixes Pip should be optional and return current state #122

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

src/index.ts

src/IVSPlayer.tsx

dawhitla

Just a couple of asks here on the PR. Additionally we are currently working on a PR to address some of the functional differences between PiP usage on Android vs iOS.

src/IVSPlayer.tsx

android/src/main/java/com/amazonaws/ivs/reactnative/player/AmazonIvsView.kt

android/src/main/AndroidManifest.xml

arthurgeron · 2023-08-18T14:48:16Z

Just a couple of asks here on the PR. Additionally we are currently working on a PR to address some of the functional differences between PiP usage on Android vs iOS.

Sounds great, I hope my changes are compatible, having some control over pip is a huge deal at my company since it'd create strange behaviors, for example: We use "track player" like lib to control and execute background playback; without any control, the pip would automatically start and duplicate the playback audio.

dawhitla · 2023-08-18T22:21:52Z

android/build.gradle

  compileSdkVersion getExtOrIntegerDefault('compileSdkVersion')
  buildToolsVersion getExtOrDefault('buildToolsVersion')
  defaultConfig {
-    minSdkVersion 21
+    minSdkVersion 26


Looks like we're getting a build issue here:
https://github.com/aws/amazon-ivs-react-native-player/actions/runs/5904085265/job/16025249877?pr=151#step:11:817

dawhitla · 2023-08-18T22:27:46Z

example/android/build.gradle

-        targetSdkVersion = 31
+        minSdkVersion = rootProject.ext.minSdkVersion
+        compileSdkVersion = rootProject.ext.targetSdkVersion
+        targetSdkVersion = rootProject.ext.targetSdkVersion


Unfortunately it seems example android project can't access rootProject's data.

arthurgeron-work · 2023-08-19T00:04:24Z

@dawhitia Not sure why the android e2e tests are failing

dawhitla · 2023-08-21T15:00:52Z

I cleared the build cache and re-running 👍

dawhitla · 2023-08-21T18:55:57Z

I cleared the build cache and re-running 👍

Native stacktrace dump:
java.lang.UnsatisfiedLinkError: couldn't find DSO to load: libhermes.so

currently tracking down the root cause here when trying to run the build on emulator.

arthurgeron-work · 2023-08-21T19:10:05Z

I cleared the build cache and re-running 👍
Native stacktrace dump:
java.lang.UnsatisfiedLinkError: couldn't find DSO to load: libhermes.so
currently tracking down the root cause here when trying to run the build on emulator.

It looks like Soloader is missing, I've noticed it isn't specified in build.gradle's dependencies field, could that be it?

I'm not sure if there are other required steps to run detox tests aside from a specific AVD name, but I wasn't able to run it locally, unfortunately.

dawhitla · 2023-08-24T16:50:29Z

I cleared the build cache and re-running 👍
Native stacktrace dump:
java.lang.UnsatisfiedLinkError: couldn't find DSO to load: libhermes.so
currently tracking down the root cause here when trying to run the build on emulator.
It looks like Soloader is missing, I've noticed it isn't specified in build.gradle's dependencies field, could that be it?

I'm not sure if there are other required steps to run detox tests aside from a specific AVD name, but I wasn't able to run it locally, unfortunately.

Considering we haven't been able to determine the root cause here, can we split the RN 0.73 compatibility into its own PR ?
I'd like to get all the PiP related work in and stable to reduce the amount of in-flight changes we have right now.

dawhitla · 2023-08-24T17:09:25Z

I'm not sure if there are other required steps to run detox tests aside from a specific AVD name, but I wasn't able to run it locally, unfortunately.

Can you let me know what is blocking you? Currently it expects the emulator to already be running before running

for build
e2e:build:android:release
start emulator, then run
e2e:test:android:release

adds control over pip behavior, can be enabled or disabled

arthurgeron · 2023-08-24T19:42:38Z

I cleared the build cache and re-running 👍
Native stacktrace dump:
java.lang.UnsatisfiedLinkError: couldn't find DSO to load: libhermes.so
currently tracking down the root cause here when trying to run the build on emulator.
It looks like Soloader is missing, I've noticed it isn't specified in build.gradle's dependencies field, could that be it?
I'm not sure if there are other required steps to run detox tests aside from a specific AVD name, but I wasn't able to run it locally, unfortunately.
Considering we haven't been able to determine the root cause here, can we split the RN 0.73 compatibility into its own PR ? I'd like to get all the PiP related work in and stable to reduce the amount of in-flight changes we have right now.

Agreed! FYI I've merged the changes and fixes related to PIP into a single commit.
PR for the RN 0.73 upgrade: #158

arthurgeron · 2023-08-29T14:03:22Z

@dawhitla I've fixed the android build errors, all checks are now green.

dawhitla

Thank you for the patience on this 👍

dawhitla reviewed Aug 16, 2023

View reviewed changes

src/index.ts Outdated Show resolved Hide resolved

dawhitla reviewed Aug 16, 2023

View reviewed changes

src/IVSPlayer.tsx Show resolved Hide resolved

dawhitla requested changes Aug 16, 2023

View reviewed changes

src/IVSPlayer.tsx Outdated Show resolved Hide resolved

android/src/main/java/com/amazonaws/ivs/reactnative/player/AmazonIvsView.kt Outdated Show resolved Hide resolved

android/src/main/AndroidManifest.xml Outdated Show resolved Hide resolved

arthurgeron requested a review from dawhitla August 18, 2023 15:05

dawhitla reviewed Aug 18, 2023

View reviewed changes

arthurgeron force-pushed the feat/pip_rn-0.73 branch from a880bed to 4cb711b Compare August 18, 2023 23:06

fix: picture in picture always opening

8dc65a9

adds control over pip behavior, can be enabled or disabled

arthurgeron force-pushed the feat/pip_rn-0.73 branch from 4cb711b to 8dc65a9 Compare August 24, 2023 19:40

arthurgeron changed the title ~~RN 0.73 support & Picture-in-Picture Fix~~ Picture-in-Picture Fix Aug 24, 2023

arthurgeron requested a review from dawhitla August 24, 2023 19:45

fix: duplicate import

87101a6

dawhitla approved these changes Aug 29, 2023

View reviewed changes

dawhitla merged commit 9ce7e60 into aws:main Aug 29, 2023

dawhitla mentioned this pull request Feb 16, 2024

Release/1.5.0 #175

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Picture-in-Picture Fix #151

Picture-in-Picture Fix #151

arthurgeron commented Aug 10, 2023 •

edited

Loading

dawhitla left a comment

arthurgeron commented Aug 18, 2023

dawhitla Aug 18, 2023

dawhitla Aug 18, 2023

arthurgeron Aug 18, 2023

arthurgeron-work commented Aug 19, 2023

dawhitla commented Aug 21, 2023

dawhitla commented Aug 21, 2023

arthurgeron-work commented Aug 21, 2023

dawhitla commented Aug 24, 2023

dawhitla commented Aug 24, 2023

arthurgeron commented Aug 24, 2023 •

edited

Loading

arthurgeron commented Aug 29, 2023

dawhitla left a comment

Picture-in-Picture Fix #151

Picture-in-Picture Fix #151

Conversation

arthurgeron commented Aug 10, 2023 • edited Loading

dawhitla left a comment

Choose a reason for hiding this comment

arthurgeron commented Aug 18, 2023

dawhitla Aug 18, 2023

Choose a reason for hiding this comment

dawhitla Aug 18, 2023

Choose a reason for hiding this comment

arthurgeron Aug 18, 2023

Choose a reason for hiding this comment

arthurgeron-work commented Aug 19, 2023

dawhitla commented Aug 21, 2023

dawhitla commented Aug 21, 2023

arthurgeron-work commented Aug 21, 2023

dawhitla commented Aug 24, 2023

dawhitla commented Aug 24, 2023

arthurgeron commented Aug 24, 2023 • edited Loading

arthurgeron commented Aug 29, 2023

dawhitla left a comment

Choose a reason for hiding this comment

arthurgeron commented Aug 10, 2023 •

edited

Loading

arthurgeron commented Aug 24, 2023 •

edited

Loading