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 basic cast support #64

Merged
merged 31 commits into from
Oct 25, 2023
Merged

Add basic cast support #64

merged 31 commits into from
Oct 25, 2023

Conversation

krocard
Copy link
Contributor

@krocard krocard commented Oct 9, 2023

This introduces Basic cast support for Flutter. The API is align as much as possible with the React Native SDK.

New API

  • BitmovinCastManager
  • player cast methods (castVideo, castStop, isCasting...)

Not yet supported

  • Cast events

Issues encountered

  • The Cast SDK expects an AppCompat Activity (FragmentActivity), which the recommended FlutterActivity is not.
    • MainActivity now extends FlutterFragmentActivity
  • The Cast SDK expects an AppCompat style
    • The Theme.AppCompat.NoActionBar is now used instead of LaunchApp
  • com.bitmovin.player.casting.ExpandedControllerActivity was not accessible in the sample app
    • Made the player an api dependency

Design

BitmovinPlayerManager

Flutter's Player is a plain object with an async initialization. Aka, the Dart Player object is created immediately, but it's resources (native player, channels...) are created in the background. The client doesn't know when it is done 1.
This was chosen for convenience as Dart has no async constructor.

The drawback of this approach is that the client code doesn't control the initialization sequence of the object explitly.

Given that the BitmovinPlayerManager must be created before the Player, the same patter is not applicable. Instead the BitmovinPlayerManager singleton is accessible through an async static create factory. This ensure that the client can initialize it reliably before creating a Player.

Nevertheless as the player creation is not async, mixing the async and sync code is not very convenient and leads to the cast use case needing a more complex setup than the non cast use-case. This is reflected in the complexity of the cast sample included in this PR.

One possible solution to avoid this would be to implicitly create the cast manager on the platform side if the player config has cast enabled set to true. Nevertheless this is no inlined with our other SDKs, which need explicit initialization of the BitmovinCastManager. As such this alternative has not been implemented, but feedback on this proposal is welcomed.

BitmovinPlayerManager.updateContext

updateContext is supposed to be called for any new activity. Nevertheless the Flutter native plugin already gets a callback on Activity attach and detach. This PR has updateContext called automatically if the BitmovinPlayerManager is initialized.

As such there should be no need to call updateContext from the Dart side. It is still exposed for alignment with React Native and because I didn't full understand the Flutter plugin Activity lifecycle when I started this PR.

I would recommend removing updateContext from the Dart API, and would welcome feedback on this proposal.

Footnotes

  1. The async part is implicitly awaited when an async method of the player is called. This is an implementation detail that should not be relied on by the client.

@krocard krocard self-assigned this Oct 9, 2023
@krocard krocard marked this pull request as ready for review October 9, 2023 17:58
@krocard
Copy link
Contributor Author

krocard commented Oct 10, 2023

@rolandkakonyi @strangesource Given that you did the React Native implementation, feel free to take a look at the Flutter one too.
Note that we will wait for @hawk23's review to merge.

Copy link
Contributor

@strangesource strangesource left a comment

Choose a reason for hiding this comment

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

Disclaimer: This review is very superficial as I haven't touched the Flutter SDK yet.

If casting is initialized for one example I think it is available in all samples. In order to avoid this somewhat confusing behaviour we should disable casting in the existing samples.

Co-authored-by: Lukas Knoch-Girstmair <[email protected]>
Copy link
Collaborator

@rolandkakonyi rolandkakonyi left a comment

Choose a reason for hiding this comment

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

I literally have no idea what is going on. I never touched Flutter.
The Android part looks good as far as I can tell.

Copy link
Contributor

@matamegger matamegger left a comment

Choose a reason for hiding this comment

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

From my side nothing to add to the other reviews. I only shallowly looked over the code, this looks fine, but I basically have no experience with flutter (yet)

Copy link
Collaborator

@hawk23 hawk23 left a comment

Choose a reason for hiding this comment

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

Wow, big feature! 🎉

Some general things I noticed:

  • There are some *.g.dart files missing in this PR
  • I saw the Bitmovin UI showing "A general error occurred" a couple of times during manual testing. I do not have steps to reproduce though. Once I tried to reproduce it manually it never happened again. Maybe the stack trace from below helps
