Bug 436710 - Playing of Flash-free ("plain") Rich Media annotations
Summary: Playing of Flash-free ("plain") Rich Media annotations
Status: ASSIGNED
Alias: None
Product: okular
Classification: Applications
Component: PDF backend (show other bugs)
Version: 21.04.0
Platform: Arch Linux Linux
: NOR normal
Target Milestone: ---
Assignee: Okular developers
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2021-05-06 21:50 UTC by Michal Vlasák
Modified: 2024-08-16 14:08 UTC (History)
3 users (show)

See Also:
Latest Commit:
Version Fixed In:
Sentry Crash Report:


Attachments
poppler merge requests (41.81 KB, image/png)
2021-05-08 21:34 UTC, Michal Vlasák
Details
okular merge requests (48.20 KB, image/png)
2021-05-08 21:34 UTC, Michal Vlasák
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Michal Vlasák 2021-05-06 21:50:51 UTC
Okular currently has a compatibility layer for "playing" video/audio that was handled by Flash player in other readers. However it can't play "plain" Rich Media (of subtypes "Video" and "Sound").

I though this would be easy to add, but it turns out poppler currently doesn't expose the needed information (https://gitlab.freedesktop.org/poppler/poppler/-/merge_requests/855). After this is resolved, it doesn't take much code to extend the support.

My attempt (requiring the linked above changes to poppler) is below. Note that I couldn't get it to really work. Apart from the poppler patch being currently wrong I may have even not loaded the proper recompiled binaries/libraries of poppler/Okular (I somehow couldn't even get debug prints to work).

