Skip to content
This repository has been archived by the owner on Mar 30, 2021. It is now read-only.

Radiant 2 Milestone #409

Open
ebramanti opened this issue Nov 12, 2015 · 38 comments
Open

Radiant 2 Milestone #409

ebramanti opened this issue Nov 12, 2015 · 38 comments
Milestone

Comments

@ebramanti
Copy link
Contributor

Note: Milestone Progress

Radiant contributors and end-users!

This is a draft of a proposed Radiant 2 milestone. I welcome any feedback anyone has on my proposed feature goals for a Radiant Player 2.0 release.

Once again, a big thanks to @kbhomes for opening the doors to more contributors who can merge PRs, and for moving this software to an open organization model. Your contributions to this app have been huge, and we all owe you a debt of gratitude for enhancing our Google Play Music experience.

@radiant-player/radiant-player-mac

Radiant Player 2.0

Proposed Features

Improved Performance

Importance - Critical

Current Issues pertaining to feature

#218, #235, #308, #328, #336, #377, #379, #403

Rationale for Inclusion in 2.0

This is an essential issue that must be fixed in Radiant Player, and I think it's a good goal for 2.0. For lower-memory systems, Radiant can be a real hog, eating a ton of memory.

This could be a side effect of using Flash over Webkit (see discussion here), but I think there are ways we can optimize Radiant's performance and memory usage without a move that extreme. The entire app even struggles in general use, and over a period of a few days many users report extremely high memory usage. I've experienced this problem myself, but I prefer to let the threads above speak to the slow performance of Radiant.

We must address this in preparation for a milestone release.

Move to google-music.js

Importance - High

Current Issues pertaining to feature

#252

Current PR addressing feature

#402, #520

Rationale for Inclusion in 2.0

This is an important issue to address, and I think @jacobwgillespie has phrased it well in his ongoing work over in #402:

In my opinion, we should use a central library for this kind of thing so that we all benefit from shared development resources (especially when Google updates something on their end that breaks us).

I heartily 👍 his opinion and I think it should be a target of 2.0 to get this implemented.

Fix/enhance mini-player

Importance - High

Current Issues/PRs pertaining to feature

#302, #394, #406, #331, #372

Current issues outstanding to mini-player (feature requests)

#225 - Feature request for more controls on mini-player
#251 - Feature request for volume slider on mini-player

Rationale for Inclusion in 2.0

The mini-player is a big differentiating factor that we have with browser plugins. From the status bar, users can view their current song, the album artwork, and have nice visual controls of what is playing.

  • I think we should absolutely fix the bugs in the player above for 2.0.
  • I also think we should consider fostering discussion about what features users are looking for in the mini-player, and aim to address them either in the milestone or post-2.0.

Last.fm reliability

Note: I recognize a lot of work is in-progress, aiming for more stability in a 2.0 release.

Importance - Minor

Current Issues directly pertaining to reliability

#285, #355, #368, #390, #396, #126

Current issues outstanding to Last.fm (feature requests)

#366

Rationale for Inclusion in 2.0

@chrismou has been pointman on these issues (thanks! 👍), and it seems that we have had on-and-off problems with Last.fm. Finding a way to stabilize our support is important. Many users find this to be an important feature; while I've prioritized it as minor, I think we should aim for stability and address users asking for expanded features with 👍 or 👎 regarding whether we can implement their requested functionality.

Improved Scrolling

Importance - Minor

Current Issues pertaining to feature

#204, #408, #275

Rationale for Inclusion in 2.0

This can also be considered a part of improving general performance, but looking into scrolling issues are essential to an experience that will make users want to use Radiant Player over a Chrome tab.

This may just be addressed through improved performance above.

EarPods Support

Importance - Low

Current Issues pertaining to feature

#43, #203

Current PR addressing feature

#384, #450

Rationale for Inclusion in 2.0

