[video_player] Add macOS support

[cbenhagen]

Add support for macOS by porting the Texture based iOS version. A PlatformView based version can be created as soon as this is possible on macOS.

Fixes flutter/flutter#41688

Tests are currently failing due to flutter/flutter#78003.

Pre-launch Checklist

• I read the Contributor Guide and followed the process outlined there for submitting PRs.
• I read the Tree Hygiene wiki page, which explains my responsibilities.
• I read and followed the Flutter Style Guide and the C++, Objective-C, Java style guides.
• I signed the CLA.
• The title of the PR starts with the name of the plugin surrounded by square brackets, e.g. [shared_preferences]
• I listed at least one issue that this PR fixes in the description above.
• I updated pubspec.yaml with an appropriate new version according to the pub versioning philosophy.
• I updated CHANGELOG.md to add a description of the change.
• I updated/added relevant documentation (doc comments with ///).
• I added new tests to check the change I am making or feature I am adding, or Hixie said the PR is test exempt.
• All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

[stuartmorgan]

I'm not sure it makes sense to land an implementation that we explicitly plan to throw away in the relatively near future. It might make more sense for this preliminary version to be hosted elsewhere and published for use as an unendorsed implementation (ideally not using the name video_player_macos, which is what the official package would be called following the normal naming conventions of this repository). @cbracken Thoughts?

If this is done here, it needs to be a separate macOS package; all new implementations here are federated, by policy.

[Sunbreak]

https://github.com/fluttercommunity/plus_plugins maybe a better place

[cbenhagen]

Of course if by "relatively near future" you mean the next few weeks then yes, it might be easier to just wait. But from my perspective it is very hard to know when this will be even possible to do or when there will be someone willing to implement this. I have not seen any work being done to implement PlatformView for macOS. And even with it being available for iOS for a long time, so far no one re-implemented the video player using it despite the advantages this might bring. But then also it's not that big of a problem if this implementation gets replaced and thrown away in the near future.

I am totally fine with a separate package but please let it live alongside the other implementations so it can profit from the CI pipeline etc.

[stuartmorgan]

Of course if by "relatively near future" you mean the next few weeks then yes, it might be easier to just wait.

There is not an arbitrary deadline of a few weeks for deciding whether this implementation is one we want to officially publish and endorse. Especially when there's the option of someone who doesn't want to wait to set up and publish an unendorsed implementation at any time.

I have not seen any work being done to implement PlatformView for macOS.

flutter/engine#22905 has a substantial portion of the necessary work. But again, the exact timing of that work being completed is not the only factor.

But then also it's not that big of a problem if this implementation gets replaced and thrown away in the near future.

That's a much easier view to have when you won't be responsible for dealing with all the issues filled about regressions in the new implementation as compared to the old one, or for explaining to people why advantages that they might not care about outweigh the disadvantages of lost features they did. Using PlatformView is not, after all, strictly better. There are trade-offs.

I am totally fine with a separate package but please let it live alongside the other implementations so it can profit from the CI pipeline etc.

Setting up a simple CI pipeline for a single package with GitHub Actions is very straightforward, so I'm not sure why that would be a significant consideration.

[cbenhagen]

I am totally aware of the responsibilities that arise from new code. A new supported platform means new bugs, more users with more problems. And changing the implementation might indeed lead to some more created issues. I still believe that the benefits of having an officially endorsed macOS implementation will outweigh those costs. But in the end it is your team which has to decide not me and If you don't want to accept this contribution I won't take this personally.

Still in the hope of a quickly available official macOS version, I updated this PR to create a new package.

[yangyxd]

vide_player , When will windows be supported?
Is there such a plan?

[cbenhagen]

@yangyxd please subscribe to this issue for updates on a windows implementation:
flutter/flutter#37673

[yangyxd]

@yangyxd please subscribe to this issue for updates on a windows implementation:
flutter/flutter#37673

OK, thanks.

[rxlabz]

@cbenhagen Hi Ben , thanks for your 2 PRs ! I'm Trying to run the example from the macOS example on your branch but the app crashes Terminating app due to uncaught exception 'NSRangeException', reason: '*** -[_NSZeroData getBytes:range:]: range {0, 1} exceeds data length 0'. Do I miss something ?

Complete log

~/tmp/plugins/packages/video_player/video_player_macos/example on video_player_macos_new
% flutter run -d mac
Launching lib/main.dart on macOS in debug mode...
Running pod install...                                           1 763ms
Building macOS application...
Syncing files to device macOS...                                    77ms

Flutter run key commands.
r Hot reload. 🔥🔥🔥
R Hot restart.
h Repeat this help message.
d Detach (terminate "flutter run" but leave application running).
c Clear the screen
q Quit (terminate the application on the device).

💪 Running with sound null safety 💪

An Observatory debugger and profiler on macOS is available at:
http://127.0.0.1:56837/7jwl__7aCHg=/
2021-04-02 15:46:00.930 example[14071:3390745] *** Terminating app due to uncaught exception 'NSRangeException', reason: '*** -[_NSZeroData getBytes:range:]: range {0, 1} exceeds data length 0'
*** First throw call stack:
(
	0   CoreFoundation                      0x00007fff3307db57 __exceptionPreprocess + 250
	1   libobjc.A.dylib                     0x00007fff6bf275bf objc_exception_throw + 48
	2   Foundation                          0x00007fff3569706e -[NSData(NSData) getBytes:range:] + 616
	3   FlutterMacOS                        0x0000000108182953 -[FlutterStandardReader readBytes:length:] + 51
	4   FlutterMacOS                        0x00000001081829ac -[FlutterStandardReader readByte] + 44
	5   FlutterMacOS                        0x0000000108182c9f -[FlutterStandardReader readValue] + 47
	6   FlutterMacOS                        0x0000000108180973 -[FlutterStandardMessageCodec decode:] + 83
	7   FlutterMacOS                        0x000000010817d7f9 __48-[FlutterBasicMessageChannel setMessageHandler:]_block_invoke + 57
	8   FlutterMacOS                        0x0000000107978b43 -[FlutterEngine engineCallbackOnPlatformMessage:] + 275
	9   FlutterMacOS                        0x00000001079778ac _ZL17OnPlatformMessagePK22FlutterPlatformMessageP13FlutterEngine + 44
	10  FlutterMacOS                        0x0000000108195f8d _ZNSt3__110__function6__funcIZ23FlutterEngineInitializeE4$_44NS_9allocatorIS2_EEFvN3fml6RefPtrIN7flutter15PlatformMessageEEEEEclEOS9_ + 125
	11  FlutterMacOS                        0x00000001081a73ca _ZN7flutter20PlatformViewEmbedder21HandlePlatformMessageEN3fml6RefPtrINS_15PlatformMessageEEE + 74
	12  FlutterMacOS                        0x0000000107f06ad9 _ZNSt3__110__function6__funcIZN7flutter5Shell29OnEngineHandlePlatformMessageEN3fml6RefPtrINS2_15PlatformMessageEEEE4$_38NS_9allocatorIS8_EEFvvEEclEv + 137
	13  FlutterMacOS                        0x00000001081a47f7 _ZN7flutter18EmbedderTaskRunner8PostTaskEy + 759
	14  FlutterMacOS                        0x000000010818ca3c FlutterEngineRunTask + 44
	15  FlutterMacOS                        0x0000000107979667 __60-[FlutterEngine postMainThreadTask:targetTimeInNanoseconds:]_block_invoke + 71
	16  libdispatch.dylib                   0x00007fff6d0756c4 _dispatch_call_block_and_release + 12
	17  libdispatch.dylib                   0x00007fff6d076658 _dispatch_client_callout + 8
	18  libdispatch.dylib                   0x00007fff6d081cab _dispatch_main_queue_callback_4CF + 936
	19  CoreFoundation                      0x00007fff33040e81 __CFRUNLOOP_IS_SERVICING_THE_MAIN_DISPATCH_QUEUE__ + 9
	20  CoreFoundation                      0x00007fff33000c87 __CFRunLoopRun + 2028
	21  CoreFoundation                      0x00007fff32fffe3e CFRunLoopRunSpecific + 462
	22  HIToolbox                           0x00007fff31c2cabd RunCurrentEventLoopInMode + 292
	23  HIToolbox                           0x00007fff31c2c7d5 ReceiveNextEventCommon + 584
	24  HIToolbox                           0x00007fff31c2c579 _BlockUntilNextEventMatchingListInModeWithFilter + 64
	25  AppKit                              0x00007fff30272039 _DPSNextEvent + 883
	26  AppKit                              0x00007fff30270880 -[NSApplication(NSEvent) _nextEventMatchingEventMask:untilDate:inMode:dequeue:] + 1352
	27  AppKit                              0x00007fff3026258e -[NSApplication run] + 658
	28  AppKit                              0x00007fff30234396 NSApplicationMain + 777
	29  example                             0x000000010792f61d main + 13
	30  libdyld.dylib                       0x00007fff6d0cfcc9 start + 1
)
libc++abi.dylib: terminating with uncaught exception of type NSException
Lost connection to device.

[cbenhagen]

@rxlabz this is due to flutter/flutter#78003. This issue is fixed in the master channel but there seems to be a memory leak which needs to be tracked down. Maybe you want to help with that?

[rxlabz]

@cbenhagen I wish I could help, but honestly I don't know how :(

[rxlabz]

@cbenhangen I profiled your example app and indeed, can confirm a memory leak. A lot of 4Mo CoreVideo object seems to not be "released" and are accumulated on each play. For now, my attempt to find the source/solution failed, but I'll try again !
Any hint ?

[cbenhagen]

@rxlabz no but flutter/flutter#79648 is already assigned.

[cbenhagen]

@rxlabz the memory leak should be fixed in master ;)

