wcjs-player hides wcjs-renderer's ability to rely on wcjs-prebuilt

[ardoramor]

I am trying to put together a build script that installs wcjs-player and uses wcjs-prebuilt.

I have previously successfully built my app with only wcjs-player by manually providing vlc libs. I am currently working on making a release by building portable version of the app. Checking in vlc libs, for different platforms, into my repo doesn't really make sense. So I started looking into wcjs-prebuilt. It looks nice because it can be installed as a separate dependency for a specific platform and arch. Its also cool that target-dir can be specified.

I then tried to see how to integrate it with wcjs-player. Because there is no guarantee the order of dependency installation, wcjs-prebuilt may install before wcjs-player. Having then wcjs-target point to wcjs-player's webchimera.js release folder is not reliable and also not a good design (in case something changes). The better option is to write a postinstall script that will drop wcjs-prebuilt bin folder into wcjs-player's webchimera.js release folder... but that's just a bit better than the previous method.

I then found that wcjs-renderer allows for specification of optional dependency in its init function. However, wcjs-player, which wraps wcjs-renderer, does not expose that option.

What this made deliberately for any reason? Can the option be introduced into wcjs-player?

Alternatively, what is the preferred way of integrating wcjs along with vlc for projects that will be built for different platforms/archs and manual introduction of vlc libs is not preferred?

[RSATom]

@ardoramor I could provide WebChimera.js archive with embedded libvlc if it will help. I already build it for OS X, and it's not a big deal to do the same for Windows.

[RSATom]

Alternatively, what is the preferred way of integrating wcjs along with vlc for projects that will be built for different platforms/archs and manual introduction of vlc libs is not preferred?

Unfortunately there are no reliable and flexible enough way to do it at the moment.

btw, @jaruba, I recommend you switch to https://github.com/RSATom/webgl-video-renderer it's more flexible, and could be used with different video sources, not only with WebChimera.js

[ardoramor]

As a separate download I can already include it within a script. I was thinking of, temporarily, copying wcjs-prebuilt to wcjs-player within my build script in npm run build. But I was just curious if wcjs-player can provide the optional dependency that feeds into wcjs-renderer. I can even provide a PR. I'm just new to electron/nw/wcjs and not aware the reasons why this option was not considered as it makes the use of vlc so much easier. Can it do with the type of compilation of vlc (like licensed codecs?).

[RSATom]

the reason is simple, this option was added little bit after initial wcjs-player release, and nobody asked about enabling it yet.

[RSATom]

So we will accept PR gratefully. I just think it will be better wait @jaruba's answer, maybe he will have some recommendations.

[ardoramor]

OK, will wait for @jaruba. Thank you guys for the help. I'd also later like to provide a PR for wcjs-prebuild as the install fails out of the box... it expects a .git or .gh to detect a route. My app is in a child directory of the project root so that fails when I kick off build from root (there is a reason for that hierarchy). So I have to provide a .gh dummy file for install to work. PR would provide a way to hint find-project-root through an env variable.

[jaruba]

Last I checked, I wasn't able to switch between Electron / NW.js in wcjs-prebuilt, it had a way of trying to figure out the platform on it's own, but that didn't work in any of my tests.

The simple truth is that if you want to make sure that it will work correctly, there is no better substitute for building it in the install process. Unless wcjs-prebuilt has a reliable way of selecting the platform, version and architecture before installation, I don't think it's a good idea to add it.

[ardoramor]

Webgl-video-renderer is a great idea at least for MIT license :)

[RSATom]

@jaruba, it the good reason to get wcjs as createPlayer parameter to wcjs-player. Maybe somebody else will have time to create reliable wcjs-prebuilt

[ardoramor]

There is a way to specify all of those options now: https://github.com/Ivshti/wcjs-prebuilt/blob/07cdcaa59f4d54ee97514d982702a93e0be42472/install.js#L122

[RSATom]

@ardoramor, maybe, but for my opinion wcjs-rebuilt is not reliable enough to use it in production... And you should know, it installs slightly modified version of libvlc - it installs patched version of avi format support plugin

[jaruba]

Well, as wcjs-player is already stable for a while, I could add prebuilt to a fork, so instead of adding wcjs-player to package.json, you could add wcjs-player@prebuilt and it will use the prebuilt instead of building.

[RSATom]

@jaruba, Are you against adding wcjs as wcjs-player.createPlayer arg? If you will do it, it will not need any dependency from wcjs/wcjs-prebuilt at all.

[ardoramor]

Hmm, yes, its complicated. What about having implement dependency injection? If wcjs-renderer or webgl-video-renderer (if they conform to the same interface) is chosen by a dev, it could be provided as a dependency to wcjs-player.

[RSATom]

unfortunately wcjs-renderer and webgl-video-renderer have different api

[RSATom]

but both supports dependency injection

[jaruba]

@RSATom What exactly is the difference between the renderers though? The reason I wouldn't change renderers now is that wcjs-renderer has already been heavily tested and does all wcjs-player needs.

[RSATom]

wcjs-renderer and webgl-video-renderer have the same render code inside, just different outer API. I've forked webgl-video-renderer to be able use it without modification with both Webchimera.js and WebChimera.js GStreamer edition

webgl-video-renderer doesn't have direct dependency from any of them, just have a function which accept video frame for render.

[RSATom]

but even if you don't want switch to webgl-video-renderer it's still right idea be able get extern wcjs instance as createPlayer argument.

[jaruba]

I expected that to be the difference.

Are you against adding wcjs as wcjs-playr.init arg? If you will do it, it will not need any dependency from wcjs/wcjs-prebuilt at all.

You mean let people pass the wcjs object to wcjs-player externally? Then wcjs-player also pass it to wcjs-renderer?

I'm not against that, but I bet people will flood my issues if I do it, as that would be even more complicated to explain to newcomers.

[RSATom]

You mean let people pass the wcjs object to wcjs-player externally? Then wcjs-player also pass it to wcjs-renderer ?

yes

I'm not against that, but I bet people will flood my issues if I do it, as that would be even more complicated to explain to newcomers.

You could make it optional, and by default use current approach. wcjs-renderer works the same way.

[RSATom]

but then it still will download WebChimera.js and will try build it, maybe not good solution.

[RSATom]

but anyway, I think add wcjs-prebuilt dependency is very very bad idea.

[RSATom]

btw, maybe newcomers just should read docs? and anyway, they will start from some sort of demo, and it will be pretty clear there it require extern wcjs object...

[RSATom]

https://github.com/jaruba/node-vlcPlayer-demo/blob/master/index.html

it will be hard miss wcjs instance creation in such simple example

[RSATom]

<script>
var wcjs = require("WebChimera.js");
var wjs = require("wcjs-player");
var player = new wjs("#player").addPlayer(wcjs, { autoplay: true });

player.addPlaylist("http://archive.org/download/CartoonClassics/Krazy_Kat_-_Keeping_Up_With_Krazy.mp4");
</script>

[RSATom]

@jaruba, btw, why var player = new wjs("#player").addPlayer({ autoplay: true }); but not just var player = wjs("#player", { autoplay: true }); ? Why do you need separate addPlayer?

[RSATom]

or var player = wjs.addPlayer("#player", { autoplay: true });

[RSATom]

Why do you need separate addPlayer?

I think I understand. You can access all created players via wjs without store it somewhere else

[jaruba]

You can access all created players via wjs without store it somewhere else

Yeah, I made wjs like jquery's $, it stores individual wcjs objects based on context.

[RSATom]

Maybe then using new is wrong there?

[jaruba]

I'm not sure why I need to use new there, but I'm sure I had a good reason. :))

[ardoramor]

new is required because otherwise this context of wjs would be window. Also, every "instance" would share the same context.

[ardoramor]

@jaruba Did you arrive to any conclusion on this? Dependency injection/integration of wcjs-prebuilt/need more consideration?

@RSATom I didn't know wcjs-prebuilt used modified version of avi. Where did you read that? I'd like to know more details.

[jaruba]

@ardoramor

I didn't know wcjs-prebuilt used modified version of avi. Where did you read that? I'd like to know more details.

No, we did that actually.. VLC has this long lasting issue, there are patches sent to VLC but the author refuses to accept them. If you play a avi stream, it won't support seeking and won't show the total length, @Ivshti fixed this in their code and rebuilt VLC. We're using his VLC version in the prebuilts.

@RSATom I have nothing against the avi fix, we've been using it in Powder and Stremio for a long time now without any issues.

But the Magics VLC prebuilts are still broken for some other reasons, I didn't dig too deep to see what the issue is there.

Did you arrive to any conclusion on this? Dependency injection/integration of wcjs-prebuilt/need more consideration?

I'm still not sure what the best way to implement it would be, either make a fork that installs with prebuilts (i don't know any other way then forking to specify a different dependency) or pass the wcjs object to the player..

I'd still go with a fork, as you can specify forks as dependencies in package.json.. That way people could use whatever they wanted.. If you guys have any other better ideas, I'm listening..

[RSATom]

@ardoramor, I know because I've builded this patched version. Don't sure if our related discussion still available somewhere. The only difference from vanilla VLC is https://github.com/Ivshti/vlc-2.2/commit/b65ed7b2d4c82c74d387a819c7e2d8e7b0f58548

[RSATom]

@jaruba, unfortunately I don't have better ideas... maybe just release wcjs-player v2.0 with different api... and without wcjs dependency...

[ardoramor]

I have quick-fixed this in my app by taking one of the approaches I described in the first post... it works and with it, I can wait for an official resolution.

In my build.js script, I specify the following gulp.task:

gulp.task('provideVlc', function () {
    return gulp.src('./app/node_modules/wcjs-prebuilt/bin/**/!(*.node)')
        .pipe(gulp.dest('./app/node_modules/wcjs-player/node_modules/wcjs-renderer/node_modules/webchimera.js/build/Release'));
});

In my preinstall script, I provide the following variables:

// For wcjs-prebuilt
process.env.wcjs_platform = process.platform;
process.env.wcjs_runtime = "electron";
process.env.wcjs_runtime_version = electronVersion;
process.env.wcjs_version = "v0.1.47"; // https://github.com/RSATom/WebChimera.js/releases shows that its version matches electron 0.36.2
process.env.wcjs_arch = process.arch;

It's not pretty but it works. The issue can be closed and maybe a separate one open that would address this on a bigger scale.

One thing I wanted to mention was that I am using fs-jetpack in buildscripts. My package.json had a devDependency for Q. Module fs-jetpack also has that dependency. When I originally ran install, Q was installed in the immediate node_modules but not in fs-jetpack. I'm relatively new to nodejs so I dont know all specifics but is it possible to repeat this effect for wcjs-player/

Maybe in preinstall, wcjs-prebuilt can be installed and its webchimera.js and vlc libs can be copied into the immediate node_modules, consequently preventing wcjs-renderer from building it. What do you think? Is it possible for wcjs-renderer to check existence of webchimera.js before building it (maybe via require('webchimera.js') and, if present, skip build step?

[jaruba]

@luigiplr

[luigiplr]

I HAVE BEEN PINGED.

[jaruba]

@luigiplr you got any thoughts on all this? either about ardoramor's last comment or this comment: https://github.com/jaruba/wcjs-player/issues/33#issuecomment-173507238

[delian]

I am suggesting another patch, which is capable of fixing that issue.

If you just change line 441 to:

if (wcpSettings && wcpSettings["vlcArgs"]) vlcs[newid].vlc = vlcs[newid].renderer.init(wjs(new).canvas,wcpSettings["vlcArgs"],wcpSettings["wcjsRendererOptions"],wcpSettings["wcjs"]);

one will be able to modify directly wcjs-render and use wcjs-prebuilt with a call like:

wcjs(...).addPlayer({ wcjs: require('wcjs-prebuilt') })

[vishalup]

How should I use wcjs-prebuilt? With following code, I am not getting any error, and screen color is white.

var wcjs = require("wcjs-prebuilt");
var player = wcjs.createPlayer("#canvas", "http://archive.org/download/CartoonClassics/Krazy_Kat_-_Keeping_Up_With_Krazy.mp4");
//player.addPlaylist("http://archive.org/download/CartoonClassics/Krazy_Kat_-_Keeping_Up_With_Krazy.mp4");
//player.play("http://archive.org/download/CartoonClassics/Krazy_Kat_-_Keeping_Up_With_Krazy.mp4");

[delian]

wcjs-prebuild is not the topic of this api (wcjs-player), so I am not sure I understand the question. If the question is, how to use wcjs-prebuilt with wcjs-player, this is the correct example:

var wcjsPlayer = require('wcjs-player');
var player = new wjsPlayer("#player").addPlayer({ autoplay: true, wcjs: require('wcjs-prebuilt') });
player.addPlaylist("http://archive.org/download/CartoonClassics/Krazy_Kat_-_Keeping_Up_With_Krazy.mp4");

[jaruba]

I think this can finally be closed, since this change removed default building of WebChimera.js.node and now wcjs is the only mandatory parameter in .addPlayer()

The README.md is now updated with info about the different ways of installing WebChimera.js separately. And the JS API Docs have been updated accordingly.

Thanks everyone for your help on this. :)