let QMPlay2 handle youtube:XXYYZZ URIs

[RJVB]

There are times where I want to refer to a YouTube video in shorthand notation that isn't usually recognised as "direct link" (e.g. on discussion forums where direct linking to videos with specific content isn't allowed). The obvious choice is youtube:XXYYZZ where XXYYZZ is the video identifier.

This patch enables QMPlay2 to process such URIs.

(This PR is a bit for fun and/or just-in-case)

[RJVB]

Figured out how to register the scheme on *nix .

[zaps166]

The (stupid) idea is that QMPlay2 doesn't know about YT, because it's an extension. But I'll check it soon!

[RJVB]

It's as you see fit, of course!

[RJVB]

On Sunday July 30 2023 12:42:17 Błażej Szczygieł wrote:

Btw. do we really need the youtube:/ (single slash) ? :smile:

The comment about answers 'no' to that question ;) You'd prefer to test for the presence of a // and remove that?

else not needed - previous block of code returns - no need to add indentation and else.

Indeed. Old habit ("compiler might be able to generate more efficient code"), plus I find it makes it easier to get reacquainted with the code flow if you haven't been in that area for a while.

[zaps166]

Indeed. Old habit ("compiler might be able to generate more efficient code"), plus I find it makes it easier to get reacquainted with the code flow if you haven't been in that area for a while.

My compiler (clang) generates the same code w/o else (same checksum of the .so file).

I'd suggest (this should not trigger the undefined behavior and looks simpler + uses https):

QString Functions::maybeExtensionAddress(const QString &url)
{
    if (url.startsWith("youtube:"))
    {
        // we want to support both youtube:vID and youtube://vID but the latter form
        // is often passed to us as "youtube://vid" (vID interpreted as the remote host).
        // So we also support the full URI form with an empty host string and strip
        // the leading '/' from the path to recover the intact vID. This implementation
        // also supports youtube:/vID as a side effect ... why not!
        const auto vid = url.mid(8).remove('/');
        while (!vid.isEmpty())
            return maybeExtensionAddress("https://www.youtube.com/watch?v=" + vid);
    }

    for (const QMPlay2Extensions *QMPlay2Ext : QMPlay2Extensions::QMPlay2ExtensionsList())
    {
        const QString prefix = QMPlay2Ext->matchAddress(url);
        if (!prefix.isEmpty())
            return prefix + "://{" + url + "}";
    }
    return url;
}

[zaps166]

Or this:

diff --git a/src/modules/Extensions/YouTube.cpp b/src/modules/Extensions/YouTube.cpp
index d838ccb4..19e8dab4 100644
--- a/src/modules/Extensions/YouTube.cpp
+++ b/src/modules/Extensions/YouTube.cpp
@@ -436,7 +436,7 @@ bool YouTube::canConvertAddress() const
 QString YouTube::matchAddress(const QString &url) const
 {
     const QUrl qurl(url);
-    if (qurl.scheme().startsWith("http") && (qurl.host().contains("youtube.") || qurl.host().contains("youtu.be")))
+    if (qurl.scheme() == "youtube" || (qurl.scheme().startsWith("http") && (qurl.host().contains("youtube.") || qurl.host().contains("youtu.be"))))
         return "YouTube";
     if (qurl.scheme().startsWith("http") && qurl.host().contains("twitch.tv"))
         return "youtube-dl";
@@ -460,7 +460,7 @@ void YouTube::convertAddress(const QString &prefix, const QString &url, const QS
         if (ioCtrl && (stream_url || name))
         {
             auto &youTubeDl = ioCtrl->toRef<YouTubeDL>();
-            const QStringList youTubeVideo = getYouTubeVideo(param, url, youTubeDl);
+            const QStringList youTubeVideo = getYouTubeVideo(param, url.startsWith("youtube:") ? url.mid(8).remove('/') : url, youTubeDl);
             if (youTubeVideo.count() == 4)
             {
                 if (stream_url)

No need to modify QMPlay2, just the YT extension :smile:

[zaps166]

Btw. where youtube: (without //) is needed? Without this static QString fileArg(const QString &arg) doesn't need to be modified, too :slightly_smiling_face:

[RJVB]

Btw. where youtube: (without //) is needed?

Well, yes, that was in fact my original intention (think of the mailto: protocol). IIRC I've had to support the slashes because they could get added by the OS (probably the Mac's) but I cannot really remember.

[zaps166]

Btw. where youtube: (without //) is needed? Well, yes, that was in fact my original intention (think of the mailto: protocol). IIRC I've had to support the slashes because they could get added by the OS (probably the Mac's) but I cannot really remember.

On Linux (xdg-open) works correctly with slashes, so if macOS needs slashes, let's drop the : behavior and no need to additional conditions in Main.cpp :slightly_smiling_face:

[RJVB]

Sorry, I wasn't clear: as per the issue title it was my intention to support the syntax without slashes. You can argue that this is more appropriate too: "youtube" is not a service protocoI (think of it more as a hostnm as used with rsync) but there's also the concern to keep it simple for the user (me, for now ;)). I'll have to check if I was obliged to add support for the syntax with the slashes or if that was a conscious choice.

[zaps166]

Sorry, I wasn't clear: as per the issue title it was my intention to support the syntax without slashes. You can argue that this is more appropriate too: "youtube" is not a service protocoI (think of it more as a hostnm as used with rsync) but there's also the concern to keep it simple for the user (me, for now ;)). I'll have to check if I was obliged to add support for the syntax with the slashes or if that was a conscious choice.

Just keep the condition in Main.cpp and add https://github.com/zaps166/QMPlay2/pull/619#issuecomment-1658843334 instead of modifying Functions::maybeExtensionAddress :slightly_smiling_face: