<table><tr><td style="">kossebau added inline comments.
</td><a style="text-decoration: none; padding: 4px 8px; margin: 0 8px 8px; float: right; color: #464C5C; font-weight: bold; border-radius: 3px; background-color: #F7F7F9; background-image: linear-gradient(to bottom,#fff,#f1f0f1); display: inline-block; border: 1px solid rgba(71,87,120,.2);" href="https://phabricator.kde.org/D10972">View Revision</a></tr></table><br /><div><strong>INLINE COMMENTS</strong><div><div style="margin: 6px 0 12px 0;"><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D10972#inline-54773">View Inline</a><span style="color: #4b4d51; font-weight: bold;">rkflx</span> wrote in <span style="color: #4b4d51; font-weight: bold;">mprismediaplayer2.cpp:46-49</span></div>
<div style="margin: 8px 0; padding: 0 12px; color: #74777D;"><p style="padding: 0; margin: 8px;">Is there a fundamental reason for that, or is this just a <tt style="background: #ebebeb; font-size: 13px;">TODO</tt>?</p>

<p style="padding: 0; margin: 8px;">Also wouldn't it be good to call this upon "Play"?</p></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">More a TODO, had not yet started to think about how and where this could be useful. Also not yet seen how existing controllers make use of this feature.</p>

<p style="padding: 0; margin: 8px;">To call this on "Play" would be a decision of the controller. From my reading of the spec this seems more about bringing any specific media app controller UI onto the screen, not the actually rendered media (though usually apps have the bundled in their UI). But room for interpretation. Then in Wayland apps might not be able to autoraise their windows.</p>

<p style="padding: 0; margin: 8px;">Added an explicit todo comment now, so  future code reader can continue from here.</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D10972#inline-54774">View Inline</a><span style="color: #4b4d51; font-weight: bold;">rkflx</span> wrote in <span style="color: #4b4d51; font-weight: bold;">mprismediaplayer2.cpp:51-57</span></div>
<div style="margin: 8px 0; padding: 0 12px; color: #74777D;"><p style="padding: 0; margin: 8px;">Why is this connected to the notion of a (auto-advancing) slideshow? If this is only about a fullscreen toggle, this could be set to <tt style="background: #ebebeb; font-size: 13px;">true</tt>. Switching tracks already works fine in windowed mode.</p>

<p style="padding: 0; margin: 8px;">In windowed mode, it would probably still make sense to make the "Play" action enter fullscreen like the corresponding "Start Slideshow" action in Gwenview does (see other part of the reply).</p>

<p style="padding: 0; margin: 8px;">Conversely, switching back to windowed mode from fullscreen should stop the slideshow, just like Gwenview does.</p></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">It was connected to things as by my initial mind model of gwenview states :)</p>

<p style="padding: 0; margin: 8px;"><a href="https://specifications.freedesktop.org/mpris-spec/latest/Media_Player.html#Property:Fullscreen" class="remarkup-link" target="_blank" rel="noreferrer">https://specifications.freedesktop.org/mpris-spec/latest/Media_Player.html#Property:Fullscreen</a> does not specify whether requesting a change of fullscreen state should result also in playbackmodus change, so should be fine to do as you prefer.</p>

<p style="padding: 0; margin: 8px;">Not aware of an controllers which make use of this, so only tested via qtdbusviewer.</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D10972#inline-54768">View Inline</a><span style="color: #4b4d51; font-weight: bold;">rkflx</span> wrote in <span style="color: #4b4d51; font-weight: bold;">mprismediaplayer2.h:20-21</span></div>
<div style="margin: 8px 0; padding: 0 12px; color: #74777D;"><p style="padding: 0; margin: 8px;"><tt style="background: #ebebeb; font-size: 13px;">MPRISMEDIAPLAYER2_H</tt>?</p></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">Was some last trace of this feature patch some years ago initially drated for Calligra Stage (which uses KPR from former KPresenter name as prefix :) )</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D10972#inline-54785">View Inline</a><span style="color: #4b4d51; font-weight: bold;">rkflx</span> wrote in <span style="color: #4b4d51; font-weight: bold;">mprismediaplayer2player.cpp:270</span></div>
<div style="margin: 8px 0; padding: 0 12px; color: #74777D;"><p style="padding: 0; margin: 8px;">Would it be possible to include the folder name somewhere?</p></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">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?</p></div></div></div></div></div><br /><div><strong>REPOSITORY</strong><div><div>R260 Gwenview</div></div></div><br /><div><strong>REVISION DETAIL</strong><div><a href="https://phabricator.kde.org/D10972">https://phabricator.kde.org/D10972</a></div></div><br /><div><strong>To: </strong>kossebau, Gwenview<br /><strong>Cc: </strong>mtijink, ngraham, nicolasfella, KDE Connect, rkflx, broulik<br /></div>