E/MediaCodecVideoRenderer(22389): Video codec error
E/MediaCodecVideoRenderer(22389):   java.lang.IllegalStateException
E/MediaCodecVideoRenderer(22389):       at android.media.MediaCodec.native_dequeueInputBuffer(Native Method)
E/MediaCodecVideoRenderer(22389):       at android.media.MediaCodec.dequeueInputBuffer(MediaCodec.java:2726)
E/MediaCodecVideoRenderer(22389):       at com.bitmovin.android.exoplayer2.mediacodec.SynchronousMediaCodecAdapter.dequeueInputBufferIndex(SynchronousMediaCodecAdapter.java:99)
E/MediaCodecVideoRenderer(22389):       at com.bitmovin.android.exoplayer2.mediacodec.MediaCodecRenderer.feedInputBuffer(MediaCodecRenderer.java:1180)
E/MediaCodecVideoRenderer(22389):       at com.bitmovin.android.exoplayer2.mediacodec.MediaCodecRenderer.render(MediaCodecRenderer.java:775)
E/MediaCodecVideoRenderer(22389):       at com.bitmovin.android.exoplayer2.ExoPlayerImplInternal.doSomeWork(ExoPlayerImplInternal.java:1017)
E/MediaCodecVideoRenderer(22389):       at com.bitmovin.android.exoplayer2.ExoPlayerImplInternal.handleMessage(ExoPlayerImplInternal.java:512)
E/MediaCodecVideoRenderer(22389):       at android.os.Handler.dispatchMessage(Handler.java:106)
E/MediaCodecVideoRenderer(22389):       at android.os.Looper.loop(Looper.java:219)
E/MediaCodecVideoRenderer(22389):       at android.os.HandlerThread.run(HandlerThread.java:67)
E/ExoPlayerImplInternal(22389): Playback error
E/ExoPlayerImplInternal(22389):   com.bitmovin.android.exoplayer2.ExoPlaybackException: MediaCodecVideoRenderer error, index=0, format=Format(1080_4800000, null, null, video/avc, avc1.4D4032, 4800000, null, [1920, 1080, 25.0], [-1, -1]), format_supported=YES
E/ExoPlayerImplInternal(22389):       at com.bitmovin.android.exoplayer2.ExoPlayerImplInternal.handleMessage(ExoPlayerImplInternal.java:588)
E/ExoPlayerImplInternal(22389):       at android.os.Handler.dispatchMessage(Handler.java:106)
E/ExoPlayerImplInternal(22389):       at android.os.Looper.loop(Looper.java:219)
E/ExoPlayerImplInternal(22389):       at android.os.HandlerThread.run(HandlerThread.java:67)
E/ExoPlayerImplInternal(22389):   Caused by: com.bitmovin.android.exoplayer2.video.MediaCodecVideoDecoderException: Decoder failed: OMX.hisi.video.decoder.avc
E/ExoPlayerImplInternal(22389):       at com.bitmovin.android.exoplayer2.video.MediaCodecVideoRenderer.createDecoderException(MediaCodecVideoRenderer.java:1639)
E/ExoPlayerImplInternal(22389):       at com.bitmovin.android.exoplayer2.mediacodec.MediaCodecRenderer.render(MediaCodecRenderer.java:794)
E/ExoPlayerImplInternal(22389):       at com.bitmovin.android.exoplayer2.ExoPlayerImplInternal.doSomeWork(ExoPlayerImplInternal.java:1017)
E/ExoPlayerImplInternal(22389):       at com.bitmovin.android.exoplayer2.ExoPlayerImplInternal.handleMessage(ExoPlayerImplInternal.java:512)
E/ExoPlayerImplInternal(22389):       ... 3 more
E/ExoPlayerImplInternal(22389):   Caused by: java.lang.IllegalStateException
E/ExoPlayerImplInternal(22389):       at android.media.MediaCodec.native_dequeueInputBuffer(Native Method)
E/ExoPlayerImplInternal(22389):       at android.media.MediaCodec.dequeueInputBuffer(MediaCodec.java:2726)
E/ExoPlayerImplInternal(22389):       at com.bitmovin.android.exoplayer2.mediacodec.SynchronousMediaCodecAdapter.dequeueInputBufferIndex(SynchronousMediaCodecAdapter.java:99)
E/ExoPlayerImplInternal(22389):       at com.bitmovin.android.exoplayer2.mediacodec.MediaCodecRenderer.feedInputBuffer(MediaCodecRenderer.java:1180)
E/ExoPlayerImplInternal(22389):       at com.bitmovin.android.exoplayer2.mediacodec.MediaCodecRenderer.render(MediaCodecRenderer.java:775)
E/ExoPlayerImplInternal(22389):       ... 5 more
I/HwExtendedUtils(22389): Set to window composer mode as 1
I/ACodec  (22389): gralloc usage: 0x80000000(OMX) => 0x80002900(ACodec)
D/SurfaceUtils(22389): disconnecting from surface 0x7450fea010, reason setNativeWindowSizeFormatAndUsage
E/ExoPlayerImplInternal(22389): Disable failed.
E/ExoPlayerImplInternal(22389):   java.lang.IllegalStateException
E/ExoPlayerImplInternal(22389):       at android.media.MediaCodec.native_flush(Native Method)
E/ExoPlayerImplInternal(22389):       at android.media.MediaCodec.flush(MediaCodec.java:2194)
E/ExoPlayerImplInternal(22389):       at com.bitmovin.android.exoplayer2.mediacodec.SynchronousMediaCodecAdapter.flush(SynchronousMediaCodecAdapter.java:166)
E/ExoPlayerImplInternal(22389):       at com.bitmovin.android.exoplayer2.mediacodec.MediaCodecRenderer.flushCodec(MediaCodecRenderer.java:858)
E/ExoPlayerImplInternal(22389):       at com.bitmovin.android.exoplayer2.mediacodec.MediaCodecRenderer.flushOrReleaseCodec(MediaCodecRenderer.java:851)
E/ExoPlayerImplInternal(22389):       at com.bitmovin.android.exoplayer2.mediacodec.MediaCodecRenderer.onDisabled(MediaCodecRenderer.java:691)
E/ExoPlayerImplInternal(22389):       at com.bitmovin.android.exoplayer2.video.MediaCodecVideoRenderer.onDisabled(MediaCodecVideoRenderer.java:609)
E/ExoPlayerImplInternal(22389):       at com.bitmovin.android.exoplayer2.BaseRenderer.disable(BaseRenderer.java:186)
E/ExoPlayerImplInternal(22389):       at com.bitmovin.android.exoplayer2.ExoPlayerImplInternal.disableRenderer(ExoPlayerImplInternal.java:1692)
E/ExoPlayerImplInternal(22389):       at com.bitmovin.android.exoplayer2.ExoPlayerImplInternal.resetInternal(ExoPlayerImplInternal.java:1426)
E/ExoPlayerImplInternal(22389):       at com.bitmovin.android.exoplayer2.ExoPlayerImplInternal.stopInternal(ExoPlayerImplInternal.java:1387)
E/ExoPlayerImplInternal(22389):       at com.bitmovin.android.exoplayer2.ExoPlayerImplInternal.handleMessage(ExoPlayerImplInternal.java:604)
E/ExoPlayerImplInternal(22389):       at android.os.Handler.dispatchMessage(Handler.java:106)
E/ExoPlayerImplInternal(22389):       at android.os.Looper.loop(Looper.java:219)
E/ExoPlayerImplInternal(22389):       at android.os.HandlerThread.run(HandlerThread.java:67)
E/SurfaceUtils(22389): Failed to disconnect from surface 0x7450fea010, err -19
E/SurfaceUtils(22389): nativeWindowDisconnect failed: No such device (19)
E/ACodec  (22389): Failed to allocate output port buffers after port reconfiguration: (-19)
E/ACodec  (22389): signalError(omxError 0x80001001, internalError -19)
E/ACodec  (22389): Error occurred while disabling the output port
E/MediaCodec(22389): Codec reported err 0xffffffed, actionCode 0, while in state 0
I/M

