D10972: [RFC] Exposing slideshow to MPRIS controllers

[Henrik Fehlauer]

rkflx added a comment.


  Thanks for the updates, works even neater than before ;) Changes to the code look good to me (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…).
  
  > 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.
  
  > 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>.
  
  > 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…
  
  > And in case of unsaved changes (e.g. rotated images) ideally we would still create a matching custom thumbnail ourselves.
  
  I believe there are also some bugs where Gwenview gets confused and displays incorrect orientations itself.
  
  > When we later want to support MPRIS tracklist/playlist, ideally that mapping can be done in both directions, without overhead.
  
  I see. If I interpret the DBus spec right, the ID can be of arbitrary length, so your base64 approach is probably fine.
  
  > 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.
  
  > 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.
  
  ---
  
  > drop support for thumbnails urls in the metadata for now
  
  Good call.
  
  > 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.
  
  ---
  
  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.

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/ef967327/attachment.html>