-
Notifications
You must be signed in to change notification settings - Fork 16
Feat/video adaptation #596
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
base: development
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -2,6 +2,14 @@ | |||||
|
|
||||||
| ## [Unreleased] | ||||||
|
|
||||||
| ### Changed | ||||||
|
|
||||||
| - Android: Refactored AdaptationConfig to enable the `onVideoAdaptation` callback | ||||||
|
|
||||||
| ### Added | ||||||
|
|
||||||
| - Android: Added support for the `onVideoAdaptation` callback | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What do you think about this?
Suggested change
|
||||||
|
|
||||||
| ## [0.36.0] - 2024-12-20 | ||||||
|
|
||||||
| ### Changed | ||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,99 @@ | ||
| package com.bitmovin.player.reactnative | ||
|
|
||
| import android.util.Log | ||
| import androidx.concurrent.futures.CallbackToFutureAdapter | ||
| import androidx.concurrent.futures.CallbackToFutureAdapter.Completer | ||
| import com.bitmovin.player.api.media.AdaptationConfig | ||
| import com.bitmovin.player.api.media.video.quality.VideoAdaptation | ||
| import com.bitmovin.player.api.media.video.quality.VideoAdaptationData | ||
| import com.bitmovin.player.reactnative.converter.toAdaptationConfig | ||
| import com.bitmovin.player.reactnative.converter.toJson | ||
| import com.facebook.react.bridge.* | ||
| import com.facebook.react.module.annotations.ReactModule | ||
| import java.lang.Exception | ||
| import java.util.concurrent.ConcurrentHashMap | ||
| import java.util.concurrent.Future | ||
| import java.util.concurrent.TimeUnit | ||
|
|
||
| private const val MODULE_NAME = "AdaptationModule" | ||
|
|
||
| @ReactModule(name = MODULE_NAME) | ||
| class AdaptationModule(context: ReactApplicationContext) : BitmovinBaseModule(context) { | ||
| /** | ||
| * In-memory mapping from `nativeId`s to `AdaptationConfig` instances. | ||
| */ | ||
| private val adaptationConfigs: Registry<AdaptationConfig> = mutableMapOf() | ||
| private val onVideoAdaptationCompleters = ConcurrentHashMap<String, Completer<String>>() | ||
|
|
||
| override fun getName() = MODULE_NAME | ||
|
|
||
| fun getConfig(nativeId: NativeId?): AdaptationConfig? = nativeId?.let { adaptationConfigs[it] } | ||
|
|
||
| @ReactMethod | ||
| fun initWithConfig(nativeId: NativeId, config: ReadableMap, promise: Promise) { | ||
| promise.unit.resolveOnUiThread { | ||
| if (adaptationConfigs.containsKey(nativeId)) { | ||
| return@resolveOnUiThread | ||
| } | ||
| val adaptationConfig = config.toAdaptationConfig() | ||
| adaptationConfigs[nativeId] = adaptationConfig | ||
| initConfigBlocks(nativeId, config) | ||
| } | ||
| } | ||
|
|
||
| @ReactMethod | ||
| fun destroy(nativeId: NativeId) { | ||
| adaptationConfigs.remove(nativeId) | ||
| onVideoAdaptationCompleters.keys.filter { it.startsWith(nativeId) }.forEach { | ||
| onVideoAdaptationCompleters.remove(it) | ||
| } | ||
| } | ||
|
|
||
| private fun initConfigBlocks(nativeId: String, config: ReadableMap) { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would just directly call |
||
| initVideoAdaptationData(nativeId, adaptationConfigJson = config) | ||
| } | ||
|
|
||
| private fun initVideoAdaptationData(nativeId: NativeId, adaptationConfigJson: ReadableMap) { | ||
| val adaptationConfig = getConfig(nativeId) ?: return | ||
| if (!adaptationConfigJson.hasKey("videoAdaptation")) return | ||
|
|
||
| adaptationConfig.videoAdaptation = VideoAdaptation { data -> | ||
| val future = onVideoAdaptationFromJS(nativeId, data) | ||
| try { | ||
| val callbackValue = future.get(1, TimeUnit.SECONDS) // set timeout to mimize playback performance impact | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Have you seen problems with the promise taking longer to resolve? Is this really needed, I don't see a reason why this could take more than a second, do we really need this? If so, we might want to consider if we need this also in other similar APIs like |
||
| callbackValue | ||
| } catch (e: Exception) { | ||
| Log.e(MODULE_NAME, "custom RN onVideoAdaptation exception $e. Using default of ${data.suggested}") | ||
| data.suggested | ||
| } | ||
| } | ||
| } | ||
|
|
||
| private fun onVideoAdaptationFromJS( | ||
| nativeId: NativeId, | ||
| data: VideoAdaptationData, | ||
| ): Future<String> { | ||
| val onVideoAdaptationId = "$nativeId@${System.identityHashCode(data)}" | ||
| val args = Arguments.createArray() | ||
| args.pushString(onVideoAdaptationId) | ||
| args.pushMap(data.toJson()) | ||
|
|
||
| return CallbackToFutureAdapter.getFuture { completer -> | ||
| onVideoAdaptationCompleters[onVideoAdaptationId] = completer | ||
| context.catalystInstance.callFunction("Adaptation-$nativeId", "onVideoAdaptation", args as NativeArray) | ||
| } | ||
| } | ||
|
|
||
| @ReactMethod | ||
| fun setOnVideoAdaptation(onVideoAdaptationId: String, data: String) { | ||
| val completer = onVideoAdaptationCompleters.remove(onVideoAdaptationId) | ||
| if (completer == null) { | ||
| Log.e( | ||
| MODULE_NAME, | ||
| "Completer is null for onVideoAdaptationId: $onVideoAdaptationId, this can cause adaptation errors", | ||
| ) | ||
| return | ||
| } | ||
| completer.set(data) | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -36,6 +36,7 @@ import com.bitmovin.player.api.media.audio.AudioTrack | |
| import com.bitmovin.player.api.media.subtitle.SubtitleTrack | ||
| import com.bitmovin.player.api.media.thumbnail.Thumbnail | ||
| import com.bitmovin.player.api.media.thumbnail.ThumbnailTrack | ||
| import com.bitmovin.player.api.media.video.quality.VideoAdaptationData | ||
| import com.bitmovin.player.api.media.video.quality.VideoQuality | ||
| import com.bitmovin.player.api.network.HttpRequest | ||
| import com.bitmovin.player.api.network.HttpRequestType | ||
|
|
@@ -144,11 +145,17 @@ private fun String.toTimelineReferencePoint(): TimelineReferencePoint? = when (t | |
| /** | ||
| * Converts an arbitrary `json` to `AdaptationConfig`. | ||
| */ | ||
| private fun ReadableMap.toAdaptationConfig(): AdaptationConfig = AdaptationConfig().apply { | ||
| fun ReadableMap.toAdaptationConfig(): AdaptationConfig = AdaptationConfig().apply { | ||
| withInt("maxSelectableBitrate") { maxSelectableVideoBitrate = it } | ||
| withInt("initialBandwidthEstimateOverride") { initialBandwidthEstimateOverride = it.toLong(); } | ||
| } | ||
|
|
||
| fun ReadableMap.toVideoAdaptationData(): VideoAdaptationData? { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is not used, do is it a leftover or are we missing something? |
||
| return VideoAdaptationData( | ||
| getString("suggested") ?: return null, | ||
| ) | ||
| } | ||
|
|
||
| /** | ||
| * Converts any JS object into a `PlaybackConfig` object. | ||
| */ | ||
|
|
@@ -905,6 +912,10 @@ fun RNBufferLevels.toJson(): WritableMap = Arguments.createMap().apply { | |
| putMap("video", video.toJson()) | ||
| } | ||
|
|
||
| fun VideoAdaptationData.toJson(): WritableMap = Arguments.createMap().apply { | ||
| putString("suggested", suggested) | ||
| } | ||
|
|
||
| /** | ||
| * Maps a JS string into the corresponding [BufferType] value. | ||
| */ | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,52 @@ | ||
| import { NativeInstanceConfig } from '../nativeInstance'; | ||
|
|
||
| /** | ||
| * Can be implemented and added to the AdaptationConfig to customize the video adaptation logic. | ||
| * | ||
| * @platform Android | ||
| */ | ||
| export interface VideoAdaptation { | ||
| /** | ||
| * Is called before the next video segment is downloaded. The quality according to VideoQuality.id that is returned will be downloaded next. Invalid IDs or null will result in a fallback to the ID provided in data. | ||
| * | ||
| * @platform Android | ||
| * @see https://cdn.bitmovin.com/player/android/3/docs/player-core/com.bitmovin.player.api.media.video.quality/-video-adaptation/on-video-adaptation.html | ||
| */ | ||
| onVideoAdaptation?: (data: VideoAdaptationData) => Promise<string>; | ||
| } | ||
|
|
||
| /** | ||
| * Holds information about the current video adaptation. | ||
| * | ||
| * @platform Android | ||
| */ | ||
| export interface VideoAdaptationData { | ||
| suggested?: string; | ||
| } | ||
|
|
||
| /** | ||
| * Configures the adaptation logic. | ||
| */ | ||
| export interface AdaptationConfig extends NativeInstanceConfig { | ||
| /** | ||
| * The upper bitrate boundary in bits per second for approximate network bandwidth consumption of the played source. | ||
| * Can be set to `undefined` for no limitation. | ||
| */ | ||
| maxSelectableBitrate?: number; | ||
|
|
||
| /** | ||
| * The initial bandwidth estimate in bits per second the player uses to select the optimal media tracks before actual bandwidth data is available. Overriding this value should only be done in specific cases and will most of the time not result in better selection logic. | ||
| * | ||
| * @platform Android | ||
| * @see https://cdn.bitmovin.com/player/android/3/docs/player-core/com.bitmovin.player.api.media/-adaptation-config/initial-bandwidth-estimate-override.html | ||
| */ | ||
| initialBandwidthEstimateOverride?: number; | ||
|
|
||
| /** | ||
| * A callback to customize the player's adaptation logic. VideoAdaptation.onVideoAdaptation is called before the player tries to download a new video segment. | ||
| * | ||
| * @platform Android | ||
| * @see https://cdn.bitmovin.com/player/android/3/docs/player-core/com.bitmovin.player.api.media/-adaptation-config/video-adaptation.html | ||
| */ | ||
| videoAdaptation?: VideoAdaptation; | ||
| } |
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,69 @@ | ||||||
| import { NativeModules } from 'react-native'; | ||||||
| import BatchedBridge from 'react-native/Libraries/BatchedBridge/BatchedBridge'; | ||||||
| import NativeInstance from '../nativeInstance'; | ||||||
| import { VideoAdaptationData, AdaptationConfig } from './adaptationConfig'; | ||||||
|
|
||||||
| // Export config types from Adaptation module. | ||||||
| export { VideoAdaptationData, AdaptationConfig }; | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hi @saravanans-github!
Suggested change
The reason is that all exported types should be re-exported on the top-level and now this is not the case. |
||||||
|
|
||||||
| const AdaptationModule = NativeModules.AdaptationModule; | ||||||
|
|
||||||
| /** | ||||||
| * Represents a native Adaptation configuration object. | ||||||
| * @internal | ||||||
| */ | ||||||
| export class Adaptation extends NativeInstance<AdaptationConfig> { | ||||||
| /** | ||||||
| * Whether this object's native instance has been created. | ||||||
| */ | ||||||
| isInitialized = false; | ||||||
| /** | ||||||
| * Whether this object's native instance has been disposed. | ||||||
| */ | ||||||
| isDestroyed = false; | ||||||
|
|
||||||
| /** | ||||||
| * Allocates the Adaptation config instance and its resources natively. | ||||||
| */ | ||||||
| initialize = () => { | ||||||
| if (!this.isInitialized) { | ||||||
| // Register this object as a callable module so it's possible to | ||||||
| // call functions on it from native code, e.g `onVideoAdaptation`. | ||||||
| BatchedBridge.registerCallableModule(`Adaptation-${this.nativeId}`, this); | ||||||
| // Create native configuration object. | ||||||
| AdaptationModule.initWithConfig(this.nativeId, this.config); | ||||||
| this.isInitialized = true; | ||||||
| } | ||||||
| }; | ||||||
|
|
||||||
| /** | ||||||
| * Destroys the native Adaptation config and releases all of its allocated resources. | ||||||
| */ | ||||||
| destroy = () => { | ||||||
| if (!this.isDestroyed) { | ||||||
| AdaptationModule.destroy(this.nativeId); | ||||||
| this.isDestroyed = true; | ||||||
| } | ||||||
| }; | ||||||
|
|
||||||
| /** | ||||||
| * Applies the user-defined `onVideoAdaptation` function to native's `data` and store | ||||||
| * the result back in `AdaptationModule`. | ||||||
| * | ||||||
| * Called from native code when `AdaptationConfig.videoAdaptation` is dispatched. | ||||||
| * | ||||||
| * @param requestId Passed through to identify the completion handler of the request on native. | ||||||
| * @param data The quality according to VideoQuality.id that is returned will be downloaded next. | ||||||
| */ | ||||||
|
|
||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
| onVideoAdaptation = (requestId: string, data: VideoAdaptationData) => { | ||||||
| this.config?.videoAdaptation | ||||||
| ?.onVideoAdaptation?.(data) | ||||||
| .then((resultData: string) => { | ||||||
| AdaptationModule.setOnVideoAdaptation(requestId, resultData); | ||||||
| }) | ||||||
| .catch(() => { | ||||||
| AdaptationModule.setOnVideoAdaptation(requestId, data); | ||||||
| }); | ||||||
| }; | ||||||
| } | ||||||
This file was deleted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this changelog isn't necessary, as the existing api's can be called the same way as before, or am I missing something?