-
Notifications
You must be signed in to change notification settings - Fork 3
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
Conversation
PFL-69
PFL-69
PFL-69
PFL-69
PFL-69
@rolandkakonyi @strangesource Given that you did the React Native implementation, feel free to take a look at the Flutter one too. |
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.
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.
example/android/app/src/main/kotlin/com/bitmovin/player/flutter/example/MainActivity.kt
Outdated
Show resolved
Hide resolved
Co-authored-by: Lukas Knoch-Girstmair <[email protected]>
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 literally have no idea what is going on. I never touched Flutter.
The Android part looks good as far as I can tell.
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.
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)
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.
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
Co-authored-by: Mr. Graf <[email protected]>
Co-authored-by: Mr. Graf <[email protected]>
Co-authored-by: Mr. Graf <[email protected]>
… fix-casting-support
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.
Nice! Just some minor comments so approving already. The CI should also pass once those are addressed
Co-authored-by: Mr. Graf <[email protected]>
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
Issues encountered
Activity
(FragmentActivity
), which the recommendedFlutterActivity
is not.MainActivity
now extendsFlutterFragmentActivity
Theme.AppCompat.NoActionBar
is now used instead ofLaunchApp
com.bitmovin.player.casting.ExpandedControllerActivity
was not accessible in the sample appapi
dependencyDesign
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 thePlayer
, the same patter is not applicable. Instead theBitmovinPlayerManager
singleton is accessible through an async staticcreate
factory. This ensure that the client can initialize it reliably before creating aPlayer
.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 hasupdateContext
called automatically if theBitmovinPlayerManager
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
The async part is implicitly
await
ed when an async method of the player is called. This is an implementation detail that should not be relied on by the client. ↩