A very old issue that has an implementation in-progress (thanks @megalithic). We want users to be able to control playback like they would in iTunes, and not having accessories like EarPods or other Apple-certified inline mic controls work with Radiant Player hampers the seamlessness of the application.

@Primigenus
Copy link

Great work! Looking forward to seeing these land and some consistent releases happening :)

@ebramanti
Copy link
Contributor Author

@radiant-player/radiant-player-mac ping

@chrismou
Copy link
Member

Looks good to me. I couldn't say if all of this is actually possible, or if some is a limitation of the webview the app uses. Someone else may be able to answer that.

This did highlight a couple of issues that have since been fixed though, so I've closed the tickets. Mostly around last.fm scrobbling.

@chrismou
Copy link
Member

Only other thing I'd add is more keyboard shortcuts (#36). Might be worth having a discussion on what would be useful and get them implemented. A nice quick win.

I'd be happy to take this on. The keyboard shortcut code looks fairly self explanatory, from what I can see.

@ebramanti
Copy link
Contributor Author

@chrismou Feel free to strikethrough addressed issues and edit my post (you should be able to I think).

@jacobwgillespie This is related to #414, if you could review my document I would really appreciate it.

@kbhomes @benburton would really appreciate your feedback as well.

@chrismou
Copy link
Member

Yep, can do! I've marked the complete / in progress ones

@ebramanti
Copy link
Contributor Author

@jacobwgillespie @kbhomes @benburton Ping.

@zackkrida
Copy link

Not to intrude, but 👍 on all of the miniplayer ideas. I think that's what really sets this apart and is arguably the key feature.

@jacobwgillespie
Copy link
Member

LGTM - I'll review the PRs in a bit to see if there's anything I can strikethrough.

Should we make this an official GitHub milestone?

@chrismou
Copy link
Member

I've updated a few of the issue, removed a few dupes for clarity and left comments on a fewer of the old issues (notably, ones I can't recreate) to see if they're still an issue. I'm referencing an interim release I've tagged here: https://github.com/chrismou/radiant-player-mac/releases/tag/v1.5.0-20151213

I'll chase the ones up I've commented on later on. If we can't recreate and there's no feedback, we're best to assume they're no longer an issue unless someone reports it again.

A few of the performance issues I've left alone, as I'm seeing them myself. Most notably, the ones that talk about the UI virtually freezing after minimising the app. I can't recreate it 100% of the time, but I do see it a lot - so much so I've started keeping radiant player unminimised but hidden behind another app.

This would be a _really_ nice thing to see fixed, but so far I haven't been able to pinpoint when it started happening or what's causing it.

@jacobwgillespie
Copy link
Member

I assigned all these issues / PRs to a new milestone for tracking progress: https://github.com/radiant-player/radiant-player-mac/milestones/v2.0.0

@chrismou
Copy link
Member

👍

@ebramanti
Copy link
Contributor Author

@jacobwgillespie Thanks for assigning all of these to a milestone!!!

@ebramanti ebramanti added this to the v2.0.0 milestone Dec 14, 2015
@myneur
Copy link

myneur commented Jan 21, 2016

I suggest:
Major: stop using Flash. E. g. in Chrome Google Music does not require Flash anymore. There is no reason to keep flash with all its security issues in computer just to play music.
Minor: Airplay support for those who has wireless HiFi at home. It's annoying to switch all sound output to AirPlay and hear even notification or video/voice communication with delay or have to switch it back and forth.

But if you solve only the performance issues and remove the Flash or even just the performance issues, to make the milestone small and quick, you will increase the value of the player enormously. I personally stopped using Radiant player because of it. It was draining my battery and making notebook hot.
Everything else can be Milestone 3:)

@chrismou
Copy link
Member

@myneur

  • Would love to switch to HTML5, but Radiant uses Safari, which doesn't support it. So unfortunately, we're stuck with Flash for now at least.
  • Airplay support would be cool - feel free to have a go at integrating it yourself, pull requests are always welcome. Personally, as the owner of 3 chromecasts, I'm probably more inclined to work on adding casting support but there's no reason why Radiant can't support both :-)

