<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="12" style="border: 1px #c9c399 solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="https://git.reviewboard.kde.org/r/127130/">https://git.reviewboard.kde.org/r/127130/</a>
</td>
</tr>
</table>
<br />
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I think the intention was that when one goes to the loadView the video is paused and only the thumbnail is shown. I am rather indifferent though, so simply letting it continue playing is fine by me as well.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">To have this work correctly you have to adjust the MainWindow::engineStateChanged loadView toggle to only run on state == Playing though.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">With the proposed patch the behavior is somewhat weird:
- playing
- click on play media
- continues playing
- manually pause
- dropped back to videoView</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I think what should be is:
- playing
- click on play media
- continues playing
- manually pause
- dragon stays on load view</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Changinge the engineStateChanged behavior to only work on playing is still a bit unfortunate though. In particular in the use case:
- playing
- click on play media
- continues playing
- manually pause
- stay in load view
- unpause again
- you are now dropped to videoView again</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I think it's fine because the way I see it one would pause the video because one got annoyed or bored with it and wants to look for something else instead, so the only scenario where one would unpause it again while being in loadView is because one didn't find anything better, at which point one would have to continue watching the current video anyway.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">So yeah, please change engineStateChanged to only toggle on playing and this is going to be perfect.</p></pre>
<br />
<p>- Harald Sitter</p>
<br />
<p>On March 28th, 2016, 7:41 p.m. UTC, Anthony Fieroni wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="12" style="border: 1px #888a85 solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">
<tr>
<td>
<div>Review request for KDE Multimedia, David Edmundson and Harald Sitter.</div>
<div>By Anthony Fieroni.</div>
<p style="color: grey;"><i>Updated March 28, 2016, 7:41 p.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
dragon
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">toggleLoadView in video mode not stop playback and even make 'blink' mainview frame</p></pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Now preview works correctly.</p></pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>src/app/mainWindow.cpp <span style="color: grey">(777467b)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/127130/diff/" style="margin-left: 3em;">View Diff</a></p>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">File Attachments </h1>
<li><a href="https://git.reviewboard.kde.org/media/uploaded/files/2016/03/28/2280d4bb-75f8-4f84-aa50-e857fe84a99b__Screenshot_20160328_223543.png">Screenshot_20160328_223543.png</a></li>
</ul>
</td>
</tr>
</table>
</div>
</body>
</html>