[cbenhagen]

@iskakaushik
Currently when using this plugin with the metal embedder we are getting a huge amount of this errors
[ERROR:flutter/shell/platform/embedder/embedder_external_texture_metal.mm(59)] External texture callback for ID 140609164393680 did not return a valid texture.

My guess is that it tries to update quicker than the video frame rate or the frame rate of the video composition.

Can those errors be ignored and if yes, can we silence them? Or should this be fixed in the plugin code? If so, do you have a suggestion on what the best approach would be?

[diegomgarcia]

I've been trying test it but I'm getting the error

VideoPlayerApi.initialize (package:video_player_platform_interface/messages.dart:156:7)

I've add on pubspec that way:

video_player:
    git:
      url: https://github.com/cbenhagen/plugins.git
      path: packages/video_player/video_player
      ref: master
      rev: 13e55fbef29f8239b5a3b3222634069b7153fc6f

Any tips ?

[binoytv9]

^2.1.15

  video_player: ^2.2.6
  video_player_macos:
    git:
      url: https://github.com/cbenhagen/plugins.git
      path: packages/video_player/video_player_macos
      ref: video_player_macos_new
  video_player_windows:
    git:
      url: https://github.com/anirudhb/flutter_packages.git
      path: video_player_windows

Above pubspec.yaml still throws error on macOS