@myneur
Copy link

myneur commented Jan 21, 2016

@chrismou I am not a programmer so I can't contribute.

One more suggestion #484: absent swipe gestures are also annoying because it is a natural way of navigation on Mac. You don't need to skip between touchpad and keyboard or try to hit small icons of arrows.

The point is everything which make Radiant experience inferior to a native browser Google Music is more likely to prevent users from switching to it than added features are likely to convince people to start using it.

@MarshallOfSound
Copy link

@chrismou Just to put my 2 cents in about flash and HTML 5, that electron player cough radiant-player-electron cough doesn't need flash to play music.

@chrismou
Copy link
Member

@MarshallOfSound Haha! Subtle ;-)

But yeah, once we've got gmusic.js into Radiant and I've wrapped up a PR I've been working on, I wouldn't mind spending a bit of time playing with that. Had never seen Electron before - been dying to have a play ever since I saw you guys talking about it!

@MarshallOfSound
Copy link

I pride myself on my subtlety 😸

Check out https://github.com/Nucleus-Player/Nucleus-Player for my attempt at electron, (still very much in progress)

@matthewdias
Copy link

What does everybody think about switching to/adding a notification center widget for the mini player? IMO it makes more sense from a usability and interface perspective.

@chrismou
Copy link
Member

chrismou commented Feb 4, 2016

@matthewdias Would this just provide users the option to add a widget to their notification centre, separate from the current icon in the menu bar?

If so, I'm certainly not against the idea, as long as it's not removing the current functionality. To be honest, I'd not even noticed you could customize those widgets until now! Having a mini player type widget there could be pretty cool.

Feel free to put in a PR and we'll check it out. It may be worth creating a ticket listing out what you're planning and assigning it to yourself, as it sounds like it might be a big job.

@matthewdias
Copy link

Yes once there is a widget as part of the bundle, users are able to add the widget and move it where they want. Here are some examples: https://i.imgur.com/C8EMbR0.png. I hesitate to assign it to myself, only because I probably don't have the time/ability to do it. Just thought I would suggest it for anyone who might want to give it a look.

@chrismou
Copy link
Member

chrismou commented Feb 4, 2016

Gotcha. Well feel free to log a separate ticket anyway with that screenshot. I'm a bit busy to tackle something like that myself right now, but perhaps someone will pick it up in the meantime.

@jacobwgillespie
Copy link
Member

So, just throwing this out there, but I'm starting to wonder if our Radiant 2.0 should be radiant-player-electron (we can even auto-update people to that build once it's ready). With the growing number of issues related to rendering differences between OSX/WebKit versions, the Google bugs in Safari, and the all-around poor performance, I'm thinking we might consider going full-Electron. And there's cross-platform compatibility as well. For everything but the latter, we could also attempt to use embedded webkit rather than an Apple-provided webview, however that's just replicating Electron. :)

I spent some of today updating the repo over at radiant-player-electron to get the build tooling up to date as well as implement some basic functionality. Right now it opens GPM, plays music, and responds to media keys. The performance differences are pretty awesome.