--- a/generators/poppler/generator_pdf.cpp
+++ b/generators/poppler/generator_pdf.cpp
@@ -250,6 +250,8 @@ QPair<Okular::Movie *, Okular::EmbeddedFile *> createMovieFromPopplerRichMedia(c
      * Flash/Video richmedia instance and parse the flashVars parameter for the 'source' identifier.
      * That identifier is then used to find the associated embedded file through the assets
      * mapping.
+     *
+     * For plain Video/Sound, we just get the asset directly from the instance.
      */
     const Poppler::RichMediaAnnotation::Content *content = popplerRichMedia->content();
     if (!content)
@@ -267,43 +269,49 @@ QPair<Okular::Movie *, Okular::EmbeddedFile *> createMovieFromPopplerRichMedia(c
 
     const Poppler::RichMediaAnnotation::Instance *instance = instances[0];
 
-    if ((instance->type() != Poppler::RichMediaAnnotation::Instance::TypeFlash) && (instance->type() != Poppler::RichMediaAnnotation::Instance::TypeVideo))
+    if ((instance->type() != Poppler::RichMediaAnnotation::Instance::TypeFlash)
+		    && (instance->type() != Poppler::RichMediaAnnotation::Instance::TypeVideo)
+		    && (instance->type() != Poppler::RichMediaAnnotation::Instance::TypeSound))
         return emptyResult;
 
     const Poppler::RichMediaAnnotation::Params *params = instance->params();
-    if (!params)
-        return emptyResult;
 
-    QString sourceId;
     bool playbackLoops = false;
-
-    const QStringList flashVars = params->flashVars().split(QLatin1Char('&'));
-    for (const QString &flashVar : flashVars) {
-        const int pos = flashVar.indexOf(QLatin1Char('='));
-        if (pos == -1)
-            continue;
-
-        const QString key = flashVar.left(pos);
-        const QString value = flashVar.mid(pos + 1);
-
-        if (key == QLatin1String("source"))
-            sourceId = value;
-        else if (key == QLatin1String("loop"))
-            playbackLoops = (value == QLatin1String("true") ? true : false);
-    }
-
-    if (sourceId.isEmpty())
-        return emptyResult;
-
-    const QList<Poppler::RichMediaAnnotation::Asset *> assets = content->assets();
-    if (assets.isEmpty())
-        return emptyResult;
-
     Poppler::RichMediaAnnotation::Asset *matchingAsset = nullptr;
-    for (Poppler::RichMediaAnnotation::Asset *asset : assets) {
-        if (asset->name() == sourceId) {
-            matchingAsset = asset;
-            break;
+
+    if (!params && instance->type() != Poppler::RichMediaAnnotation::Instance::TypeFlash) {
+        // Plain Audio/Video, we have the asset here
+        matchingAsset = instance->asset();
+    } else {
+        QString sourceId;
+
+        const QStringList flashVars = params->flashVars().split(QLatin1Char('&'));
+        for (const QString &flashVar : flashVars) {
+            const int pos = flashVar.indexOf(QLatin1Char('='));
+            if (pos == -1)
+                continue;
+
+            const QString key = flashVar.left(pos);
+            const QString value = flashVar.mid(pos + 1);
+
+            if (key == QLatin1String("source"))
+                sourceId = value;
+            else if (key == QLatin1String("loop"))
+                playbackLoops = (value == QLatin1String("true") ? true : false);
+        }
+
+        if (sourceId.isEmpty())
+            return emptyResult;
+
+        const QList<Poppler::RichMediaAnnotation::Asset *> assets = content->assets();
+        if (assets.isEmpty())
+            return emptyResult;
+
+        for (Poppler::RichMediaAnnotation::Asset *asset : assets) {
+            if (asset->name() == sourceId) {
+                matchingAsset = asset;
+                break;
+            }
         }
     }
 

For testing PDF file see the linked poppler merge request.
Comment 1 Nate Graham 2021-05-07 19:29:31 UTC
Thanks; can you please submit code in the form of a merge request at https://invent.kde.org/graphics/okular/-/merge_requests?
Comment 2 Michal Vlasák 2021-05-08 16:14:32 UTC
(In reply to Nate Graham from comment #1)
> Thanks; can you please submit code in the form of a merge request at
> https://invent.kde.org/graphics/okular/-/merge_requests?

It seems that I can't create new merge request normally. Maybe "E-mail a new merge request to this project" will work, but I didn't try.

I didn't even think this is ready for merge request, yet:
 - it requires changes in poppler, that I didn't yet implement correctly,
 - there may be more to it, and I am probably not entitled to do the decision.

My patch was more or less a rough sketch of what somebody more knowledgeable may do. For instance, I changed the handling of "Instance::Type*" to allow even audio (which works fine in Okular IIRC) but I don't know how mismatched the Flash/Audio/Video type is in real PDFs (there probably was a reason why it was like set like that).

Another thing I remembered: currently Acrobat uses very similiar code to Okular's to play Flash video (it no longer uses Flash player). But because Flash-less Rich Media doesn't normally have controls it emulates it if it finds a certain "skin" in `/Params`. Doing same in Okular may bring it up to par with current Acrobat for old documents. Details can be found at https://gitlab.com/agrahn/media9/-/issues/9#note_554144989.

I also noticed, that Okular currently unconditionally uses first RichMediaConfiguration, which works for all usual cases, but handling this generally wouldn't be hard.

All in all, I would like to first get poppler to even allow for the code above and in the mean time get feedback from you guys to know whether it could work like this. I didn't even manage to get it running. Is there Okular/Qt way  I can use to assure myself that I am even running the code I added? I didn't find success with std::out.
Comment 3 Albert Astals Cid 2021-05-08 20:40:19 UTC
> It seems that I can't create new merge request normally.

You're going to need to be a bit more verbose on what's the problem you're having ;)
Comment 4 Michal Vlasák 2021-05-08 21:34:16 UTC
Created attachment 138260 [details]
poppler merge requests
Comment 5 Michal Vlasák 2021-05-08 21:34:34 UTC
Created attachment 138261 [details]
okular merge requests
Comment 6 Michal Vlasák 2021-05-08 21:37:38 UTC
(In reply to Albert Astals Cid from comment #3)
> > It seems that I can't create new merge request normally.
> 
> You're going to need to be a bit more verbose on what's the problem you're
> having ;)

Sorry for being unclear. I don't see the button normally used for creating merge requests. I thought it was intentionally limited only to orginization members.

(The difference between poppler/okular repositories is visible in files attached above).
Comment 7 Albert Astals Cid 2021-05-09 10:09:35 UTC
Please read https://community.kde.org/Infrastructure/GitLab#Submitting_a_merge_request
Comment 8 Michal Vlasák 2021-05-09 10:18:59 UTC
(In reply to Albert Astals Cid from comment #7)
> Please read
> https://community.kde.org/Infrastructure/GitLab#Submitting_a_merge_request

I am very sorry for wasting your time due to my mistake. I completely overlooked that I had not yet forked okular repository. I somehow got confused because I have previously done so for poppler.

Would it be worth to open this merge request when 'instance->asset()' is not yet possible with current poppler?
Comment 9 Albert Astals Cid 2021-05-09 10:44:55 UTC
Just do it and reference the other merge request, it will fail to compile and that's *good*
Comment 10 Bug Janitor Service 2021-05-13 21:41:31 UTC
A possibly relevant merge request was started @ https://invent.kde.org/graphics/okular/-/merge_requests/426