Skip to content

Implement some useful API methods#7

Closed
NBTX wants to merge 27 commits into
solid-software:masterfrom
NBTX:master
Closed

Implement some useful API methods#7
NBTX wants to merge 27 commits into
solid-software:masterfrom
NBTX:master

Conversation

@NBTX

@NBTX NBTX commented May 5, 2019

Copy link
Copy Markdown
Contributor

This PR adds the following methods:

  • setPlaybackSpeed(double speed)
  • setStreamURL(String url) (previous implementation moved to initialize())
  • addListener(VoidCallback listener)
  • removeListener(VoidCallback listener)
  • clearListeners()
  • setTime(int time)
  • play()
  • pause()
  • stop()

and exposes the following properties:

  • aspectRatio
  • size
    • width
    • height
  • position
  • duration
  • playbackSpeed
  • playingState [PlaybackState - one of STOPPED, BUFFERING, PLAYING]

@NBTX NBTX mentioned this pull request May 5, 2019
@solid-software

Copy link
Copy Markdown
Collaborator

Wow @NBTX - thanks for your contribution! Give us some time to review everything so we can merge and then publish new version to the pub.

@NBTX

NBTX commented May 5, 2019

Copy link
Copy Markdown
Contributor Author

@solid-software sorry, I've been testing this as I go so it's gotten kind of messy in here. If there's anything you want me to do to clean this up, let me know.
I pushed all this crap before I realised anyone noticed the PR 😄

@solid-software

Copy link
Copy Markdown
Collaborator

Hey, no worries - just let us know when you are done so we can schedule review & merge.

@NBTX

NBTX commented May 5, 2019

Copy link
Copy Markdown
Contributor Author

