Skip to content
This repository has been archived by the owner on May 3, 2019. It is now read-only.

Add Wynk Music plugin #125

Merged
merged 4 commits into from
Oct 10, 2017
Merged

Add Wynk Music plugin #125

merged 4 commits into from
Oct 10, 2017

Conversation

shreyanshk
Copy link
Contributor

Proposed Changes

Add support for Wynk Music service

@coveralls
Copy link

Coverage Status

Coverage remained the same at 92.64% when pulling 8aefd81 on shreyanshk:develop into 819c37d on ColinDuquesnoy:develop.

@ColinDuquesnoy
Copy link
Owner

Thank you!

Do you know if that service require proprietary codecs to work? On which platform did you tested it?

May I ask you to change the service logo colors to match with other logos (gray background and white foreground)? Here is how it looks at the moment:

wynklogo

@shreyanshk
Copy link
Contributor Author

There is no dependency on any proprietary codec and this was tested on Arch Linux. Also, I have lightened the icon a little bit. Please review.

Thank you.

@ColinDuquesnoy
Copy link
Owner

There is no dependency on any proprietary codec and this was tested on Arch Linux.

If you tested it on archlinux with the AUR package there might be a dependency on proprietary codecs you are not aware of (arch enable them by default). A good way to check if proprietary codecs are needed is to run your plugin from the AppImage release. If it works then it means it does not require qtwebengine to be compiled with proprietary codecs support.

I have lightened the icon a little bit. Please review.

Thank you. This looks much better now. Maybe I am litlle nitpicky but I'd also remove the white border on the edge of the gray circle and put all waves in white (like they do during the loading of the website).

@coveralls
Copy link

Coverage Status

Coverage remained the same at 92.64% when pulling 23ff97b on shreyanshk:develop into 819c37d on ColinDuquesnoy:develop.

@shreyanshk
Copy link
Contributor Author

The icon has been updated.
And please guide me on how to make an AppImage release for this application?
Never heard about AppImage before.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 92.64% when pulling befc08a on shreyanshk:develop into 819c37d on ColinDuquesnoy:develop.

@ColinDuquesnoy
Copy link
Owner

ColinDuquesnoy commented Sep 30, 2017

The icon has been updated.

Thank you.

And please guide me on how to make an AppImage release for this application?

Official AppImages are available from the release section: https://github.com/ColinDuquesnoy/MellowPlayer/releases

Just download it, make it executable (chmod +x MellowPlayer-x86_64.AppImage) and run it.

You can read more about what an AppImage is here: https://appimage.org/

@ColinDuquesnoy
Copy link
Owner

@ZeroDot1 Testing with the AppImage is the only way I found to check if a service require proprietary codecs to work, because the AppImage is built with a version of QtWebEngine compiled without support for proprietary codecs. So, @shreyanshk please test your plugin with an AppImage, not the way described by @ZeroDot1

@shreyanshk
Copy link
Contributor Author

There seems to be some dependency on external library as the AppImage couldn't play any audio and became stuck at the buffering stage on Wynk but Youtube seems to be working just fine.
For the record, I have only installed google-widevine plugin for chromium and fyi, pepper-flash package is not installed.

@shreyanshk
Copy link
Contributor Author

shreyanshk commented Oct 2, 2017

https://justpaste.it/1bws4 is the list of packages installed on the computer (ouput of pacman -Qq). no debs were ever manually extracted on the root filesystem or any package management related task was done using pacman.

Please help me identify the dependency.

@ColinDuquesnoy
Copy link
Owner

Thank you for testing the AppImage. So most likely Wynk relies on some proprietary codecs to decode html5 audio tags, those proprietary codecs are supplied by the ffmpeg package which is not used by the AppImage because the version of QtWebEngine statically links to a version of ffmpeg compiled without support for proprietary codecs (MP3, MPEG-4,...).

So the require_proprietary_codecs (from metadata.ini) should be set to true. Note that in version 3.1, this key has been replaced by a list of supported platforms and should be set to Linux only. See #115 (example of service that require proprietary codecs: https://github.com/ColinDuquesnoy/MellowPlayer/blob/develop/plugins/mixcloud/metadata.ini#L5)

@shreyanshk
Copy link
Contributor Author

Added supported_platforms field and kept require_proprietary_codecs key for backward compatibility purposes.
FYI: I can confirm that this service uses mp3 format so probably that is where the dependency is.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.007%) to 92.632% when pulling d04392e on shreyanshk:develop into 819c37d on ColinDuquesnoy:develop.

@ColinDuquesnoy ColinDuquesnoy merged commit 9717a32 into ColinDuquesnoy:develop Oct 10, 2017
@ColinDuquesnoy
Copy link
Owner

I changed the supported-platform key to Linux only. Thank you for your contribution!

@cpjeanpaul cpjeanpaul mentioned this pull request Jul 10, 2018
@ColinDuquesnoy
Copy link
Owner

@shreyanshk A user reported that the Wynn plugin is now broken. Could you have a look at https://gitlab.com/ColinDuquesnoy/MellowPlayer/issues/337
You won’t be able to answer here on github as the repository is archived.

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

Successfully merging this pull request may close these issues.

3 participants