Anghami

[mahmoudhossam]

This adds support for Anghami, a music streaming service in the MENA region.

[coveralls]

Coverage remained the same at 92.784% when pulling f8f2380 on mahmoudhossam:anghami into 45a0d70 on ColinDuquesnoy:develop.

[ColinDuquesnoy]

Thank you! I have a left a few code review comments.

It would also be nice if you could adapt the logo to match the style used in the application (grey circle with white foreground).

[ColinDuquesnoy]

Are you sure that service does not require proprietary codecs? On which platform did you tested it?

[mahmoudhossam]

I tested it on Vivaldi on Arch Linux, without the widevine plugin.

Is there a way to be sure it doesn't require any proprietary codecs?

[ColinDuquesnoy]

ffmpeg is the package that provides the proprietary codecs but you won't be able to remove it easily as it will also remove qtwebengine (and many other packages...)

I'd suggest to run your plugin from the MellowPlayer AppImage (the AppImage should find your plugin automatically).

[mahmoudhossam]

@ColinDuquesnoy I'll check out the AppImage, thanks.

[mahmoudhossam]

@ColinDuquesnoy I tried the AppImage and it didn't play apparently because pepper flash is required.

Is that considered a proprietary codec?

There's nothing much on this topic in the documentation, and other services don't seem to have this flag set at all.

[ColinDuquesnoy]

There are two cases to consider:

1. the service uses flash. In that case, yes, flash will provide the proprietary codecs. This is not an issue on most platforms because you just need to install pepper flash plugin (it will work from an AppImage). On ArchLinux, just install pepper-flash

2. The service use HTML 5 audio/video capabilities. In that case, proprietary codecs are provided by ffmpeg. It will not work in an AppImage (and on Windows and OSX) because QtWebEngine statically links with a version of ffmpeg compiled without proprietary codecs support (for licensing reasons).

There is a bit of documentation about that in the readme and on the qtwebengine documentation. Apparently there is not enough documentation so I will include a section that talk about that in the developer documentation.

other services don't seem to have this flag set at all.

Mixcloud, soundcloud and spotify plugins set this flag to true (in the master branch). On the develop branch, I replaced this flag by a list of supported platforms to avoid confusion (for end users). See #115.

[mahmoudhossam]

@ColinDuquesnoy I have pepper-flash installed on Arch Linux and the AppImage still does not play.

I get this warning in the AppImage output though:

[WARNING:flash/platform/pepper/pep_module.cpp(63)] SANDBOXED

[ColinDuquesnoy]

Ok so most likely the service needs codecs from ffmpeg (I assume the service works with the mellowplayer AUR package).

[ColinDuquesnoy]

Don't worry about the flag. I'll set it up myself.

[ColinDuquesnoy]

Don't worry about the flag. I'll set it up myself.

[ColinDuquesnoy]

songId must be set. You can use the below method to generate a unique id based on the song title:

function getHashCode(s) {
    return s.split("").reduce(function(a, b) {
        a = ((a << 5) - a) + b.charCodeAt(0);
        return a & a
    }, 0);
}

[mahmoudhossam]

I couldn't figure out what songID is just by looking at the documentation, can you please make it more clear like this comment? thanks.

[ColinDuquesnoy]

You're right, the documentation is not clear. I've updated it. Thank you for the feedback!

[mahmoudhossam]

@ColinDuquesnoy You're welcome.

[ColinDuquesnoy]

Are you sure you cannot get song duration and position? It is easy to convert a duration string to a number of seconds using the below function:

function toSeconds(string) {
    try {
        var dtimes = string.split(":");
        var dminutes = dtimes[0];
        var dseconds = dtimes[1];
        var duration = parseInt(dseconds, 10) + (parseInt(dminutes, 10) * 60);
    } catch (e) {
        var duration = 0;
    }

    return duration
}

[mahmoudhossam]

Anghami prints elapsed and remaining time only, so I think I can get the position easily, but the duration is going to need a bit of work.

Player shown below

[ColinDuquesnoy]

I did a similar thing for Mixcloud. I just added the elapsed time to the remaining time to get the duration.

[mahmoudhossam]

@ColinDuquesnoy Okay, I'll do that then.

[mahmoudhossam]

@ColinDuquesnoy I'll work on the comments when I get home, but I don't think I'll be able to do the logo thing because I'm not that good with GIMP unfortunately.

[ColinDuquesnoy]

I don't think I'll be able to do the logo thing because I'm not that good with GIMP unfortunately.

Ok that's not a big deal. I'll ask @CelineThiry to adapt the icon.

[mahmoudhossam]

@ColinDuquesnoy I think I addressed all the issues with the integration script, take a look.

[coveralls]

Coverage increased (+0.03%) to 92.81% when pulling 269e50b on mahmoudhossam:anghami into 45a0d70 on ColinDuquesnoy:develop.

[coveralls]

Coverage increased (+0.03%) to 92.81% when pulling 1fae462 on mahmoudhossam:anghami into 45a0d70 on ColinDuquesnoy:develop.

[ColinDuquesnoy]

Thank you!