feat: add notification controls

[KrzysztofMoch]

Summary

Add showNotificationControls prop to Android & iOS

Motivation

Allow to control video from notification controls

Test plan

• Tested locally
• CI passes

[zoriya]

Speaking for android: why do you use custom Notifications with custom actions instead of using the standard MediaSessionCompat and MediaSessionConnector? Those two classes create system's media notifications and listen to hardware volume/media buttons (like headphones or remotes) and assistant on TV.

[KrzysztofMoch]

I want in feature to allow set custom actions

[zoriya]

You can use custom actions with the built-in media session classes: https://developer.android.com/codelabs/supporting-mediasession#4

I believe the built-in one has a huge value, especially to run react-native-video on android tv.

[KrzysztofMoch]

👀 thanks for info, will check out this

[freeboub]

A lot of small comments, but nothing big to change.
I am just doubting about metadata source. I have never see on my project video with metadata. I really think we should fallback on field provided by the application.
You have 2 choices:

• either use metadata from the source (quick low impact, but not dynamic during playback, and not yet implemented on android)
• either create a new metadata field in the props to allow supporting dynamic update.

And thank you for the PR !

[KrzysztofMoch]

@zoriya seems like MediaSessionConnector from exoplayer is deprecated in favour of MediaSession from media3

I will check if with current implementation, tv pilots will work (I will test it via adb)

[zoriya]

Yes, I tried to use MediaSession from media3 in the draft PR. Is there a reason we can't use it?

[KrzysztofMoch]

We don't have exoplayer2 any more as we migrated to media3

[KrzysztofMoch]

I have tested and with current implementation we support key events like 126, 85 so I guess with this TV should be supported ?

[zoriya]

Yes, since everything was migrated to media3 I don't see why we can't simply use: androidx.media3:media3-session:$media3_version

[KrzysztofMoch]

We are using it
https://github.com/react-native-video/react-native-video/blob/edbb3b60aab0085ccd047f69bd04e7898a0b62a1/android/build.gradle#L226-L229

And I am using MediaSession from androidx.media3:media3-session (you can look into imports in VideoPlaybackService) as MediaSessionConnector is no longer available in media3

[zoriya]

Okay, I'm sorry I was referring to outdated docs. I got the right one now.

I don't understand why you create a notification manually. media3's session support notifications out of the box: https://developer.android.com/media/media3/session/background-playback#notification

[KrzysztofMoch]

@freeboub I don't see a way to dynamically update metadata... for now I am using player.replaceMediaItem but this resets video. I also have tried to update notification content when setting metadata but "content" is lost when you do any action (like pause, seek)

[freeboub]

@freeboub I don't see a way to dynamically update metadata... for now I am using player.replaceMediaItem but this resets video. I also have tried to update notification content when setting metadata but "content" is lost when you do any action (like pause, seek)

Ok, thank you for double checking !

[KrzysztofMoch]

So I guess in this case will be better to move prop metadata to source ? or should I leave it outside

[KrzysztofMoch]

@Duell10111 ping for you as I made breaking change for your code for custom metadata. I moved propreties for user metadata from source to source.metadata to make it more readable

[KrzysztofMoch]

