Skip to content
This repository was archived by the owner on Feb 22, 2023. It is now read-only.

Conversation

szakarias
Copy link
Contributor

No description provided.

Copy link
Contributor

@mravn-google mravn-google left a comment

Choose a reason for hiding this comment

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

LGTM

@@ -31,12 +32,45 @@
private final EventChannel eventChannel;
private boolean isInitialized = false;


VideoPlayer(
final EventChannel eventChannel,
Copy link
Contributor

Choose a reason for hiding this comment

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

None of these parameters need to be final. Same in the other constructor.


class _NetworkPlayerLifeCycleState extends _PlayerLifeCycleState {

_NetworkPlayerLifeCycleState();
Copy link
Contributor

Choose a reason for hiding this comment

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

Redundant


class _AssetPlayerLifeCycleState extends _PlayerLifeCycleState {

_AssetPlayerLifeCycleState();
Copy link
Contributor

Choose a reason for hiding this comment

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

Redundant

[registrar addMethodCallDelegate:instance channel:channel];
}

- (instancetype)initWithRegistry:(NSObject<FlutterTextureRegistry>*)registry
messenger:(NSObject<FlutterBinaryMessenger>*)messenger {
messenger:(NSObject<FlutterBinaryMessenger>*)messenger
registrar:(NSObject<FlutterPluginRegistrar>*)registrar {
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe reduce parameters to just the registrar, then access the FlutterBinaryMessenger and the FlutterTextureRegistry from there.

if (dataSource) {
NSString* assetPath = [_registrar lookupKeyForAsset:dataSource];
player = [[FLTVideoPlayer alloc] initWithAsset:assetPath
frameUpdater:frameUpdater];
Copy link
Contributor

Choose a reason for hiding this comment

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

Reformat?

Timer timer;
bool isDisposed = false;
Completer<Null> _creatingCompleter;
StreamSubscription<dynamic> _eventSubscription;
_VideoAppLifeCycleObserver _lifeCycleObserver;
dynamic _createMethodArg;
Copy link
Contributor

Choose a reason for hiding this comment

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

It is a bit sad that we need to carry this map around for the lifetime of the controller, given that it is only used during initialization. What about having final String dataSource and final bool isNetwork fields instead? Those may be useful for other purposes. Error reporting on playback failure should be very different for a network datasource compared to an asset one.

@sethladd
Copy link
Contributor

FYI: this PR came up in a Reddit thread here: https://www.reddit.com/r/FlutterDev/comments/87qqn2/has_anyone_used_video_in_an_app/ :)

@mravn-google mravn-google self-assigned this Mar 28, 2018
Copy link
Contributor

@mravn-google mravn-google left a comment

Choose a reason for hiding this comment

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

LGTM

@szakarias szakarias force-pushed the videoPlayFromAsset branch from 0e04e1c to c176c1a Compare April 3, 2018 19:48
@szakarias szakarias force-pushed the videoPlayFromAsset branch from c176c1a to 9466d55 Compare April 3, 2018 20:02
@szakarias szakarias merged commit 6e5de75 into flutter:master Apr 3, 2018
@fmatosqg
Copy link

fmatosqg commented Apr 9, 2018

@szakarias

I gave a hand to @osorito who is using windows and could try version 0.3.0 of video player plugin but not on version 0.4.0.

name: flutter_video_example
description: A new Flutter application.

dependencies:
  flutter:
    sdk: flutter

  cupertino_icons: ^0.1.0
  video_player: "^0.4.0"

dev_dependencies:
  flutter_test:
    sdk: flutter



flutter:


  uses-material-design: true
Using hardware rendering with device Android SDK built for x86. If you get graphics artifacts, consider enabling software rendering with "--enable-softwa
re-rendering".
Launching lib/main.dart on Android SDK built for x86 in debug mode...
Initializing gradle...
C:\Users\Omar Osorio\AppData\Roaming\Pub\Cache\hosted\pub.dartlang.org\video_player-0.4.0\android\src\main\java\io\flutter\plugins\videoplayer\VideoPlaye
rPlugin.java:244: error: cannot find symbol
                    registrar.lookupKeyForAsset(
                             ^
  symbol:   method lookupKeyForAsset(String,String)
  location: variable registrar of type Registrar
C:\Users\Omar Osorio\AppData\Roaming\Pub\Cache\hosted\pub.dartlang.org\video_player-0.4.0\android\src\main\java\io\flutter\plugins\videoplayer\VideoPlaye
rPlugin.java:247: error: cannot find symbol
                assetLookupKey = registrar.lookupKeyForAsset((String) call.argument("asset"));
                                          ^
  symbol:   method lookupKeyForAsset(String)
  location: variable registrar of type Registrar
2 errors

FAILURE: Build failed with an exception.

* What went wrong:
Execution failed for task ':video_player:compileDebugJavaWithJavac'.
> Compilation failed; see the compiler error output for details.

Both versions work for me on OS X, although I'm using Flutter (Channel dev, v0.2.2-pre.2, on Mac OS X 10.12.6 16G29, locale en-AU)

Our Dart code is different, we tried to follow from the examples published on github but independently. Only difference in dart from v 0.3.0 to 0.4.0 was from VideoPlayerController( to VideoPlayerController.network( and the version number in pubspec.yaml

I'm not sure this is the place for this kind of problem reporting but I couldn't open a new issue.

Also I'm not sure how to track the code for this change, my educated guess is that he's not bringing flutter.jar with his gradle configuration. That said I don't know how to find the commit for version 0.3.0 / 0.4.0 and have a look at all the changes.

I've also heard from other people in gitter having problems in java / swift compiling when importing different packages, is there a documentation where I can understand the configuration for gradle / pods (?) and how they bring flutter code into native projects so build tools find them?

@osorito
Copy link

osorito commented Apr 9, 2018

This is the change that made it work under version 0.3.0

class _NetworkPlayerLifeCycleState extends _PlayerLifeCycleState {
@OverRide
VideoPlayerController createVideoPlayerController() {
return new VideoPlayerController(widget.dataSource);
}
}

class _AssetPlayerLifeCycleState extends _PlayerLifeCycleState {
@OverRide
VideoPlayerController createVideoPlayerController() {
//return new VideoPlayerController.asset(widget.dataSource);
return new VideoPlayerController(widget.dataSource);
}
}

@osorito
Copy link

osorito commented Apr 9, 2018

This is my flutter doctor result

C:\Users\Omar Osorio\AndroidStudioProjects\flutter_video_example>flutter doctor
Doctor summary (to see all details, run flutter doctor -v):
[√] Flutter (Channel beta, v0.2.3, on Microsoft Windows [Version 6.1.7601], locale en-US)
[√] Android toolchain - develop for Android devices (Android SDK 27.0.3)
[√] Android Studio (version 3.1)
[√] Android Studio (version 3.2)
[√] IntelliJ IDEA Community Edition (version 2017.3)
[√] Connected devices (1 available)

• No issues found!

Since i'm using Android Studio 3.1 in my graddle propperties

#Fri Jun 23 08:50:38 CEST 2017
distributionBase=GRADLE_USER_HOME
distributionPath=wrapper/dists
zipStoreBase=GRADLE_USER_HOME
zipStorePath=wrapper/dists
distributionUrl=https://services.gradle.org/distributions/gradle-4.4-all.zip

And my build graddle

dependencies {
classpath 'com.android.tools.build:gradle:3.1.0'
}

@fmatosqg
Copy link

fmatosqg commented Apr 9, 2018

I found the problem,
Java interface Registrar created method lookupKeyForAsset that is still not available on beta build. Once beta catches up with dev I'd expect it to work.

That's completely understandable since it's both backwards compatible and you guys are entitled to API changes while in beta. But please understand that this strongly affects the perception of stability of the build system.

It would be nice to have a note added to this plugin such as "Use flutter 0.2.x or later".

@mravn-google
Copy link
Contributor

Thanks for diving into this. Yes, the video_player plugin should have had its Flutter SDK dependency bumped. I'll do that immediately.

@mravn-google
Copy link
Contributor

#474

slightfoot pushed a commit to slightfoot/plugins that referenced this pull request Jun 5, 2018
@masudsarker093
Copy link

Using Chewie video player can I play video from local storage? For example, I want to download video from network and save to device and want to play in apps by Chewie video player. Is it possible?

async {
final _openFile = await OpenFile.open(_filePath);

Want to open this video file into Chewie video player.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants