Split out server/controller process from renderer process (and more)

[bmathews]

There's a lot here, but I'd like to get some feedback and have people test this out. This improves code quality, manageability, and stability/behavior of the app. Browse the code here: https://github.com/bmathews/playback/tree/refactor-rebase/app

The new command to run things is npm run dev

Changes:

• Split out server/controller process from renderer process move playlist + http server to main process #58
  • Communication is done via IPC
• Split out loaders
• Split out 'players' into swappable interface
  • Have webchimera working in a different branch, but I haven't been able to get webchimera to package into a .app properly. Add AVI Support with WebChimera? #75
• Use React for UI
• Adds eslint for code style/quality consistency
• Various UI/UX improvements
• Added media hotkey support
• Improved styling of titlebar to better match Yosemite.
• Improved responsive/wrapping behavior of controls Controls break when title is too long (or window to narrow) #30
• Added 'loading' and 'buffering' indicators for various actions Loading screen #18
• Added buffer progress to timeline Add download progress when streaming from a magnet link #49
• Change how drag is done Drag the Video Widget On Fullscreen Behaviour #74
• Change how youtube videos are loaded, choosing a higher resolution codec when possible.
• Gets us really close to a mobile/desktop remote web UI, since the UI has basically no knowledge of electron Mobile Remote Web UI #37

[seungjulee]

+1 except for the original default screen of cat. Great work!!

[mafintosh]

@bmathews is this LGTM?

[bmathews]

@mafintosh I'd say so, yeah.

[mafintosh]

@freeall do you have any final comments on this? Otherwise I'm gonna merge it tomorrow

[freeall]

I'm a little conflicted on this one. It's really good to see some activity on the project, and I love the fact that it's rewritten to React. On the other hand I don't like that in the same PR the code's been rewritten to ES6 and we now use Babel.

If I were to comment on just the style it's more frontend javascript than node which is probably fine in itself, but it changes the whole codebase and I would rather have that discussion in a PR for itself.

Just to get more activity on this I'd say we should merge it, though. Thoughts?

[bmathews]

@freeall I'd be happy to remove the Babel usage for everything other than the react compilation! It'll add some code, but shouldn't be too bad. Happy to fix/change/remove anything you're not stoked on, though.

Thanks!

[zeke]

It's kind of scary to make changes this big on an app that doesn't have a test suite, but I agree that forward momentum is also important. I'm not really a stakeholder here; just leaving my ¢¢.

it's more frontend javascript than node

Can you elaborate on this, @freeall?

[freeall]

@bmathews Can't we remove Babel altogether? @mafintosh says that electron has es6 support

Edit: misspelling

[bmathews]

@freeall We're taking advantage of electron's ES6 support and using a few extra babel transforms for things that aren't in it yet: import, destructuring, rest params, class properties, and the spread operator.

@zeke 💯 The tricky part with the old code base was that the server and view were really coupled, so it would have been difficult to create any tests at all. After splitting these things up, we'll be in a much better place to add tests.

[mafintosh]

ah i didn't notice that this changes all the code to es6. i'm 👎 on that in general.

[bmathews]

@mafintosh @freeall Thanks for the feedback! I'll drop the babel transforms for everything but the React JSX and we'll give it another review.

[mafintosh]

the design is looking awesome though! really wanna pull that in. great job on that @bmathews

[bmathews]

@mafintosh @freeall Pushed changes that remove babel compilation for everything but the React JSX and I switched the eslint config to https://github.com/feross/standard instead of airbnb's preset.

[feross]

FWIW, we're using Electron for Webtorrent.app and it supports spreads and the rest operator natively (actually pretty useful). So you shouldn't need Babel for that. import is just silly and doesn't give you anything over require, IMO.

Also, instead of JSX you might consider substack's hyperx which just uses ES6 template strings so there's no preprocessor required.

[freeall]

I hadn't seen hyperx before. I really like it.

[ppeczek]

Why isn't it merged? ;<

[phated]

Too complicated (should have been small PRs) and now way out of sync.

[bmathews]

Yeah, it was a big architectural change needed to split out the view from the 'server' and still support the 'follow' functionality.

I'm happy with my changes though. :) It solves about half of the open issues, and the VLC/MKV support is nice.

[phated]

@bmathews still, it doesn't merge cleanly and should all be squashed

[bmathews]

It did in February and can easily be squashed. That's the least of the
people's concerns, I think.
On Jun 18, 2016 6:54 PM, "Blaine Bublitz" [email protected] wrote:

@bmathews https://github.com/bmathews still, it doesn't merge cleanly
and should all be squashed

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#86 (comment),
or mute the thread
https://github.com/notifications/unsubscribe/AAzx2_ZGcoybK0RrpmzyHvzbpHy-Df3Wks5qNKFugaJpZM4HdZD3
.

[overlookmotel]

@bmathews I know this PR has been quiet a long time and I guess is sadly not getting merged.

I haven't looked through the code - obviously there's a lot of it. But one of your comments above got me intrigued - you said "the VLC/MKV support is nice". I opened an issue to discuss how to support other codecs #124. Can you elaborate on how you managed VLC/MKV support, or point me in the direction of the right bit of code please?

[bmathews]

The architecture I put together allowed for swappable 'players'. E.g., the normal HTML <video />, or a chromecast, etc. This allowed me to create a 'player' that used WebChimera (libVLC) to render individual frames of videos that I'd then display on an html canvas.

https://github.com/bmathews/playback/blob/refactor-chimera-rebase/app/players/WebChimera.js

It'd probably need a bit of work to resurrect at this point though, and I'd probably go down this route to improve performance: webtorrent/webtorrent-desktop#938 (comment)

[overlookmotel]

@bmathews Thanks loads for these links. I was totally unaware of WebChimera - looks awesome.

I also wonder if a similar thing could be done with FFMPEG? I'm thinking FFMPEG decoding video to raw frames which are passed to Electron and then written pixel-by-pixel into a canvas element. If I've understand correctly, this is roughly what WebChimera is doing.

I have a need to play some codecs which VLC does not handle e.g. Apple Prores4444 and JPEG2000 (used in Digital Cinema DCP format).

Your comment you linked to about OpenGL is also interesting. Was the high CPU usage related to rendering the canvas in JS?

Please excuse my newbie questions. This is an area I'm interested in but I have no knowledge at all of OpenGL and the like. I'm trying to feel out whether it'd take a massive amount of time for me to learn enough and implement to achieve what I'm after, and to what degree the foundations are already out there in the open source world.