lib/src/casting/casting_config.dart Outdated Show resolved Hide resolved
lib/src/api/casting/casting_config.dart Outdated Show resolved Hide resolved
example/android/app/build.gradle Outdated Show resolved Hide resolved
example/lib/main.dart Outdated Show resolved Hide resolved
lib/src/api/casting/casting_config.dart Outdated Show resolved Hide resolved
lib/src/api/player/player_api.dart Show resolved Hide resolved
lib/src/api/player/player_api.dart Outdated Show resolved Hide resolved
lib/src/api/player/player_config.dart Outdated Show resolved Hide resolved
lib/src/player.dart Outdated Show resolved Hide resolved
@hawk23 hawk23 mentioned this pull request Oct 20, 2023
@krocard krocard requested review from hawk23 and strangesource and removed request for strangesource October 24, 2023 09:41
Copy link
Collaborator

@hawk23 hawk23 left a comment

Choose a reason for hiding this comment

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

Nice! Just some minor comments so approving already. The CI should also pass once those are addressed

lib/bitmovin_player.dart Outdated Show resolved Hide resolved
lib/src/api/player/player_config.dart Outdated Show resolved Hide resolved
example/lib/pages/casting.dart Outdated Show resolved Hide resolved
@krocard krocard merged commit ab0af2f into main Oct 25, 2023
4 checks passed
@krocard krocard deleted the PFL-69/fix-casting-support branch October 25, 2023 12:38
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.

5 participants