@freeboub issue with "metadata reset changing source" should be fixed now (now I am disabling notification controls for player with "no media item" (mostly invalid url/source no view etc.)

[KrzysztofMoch]

I have tested iOS simulator and it does not support "NowPlayingInfo" notification so I added to docs info that fot iOS you need to run on real device

[YangJonghun]

@KrzysztofMoch
#3723 is awesome! however, prop name seems narrow in scope compared to the actual functionality.
MediaSession is not only about controlling notifications and showing metadata about the video, but it's also about synchronizing processing for external controls (airplay, car, air pod, remote controller etc..).
But it's hard to tell from the prop name if it covers this functionality.

[KrzysztofMoch]

@YangJonghun you right, I had not better idea then - we can add this information to docs maybe ?

[freeboub]

@YangJonghun you right, I had not better idea then - we can add this information to docs maybe ?

I agree it is a good idea!

[zoriya]

Custom notifications require editing the AndroidManifest.xml (as stated in the readme), would it be possible to include those changes on the repository, so expo could automatically apply them? Should I make a new issue to track that?

[freeboub]

@zoriya Your request make sens, but I am 100% sure if need, can you please open a ticket or discussion to discuss about it with @KrzysztofMoch ? (or we can continue here ... ).
The issue of integrating it in manifest (move it by default to: android/src/main/AndroidManifest.xml) is that it will make the feature mandatory.
I don't really care about service declaration (It can be move without side effects). but I worry about new permissions:

<uses-permission android:name="android.permission.FOREGROUND_SERVICE" />
<uses-permission android:name="android.permission.FOREGROUND_SERVICE_MEDIA_PLAYBACK" />

Today, if user doesn't want the feature and don't add it his manifest, will the app still work ?

[zoriya]

That's true, it will always request for permissions. I'm not sure if media notifications is a permission that require user consent or if it is automatically granted.

I think this should be opt-out more than opt-in tho, as users, we expect to be able to control media using media buttons.

[YangJonghun]

@KrzysztofMoch @freeboub @zoriya
I think we can make people choose changeable opt-in.

some of the Player's features require permissions, services, and activities.

• service & FOREGROUND_SERVICE(permission) : download, media session, google cast ...
• activity : picture in picture ...

Currently, If someone need these features, they have to write within their AndroidManifest.xml. because Android doesn't support programmatically generate Service or Activity
But, If we write each feature as native Android dependency, people can choose and install with native flag like below

    if (ExoplayerDependencies["useReactNativeVideoPIP"]) {
      // For PictureInPIcture support with react-native-video
      implementation "com.react-native-video.picture-in-picture:$rnv_version"
    }
    if (ExoplayerDependencies["useReactNativeVideoMediaSession"]) {
      // For MediaSession support with react-native-video
      implementation "com.react-native-video.media-session:$rnv_version"
    }

(As far as I know, react-native-notifee's app.notifee.core takes that approach, which allows it to get around some of the limitations of RN library.)

fyi) if we choose this approach, #3385 can be rewrite with Activity.

[freeboub]

@YangJonghun Thank you for the clue.
We can also imagine another package react-native-video-media-session which would be responsible of the feature.
BTW, I think the real question we need to answer is should we want to remove mediaSession from the app ?!

jetified-media3-session-1.3.1.aar: 506K
And same question can come for UI:
jetified-media3-ui-1.1.1.aar: 419K

I am not 100% convinced yet of this necessity :/
Maybe moving the tags in the manifest can be enough for now ...

[YangJonghun]

@freeboub
I don't care about the size of the package, I want the library to support users to specify only the permissions for the features they actually use (because Play Store has strict permission checks for background behavior).

There are two ways to do this: opt-in and opt-out, and currently we choose opt-out, where users specify the permissions and services they need in their code to use certain features.

My hypothesis is that react-native-video supports opt-in to improve library user's DX. If it's a regular React-Native library, not a native dependency, you can't specify the service, activity, permission, etc. required for the feature separately.

I think opt-in and opt-out are both good!👍

[freeboub]

As someone said and remove his comment: "You may not but other developers are care about package size"
That is not the main topic for now :)
Regarding to optin vs optout, I think you choose the good strategy to avoid changing default behavior of the package !
Regarding to the 2 new permissions, I don't think they change anything for store submission even if the feature is not used ? What's why I wonder if we can add it to the AndroidManifest from the package.

[YangJonghun]

Recently, Google Play Console has been requiring proof of app content actually using Foreground Service, so if someone not using it and only have the permission added, this could be a reason for rejection.
fyi) https://developer.android.com/about/versions/14/changes/fgs-types-required#google-play-enforcement

[freeboub]

@YangJonghun Thank you for the information ! So I think we should keep implementation as is !

[YangJonghun]

@KrzysztofMoch
When the playInBackground prop is false, I was expecting the notification control to exit as well, but currently it doesn't behave that way. Is there any plans to change it?

[KrzysztofMoch]

On iOS or Android ?

[YangJonghun]

@KrzysztofMoch
I tested on Android

[KrzysztofMoch]

@KrzysztofMoch When the playInBackground prop is false, I was expecting the notification control to exit as well, but currently it doesn't behave that way. Is there any plans to change it?

I think it makes sense - will try to care of it next week