If we did decide to go that route, we'd have to build out the existing features from 1.0 in 2.0 - here's a non-exhaustive list of stuff that needs to get done:

  • Properly managing internal state (what song is playing, what are the repeat settings, what's the volume at, etc.) - this trickles down to the other stuff
  • All the actions (the stuff called from the menu items, etc.)
  • The menu
  • The menubar app
  • The settings dialog (and a system for storing the settings)
  • Theming system
  • Fixing the stoplight buttons placement
  • LastFM scrobbling
  • General app organization and build tooling
  • App support services (like the website, update server, etc.)
  • If we want cross-platform, we'd need different UI elements for the other OSs

There's probably more, but that covers most of it.

Interestingly enough, there's always been google-music-electron and Google-Play-Music-Desktop-Player-UNOFFICIAL- just switched to electron, so there's definitely some duplication of effort going on (with a little bit of shared work on gmusic.js) - I think Radiant the Cocoa app brings some nice features to the table as well as an existing user base, so I think it makes sense to accept some of that duplication.

Anyways, it's late for me, I'll stop rambling. Thoughts?

@MarshallOfSound
Copy link

@jacobwgillespie I don't know how to phrase this right but here goes 👍

Those bullet points you listed are almost the exact feature list of GPMDP, there is already a booming cross platform userbase over there and I feel that the duplication you talk about is simply spreading us thinly across doing the same thing in different ways.

I think there needs to be a deeper discussion into the duplication of pretty much entire applications 😄 Although we have shared work on gmusic.js and @chrismou has dived in to gmusic-theme.js recently it still doesn't make sense to me duplicating the entire application basically across two user bases and this kinda comes back to what I raised in radiant-player/radiant-player-electron#1

I'm not really sure what I'm thinking at the moment (bit late over here) but I definitely think there needs to be a discussion about the duplication.

@jacobwgillespie
Copy link
Member

I agree - I reopened radiant-player/radiant-player-electron#1 so we can continue the discussion there. 👍

@jacobwgillespie
Copy link
Member

The first "alpha" build of radiant-player-electron is available for download here: https://github.com/radiant-player/radiant-player-electron/releases. The README is a little out of date now, but this release is mainly the result of getting the build / package / release pipeline initially functional.

Some things that can be tested:

  • overall app performance
  • ability to use HTML 5 audio
  • ability to use visualizations
  • miniplayer
  • keyboard shortcuts
  • the custom menu bar (test drag performance, etc.)

Things still in progress:

  • settings dialog
  • LastFM scrobbling
  • themes
  • some gestures (i.e. two finger swipe for forward / back)

Known issues:

  • the playback position slider in the miniplayer "jumps" when changing positions

There's also more work to be done on the build pipeline. I removed some development tools (live reload, etc.) to get this initial build functional, so there's still some cleanup there as well. Additionally, auto-updates require signed applications, so I've been looking into that process as well. As a potential plus, perhaps we can distribute Radiant in the Mac App Store after we get a signing certificate.

Feedback appreciated!

@MarshallOfSound
Copy link

@jacobwgillespie Chiming in because I can 👍

As a potential plus, perhaps we can distribute Radiant in the Mac App Store after we get a signing certificate.

Unfortunately, no 😢 As per the info here in order to submit an app to the MAS ffmpeg must be removed as it contains dodgy codecs (including the MP3 codec that GPM uses)

some gestures (i.e. two finger swipe for forward / back)

Can you ping me RE this if you get it working, I've had nightmares about the code I've written trying to make this work 😆

@jacobwgillespie
Copy link
Member

As per the info here

Is that info out of date? There are official electron docs on MAS submission and the packager tools (electron-builder / electron-packager) provide MAS builds. It appears that the MAS build PR landed after that article was written. I can't seem to find a lot of info concerning ffmpeg though.

Can you ping me RE this if you get it working

Sure! I think I had some kind of initial version working, and I'll let you know when it's finalized. For the custom titlebar that I'm using, it's reaching out to native mouse tracking via a C module, and I think it's possible to grab gestures from the underlying APIs as well. You might want to check out Nylas N1's source as I think they have a working implementation of this.

@MarshallOfSound
Copy link

To be honest as far as I know, at the moment electron appd haven't been successfully submitted to the App Store since that blog post. There are still several issues (I think a few closed recently for 1.1.1) regarding apps being rejected.

Until someone reports success without removing ffmpeg we'll never know :). And I simply haven't tried 👍

@chrismou
Copy link
Member

@jacobwgillespie Sorry dude, been really busy recently. I'll try and find time to pull down master and have a play this week

@chrismou
Copy link
Member

chrismou commented May 30, 2016

@MarshallOfSound Is the ffmpeg codec included with Electron? Seems like a bit of an oversight if it means no Electron apps can ever make it to the App Store.

Might be some way we can pull it in on first install. Similar to how Ubuntu does/used to, when you first install and it asks you if you want to install "extras" to enhance the OS.

Though now I say that, I suspect the fact Ubuntu does that (or used at least used to) probably means the including it with a distribution is a bit murky, legal-wise.

@jacobwgillespie
Copy link
Member

@chrismou no worries, been busy myself too!

Also it looks like Electron apps are able to be distributed in the MAS - there's an official build path and docs, the third-party build tools have MAS targets, and apps like https://rocket.chat/ are available in the MAS. Seems like the issues have been resolved.

@chrismou
Copy link
Member

chrismou commented Jun 3, 2016

  • npm install
  • npm run watch
  • npm run start
Warning: Error: Could not get code signature for running application
    at Error (native)
    at eval (eval at <anonymous> (/Users/chrischrisostomou/Dev/radiant-player-electron/app/backend.js:114:2), <anonymous>:54:25)
    at Object.<anonymous> (/Users/chrischrisostomou/Dev/radiant-player-electron/app/backend.js:114:2)
    at __webpack_require__ (/Users/chrischrisostomou/Dev/radiant-player-electron/app/backend.js:21:30)
    at eval (eval at <anonymous> (/Users/chrischrisostomou/Dev/radiant-player-electron/app/backend.js:48:2), <anonymous>:5:1)
    at Object.<anonymous> (/Users/chrischrisostomou/Dev/radiant-player-electron/app/backend.js:48:2)
    at __webpack_require__ (/Users/chrischrisostomou/Dev/radiant-player-electron/app/backend.js:21:30)
    at /Users/chrischrisostomou/Dev/radiant-player-electron/app/backend.js:41:18
    at Object.<anonymous> (/Users/chrischrisostomou/Dev/radiant-player-electron/app/backend.js:44:10)
    at Module._compile (module.js:541:32)
App threw an error during load
Error: Module version mismatch. Expected 48, got 46.
    at Error (native)
    at process.module.(anonymous function) [as dlopen] (ELECTRON_ASAR.js:158:20)
    at Object.Module._extensions..node (module.js:568:18)
    at Object.module.(anonymous function) [as .node] (ELECTRON_ASAR.js:158:20)
    at Module.load (module.js:456:32)
    at tryModuleLoad (module.js:415:12)
    at Function.Module._load (module.js:407:3)
    at Module.require (module.js:466:17)
    at require (internal/module.js:20:19)
    at bindings (/Users/chrischrisostomou/Dev/radiant-player-electron/node_modules/bindings/bindings.js:76:44)

Haven't had chance to dig into the error, was only booting it up for a quick play while I'm in the office.

@MarshallOfSound
Copy link

@chrismou This is funny because I was going to link a relevant issue. But guess who wrote it 😆

MarshallOfSound/Google-Play-Music-Desktop-Player-UNOFFICIAL-#370

The second issue (module version mismatch) is due to you needed to rebuild native modules. I'm not sure how your build tooling is set up but normally you run some kind of electron-rebuild script 👍

@chrismou
Copy link
Member

chrismou commented Jun 3, 2016

Oh the irony. I thought that error looked familiar 😂

@jacobwgillespie
Copy link
Member

@chrismou just pushed a fix:

git pull origin master
rm -rf node_modules app
npm install

It should rebuild any native dependencies for electron after install now.

@chrismou
Copy link
Member

chrismou commented Jun 7, 2016

@jacobwgillespie Looking good so far. Definitely taking shape. I'm going to throw together some tickets on the electron repo to get the remaining jobs down and listed. Feel free to add anything I've missed!

I'm still a bit tied up with other work, but at least when I get some time I can assign myself a ticket so I know we're not duplicating effort. Probably worth you doing the same if you get a chance!

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

No branches or pull requests

8 participants