@solid-software Sure thing. I'd also like to have a look at the iOS side. If it's not too mind-boggling (I haven't really touched iOS development) I might give it a go with implementing it on that side.

(Also I guess it would be nice to have everything properly documented 😄- I hastily typed my first message up because it was something like 3AM when I committed it.)

@wolfljj

wolfljj commented May 8, 2019

Copy link
Copy Markdown

@NBTX how can i use your fork version?

@NBTX

NBTX commented May 10, 2019

Copy link
Copy Markdown
Contributor Author

@wolfljj I don't know that you'll have success with it (I still need to do a bit more work on this - at least, I'm having some trouble implementing it into my app), however if you want to give it a shot, add this to the dependencies section of your pubspec.yaml:

  flutter_vlc_player:
    git:
      url: git://github.com/NBTX/flutter_vlc_player.git
      ref: master

Here's an example implementation:
https://github.com/NBTX/flutter_vlc_player/blob/master/example/lib/main.dart

@NBTX

NBTX commented Jul 2, 2019

Copy link
Copy Markdown
Contributor Author

Hey just to update everyone watching this PR. This is not dead, we're prepping it for use it over at @ApolloTVofficial (@ApolloTVofficial/kamino) for our built-in player.

This has kind of been on-hold because I've experienced some bugs implementing it into our app and I wanted to sort of clean everything up both in here and in our app before I throw a ton more features at it and work on landing this PR. Plus, I'm reading up on Objective-C and Swift to work on the iOS side of this project.

TL;DR, expect some more commits soon cleaning up the code, along with the necessary iOS platform channel code to even out this PR and get it ready to land.

@solid-software

Copy link
Copy Markdown
Collaborator

Thanks for the update @NBTX! Great job!

@verylovestars

Copy link
Copy Markdown

@NBTX Hi, you did not abandon the development?

@moeiscool

moeiscool commented Feb 20, 2020

Copy link
Copy Markdown

@moeiscool Did you implement your VlcPlayerController as follows?

final VlcPlayerController controller = new VlcPlayerController(
      // Start playing as soon as the video is loaded.
      onInit: (){
          controller.play();
      }  
);

Edit: Also I'm positive the other changes are to do with permissions rather than line endings - though I'm familiar with what you're describing.
image

yes i did, first test i missed it and it just crashed. after that i noticed it in the compare on the PR. it doesn't crash after adding that but it does the placeholder thing forever

iPhone XS Max, iOS 13.1.3

@NBTX

NBTX commented Feb 20, 2020

Copy link
Copy Markdown
Contributor Author

@moeiscool Can you open a new issue (just so we're not chasing two things on the same thread) - and I'll follow up while we merge this one.

@NBTX

NBTX commented Feb 20, 2020

Copy link
Copy Markdown
Contributor Author

@solid-software I'll take a look, I noticed upon looking again that aside from the permissions tweaks there is a lot of extra garbage for some reason, so I'll try and cut that down (per your comments).

@NBTX

NBTX commented Feb 20, 2020

Copy link
Copy Markdown
Contributor Author

@solid-software that should be the junk removed. Again, I've left the permissions changes as those are correct but I can remove them if you want.

@mitchross

mitchross commented Feb 22, 2020

Copy link
Copy Markdown
Contributor

This PR doesnt build for me when referenced in another project. lib/ui/widgets/CameraVLCStreamCard.dart:42:26: Error: The method 'VlcPlayer' isn't defined for the class 'CameraStreamVLCCardState'.

  • 'CameraStreamVLCCardState' is from 'package:muticam_dashboard/ui/widgets/CameraVLCStreamCard.dart' ('lib/ui/widgets/CameraVLCStreamCard.dart').
    Try correcting the name to the name of an existing method, or defining a method named 'VlcPlayer'.
    child: VlcPlayer(

@NBTX

NBTX commented Feb 22, 2020

Copy link
Copy Markdown
Contributor Author

@mitchross check the imports. This changes the API and consolidates the main functionality into a single file.

@mitchross

Copy link
Copy Markdown
Contributor

I got past import issues, now im seeing this error being logged when the app runs.

[VERBOSE-2:ui_dart_state.cc(157)] Unhandled Exception: NoSuchMethodError: The method 'call' was called on null.
Receiver: null
Tried calling: call()
#0 Object.noSuchMethod (dart:core-patch/object_patch.dart:53:5)
#1 VlcPlayerController._initialize (package:flutter_vlc_player/flutter_vlc_player.dart:279:5)
#2 _rootRunUnary (dart:async/zone.dart:1134:38)
#3 _CustomZone.runUnary (dart:async/zone.dart:1031:19)
#4 _FutureListener.handleValue (dart:async/future_impl.dart:140:18)
#5 Future._propagateToListeners.handleValueCallback (dart:async/future_impl.dart:682:45)
#6 Future._propagateToListeners (dart:async/future_impl.dart:711:32)
#7 Future._completeWithValue (dart:async/future_impl.dart:526:5)
#8 _AsyncAwaitCompleter.complete (dart:async-patch/async_patch.dart:34:15)
#9 _completeOnAsyncReturn (dart:async-patch/async_patch.dart:293:13)
#10 MethodChannel._invokeMethod (package:flutter/src/services/platform_channel.dart)
#11 _rootRunUnary (dart:asy<…>

@solid-software solid-software left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I reviewed most of the code, looks good to me just wanted to clear last piece I indicated in comments and I hope we can move it forward.

Thanks again for your contribution @NBTX !

}
}

class VlcPlayerController {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@NBTX is there any particular reason to place all classes in one file? I think it violates SRP - if we want to simplify imports we can create library, but that will make migration a bit more complex. What do you think?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

If you merge this PR, @solid-software you will need to version the library to 2.0.0 as the code is breaking change and backwards compatibility is broken. I tested this by importing this library in my own code and I had to refactor the VLC widget.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@mitchross I changed the version code.
I think I will follow @solid-software's advice and use multiple files. When I refactored initially one file made sense, but obviously it no longer does.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Cool. let me know when you are ready for a retest!

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

i wanna test things and give thumbs up to things

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@NBTX When are you going to update?

@Michael24884

Copy link
Copy Markdown

Hello! Are there any plans on adding http headers? Some videos that I use require a referer

@mitchross

Copy link
Copy Markdown
Contributor

If anyone wants this PR, I completed it on my fork,fixed the build errors, and upgraded to latest vlckit. I don't want to take the original authors credit, so I'm not gonna submit a PR. However, we haven't seen any communication form @NBTX , so if you want to go grab this PR without waiting you can use my version. https://github.com/mitchross/flutter_vlc_player

@Michael24884

Copy link
Copy Markdown

@mitchross hey thanks a lot for cleaning this up. By any chance do you know if it’s possible to implement headers? Really crucial for my project

@moeiscool

moeiscool commented Mar 13, 2020 via email

Copy link
Copy Markdown

@mitchross

mitchross commented Mar 13, 2020

Copy link
Copy Markdown
Contributor

@mitchross hey thanks a lot for cleaning this up. By any chance do you know if it’s possible to implement headers? Really crucial for my project

EDIT- Not possible. https://forum.videolan.org/viewtopic.php?t=129773

I would do the suggestion of forwarding the request on another server. Such that request to video from VLC APP -> middle ware server -> middleware server adds referrer ( easily set up with express/nodejs ) -> source video stream

@mitchross

Copy link
Copy Markdown
Contributor

@Michael24884 See my comment above, github wont let me tag you in the edit.

@Michael24884

Copy link
Copy Markdown

It looks like referrers are allowed based on the forum response? Is it possible to set it? That’s the only header I need to pass

@mitchross

Copy link
Copy Markdown
Contributor

@Michael24884 I see how to do it now. Can you give me an example url that requires a http-referrer so I can test ?

@Michael24884

Copy link
Copy Markdown

@solid-software

Copy link
Copy Markdown
Collaborator

Another PR landed that continued work from this PR so I'm closing this now.

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.