D10972: [RFC] Exposing slideshow to MPRIS controllers

Wed Mar 14 00:54:16 UTC 2018

kossebau marked an inline comment as done.
kossebau added a comment.

  In D10972#225101 <https://phabricator.kde.org/D10972#225101>, @rkflx wrote:

  >   (even though you sneaked in more changes than announced, making me wonder whether some parts could be shared with other users like Okular in the future…).

  You mean D-Bus registration code? Indeed, forgot to mention that, sorry. This was feedback from issues found with the parallel patch for Okular, where the Plasma MPRIS dataengine sometimes ignored the appeared service. Reason is that as I now learned since Qt 5.6 D-Bus registration is done in an own thread, so not waiting on main event loop. Thus in the moment the service appears on the D-Bus with the well-known name, any other process which sees that can already make requests to it. Just if the D-Bus object as the code has not yet advanced to to their registrartion, the other process' request will fail with error, and they will then tend to ignore the service. So the order is now important and needs to be: first prepare objects, then announce service by name :)

  >> For now I would just opt for: say No to screen locker media controls in general.
  > 
  > On Tumbleweed this worked (ignoring the fact that normal users will have a hard time digging for the option), but on Neon and Bionic that config setting was missing. I'm pretty sure that's not a custom openSUSE patch, so probably an issue in the theming support on the other distros?
  > 
  > I guess my question was more about the how, not about the if. I cannot see any reason to show Gwenview on the lockscreen, why would you want to navigate images which are hidden behind the lockscreen anyway? I anticipate we'll get bug reports about this, because simply having Gwenview open (regardless of fullscreen state, and that's good!) will trigger this behaviour.
  > 
  > Note that Gwenview inhibits power management (in theory, did not test), so even the use case of stopping the slideshow from the lockscreen when you've fallen asleep does not apply.

  In case of video playing (with sound) this could be even wanted ;) (have some old music videos directories which I sometimes play just for the music).

  Will ask tomorrow the Plasma people what could be done here.

  >> Any pointers in the Gwenview code where one could read-out the matching paths to the thumbnail in the cache?
  > 
  > Grepping for `256` I quickly ended up in `generateThumbnailPath` <https://phabricator.kde.org/source/gwenview/browse/master/lib/thumbnailprovider/thumbnailprovider.cpp;d883cdcdc1ea97915a39814bfe8d358fbb1a06fe$72>.

  Yes, thanks, meanwhile also have some first experimental code which hooks into this. Though plan to only work more on this when I get to "playlist"/"tracklist" support and for that  also  work on extending kdeconnect mpris plugin to allow scanning through the list, as then the thumbnail views will be mainly interesting (same with Okular for the pages). Planned for somewhen in the summer, when it is again feature development time.

  >> Though some thumbnailers might only generate thumbnails on the fly (official option, e.g. when there are already thumbnails embedded in the file itself).
  > 
  > Did not read that part of the codebase yet, but I'd assume there are three sources for thumbnails: embedded, cached and computed; and there's also some smoothing going on in a delayed step (if you observe the rendering carefully). Then we have the problem that per FDO spec the cache maxes out at 256px, which clashes with HiDPI requirements.
  > 
  > Perhaps we should stretch the spec a bit by caching thumbnails regardless of type and in higher resolution, so that the cache would always contain a matching entry? This needs more investigation at a later time…

  Yes. Possibly this is something to do in sync with FDO & Co., given HiDPI is also interesting for thumbnails in other file managers/browsers. Big note made.

  >> I saw that "Auto-Advance" seems to be a term that can be found in image/slides related software, perhaps worth a consideration.
  > 
  > The only place the string is used besides tooltips is the menu entry which also triggers fullscreen, i.e. "Start Slideshow". Renaming to "Activate Auto-Advance and switch to Fullscreen" is a bit unwieldy.

  :) Just my 2 cents.

  >> The folder name could be mapped onto "xesam:album" semantics instead of the "Gwenview Slideshow". What would be a good source to read the folder name from?
  > 
  > I'm not aware of any special handler for that, so your best bet might be `adjust`ing the URL.

  Given Gwenview thankfully supports KIO protocols, there might be url schemes where the "path" can not be simply decomposed into hierarchy patterns from the file url itself. Though for now we could simply first test for localFile, then do normal filesystem query, otherwise just do a good guess with taking xxx/(FOLDER)/filename.foo.bar match from the path. Ah, or just see if the semantical dir model has a folder property? Will have a look into that.

  >  ---
  > 
  >> drop support for thumbnails urls in the metadata for now
  > 
  > Good call.

  Okay, glad you agree.

  >> I want to focus this first version of MPRIS support on enabling remote control of the "slideshow"/image browsing by the navigation and play/pause/stop commands.
  > 
  > I found one more niggle, but this one could also be fixed later: The stop action should be disabled once the slideshow is stopped, i.e. in terms of our current behaviour when Gwenview is in windowed mode. VLC and JuK seem to support this when testing the mediacontroller applet.

  Okay, no problem, will change.
  In the MPRIS patch for Okular even I initially propose to simply remove the MPRIS service completely when not in presentation mode. Based on the idea that in normal reading mode one does not want any remote mediaplayer-like navigation, but instead interacts using the normal custom UI of Okular. Only once the presentation mode is activated and entered on, the MPRIS service is registered.

  Perhaps this model might make sense for Gwenview as well? So no exposure as MPRIS in windowed mode?

  >  ---
  > 
  > To summarize: Patch seems to be able to make it into the Beta in time. Folder name and stop button are not really important, but I'm still feeling a bit uneasy about the lockscreen issue.

  Sounds good. Will see to get more info about the lockscreen thing tomorrow. Hoping you are around again the night again, so you can hopefully give the final blessing to get this patch in for the Dependency Freeze on Thursday, given this adds the (optional) dependency on QtDBus coming with it :)

REPOSITORY
  R260 Gwenview

REVISION DETAIL
  https://phabricator.kde.org/D10972

To: kossebau, #gwenview
Cc: mtijink, ngraham, nicolasfella, #kde_connect, rkflx, broulik
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdeconnect/attachments/20180314/c51c4a2a/attachment-0001.html>