[ERROR:flutter/lib/ui/ui_dart_state.cc(209)] Unhandled Exception: PlatformException(video_player, not implemented, null, null)
#0      VideoPlayerApi.create
package:video_player_platform_interface/messages.dart:189
<asynchronous suspension>
#1      MethodChannelVideoPlayer.create
package:video_player_platform_interface/method_channel_video_player.dart:50
<asynchronous suspension>
#2      VideoPlayerController.initialize
package:video_player/video_player.dart:329
<asynchronous suspension>

[yasinarik]

Edit:

As @maxbeech stated, only .network() constructor works. I can say that .asset() is not working after my test.

VideoPlayerController.network(url);

@cbenhagen Sorry for disturbing but I couldn't find an example showing how it is possible to run the video on macOS.

dependencies:
  flutter:
    sdk: flutter
  video_player: ^2.1.15 #(also tried ^2.2.6)
  video_player_macos:
    git:
      url: https://github.com/cbenhagen/plugins.git
      path: packages/video_player/video_player_macos
      ref: video_player_macos_new

Error:

PlatformException(video_player, not implemented, null, null)

[stuartmorgan]

Cosing per #3712 (comment) (I meant to close it at the time of that comment).

[ollyde]

Has anyone got a maintained version of this? Since it's 7 months and we still don't have a critical video component..

@stuartmorgan how come you closed this? With no solution in sight for a very long time, this was perfect! :-D

[ollyde]

Ok, I updated it myself.

Based on what @cbenhagen made.

Here is an updated and working version here
https://github.com/ollydixon/flutter_macos_video_player

video_player_macos:
git:
    url: https://github.com/ollydixon/flutter_macos_video_player
    path: packages/video_player/video_player_macos

@stuartmorgan would be nice to get this on the main plugins package. I don't see desktop video coming for along time.

[stuartmorgan]

@stuartmorgan how come you closed this?

That question is answered at length in the comments above; I don't have anything to add beyond what's already there.

Here is an updated and working version here
https://github.com/ollydixon/flutter_macos_video_player

As noted in comments above, it would be helpful if you could change the name of the package (in the pubspec.yaml) to ensure that it doesn't get published to the name that the official package intends to use once it's available. And per discussion above and in the issue, there's no reason it couldn't be published under than new name to make it easier for people to use, instead of requiring git references. Published, unofficial implementations of federated plugins are an intended feature of the federated plugin system.

(FYI, you don't need to keep any of the flutter/plugins repository structure, or the other sub-packages, in your repo. All you need is the implementation package.)

@stuartmorgan would be nice to get this on the main plugins package.

Nothing has changed about the plans for the official implementation since my comments above.

[ollyde]

@stuartmorgan I've cleaned up the repository. I'm not entirely sure what you mean, if you could kindly make a pull request that would be great. I will publishing this on pub.dev since there are many folks who need it. 🚀

[ollyde]

For anyone else, you can find it on pub dev

video_player_macos 1.0.1

[stuartmorgan]

What I meant is that you should not publish it under the name video_player_macos. As I said here, here, and here.

But it's too late now, because you just did exactly that, so that name is no longer available for the Flutter team to use in the future for an official plugin.

[ollyde]

@stuartmorgan sorry man, first time publishing on pub dev. When the time comes I can update / migrate access to this named route to you guys, but right now we cannot wait another year for a critical plugin such as videos on desktop.

[stuartmorgan]

right now we cannot wait another year

Using another name would not have required waiting any longer than it took you to change one line in the pubspec.yaml file, so I'm not sure what the relevance here is. But regardless, it's too late to fix now.

[cbracken]

One option here that I think we've used in the past is to publish to a new plugin name and hand the existing name off to the flutter org. I think we've done the opposite in the past -- reassigning ownership of a Flutter-developed package to an enthusiastic team in the community.

[stuartmorgan]

The primary issue there is that it has the basically same problem that led us to not accept this implementation as a temporary implementation in the first place and then change to a platform view implementation later: the behavior will be different, potentially in some significant ways. Having that show up to existing clients as an "update" would be confusing.

At this point the best option is for us to just use another name. That may happen anyway; the iOS implementation was federated as video_player_avfoundation in the hope that we can share enough implementation with macOS (even with a different rendering approach) that it will make sense to share a package. (But since we don't know if that will be the case, we wanted to keep the other name available. In addition to avoiding people potentially thinking some other implementation was the 1P implementation based on the name.)

[ollyde]

If you guys want the name I can change it next week. It’s still early ;-)

[stuartmorgan]

You could change the name, republish, mark video_player_macos as discontinued so that hopefully nobody starts using that named version, and then we'd have the option to transfer the name later with minimal ecosystem impact if that ends up being necessary.

[ollyde]

You could change the name, republish, mark video_player_macos as discontinued so that hopefully nobody starts using that named version, and then we'd have the option to transfer the name later with minimal ecosystem impact if that ends up being necessary.

I'll do this tomorrow :-)

[ollyde]

Tried to rename it this morning, but it wouldn't compile after.

Error on line 1, column 7: "name" field doesn't match expected name "flutter_video_player_macos".
  ╷
1 │ name: video_player_macos
  │       ^^^^^^^^^^^^^^^^^^
  ╵

It's definitely changed.
name: flutter_video_player_macos

Searching the whole project and rename didn't seem to work. Clearing the pub cache nope.

We don't have time to fiddle around with this right now, will take a look later this week.

@stuartmorgan Do you have any estimate on the Windows/MacOS official plugin?

[dxvid-pts]

Tried to rename it this morning, but it wouldn't compile after.

You could change the name, republish, mark video_player_macos as discontinued so that hopefully nobody starts using that named version [...]

The more relevant part for now would be marking video_player_macos as discontinued on pub.dev I guess.

[stuartmorgan]

This has gotten very off-topic from the discussion of the PR itself; let's move any further discussion of the plugin version to that project's repository.

[ollyde]

@stuartmorgan I think it was a bad idea to close this. It's still a very valid ticket.

@peterscodee I'm not discouraged to remove since Stuart has closed this ticket and no estimates have been giving on when we will get video in Flutter.

[stuartmorgan]

@stuartmorgan I think it was a bad idea to close this. It's still a very valid ticket.

We do not leave PRs open that we are not going to land. This PR is proposing a specific implementation which, as explained above, we have explicitly decided not to move forward with.