<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/126214/">https://git.reviewboard.kde.org/r/126214/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On desembre 2nd, 2015, 5:11 p.m. UTC, <b>Sumit Sahrawat</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<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;">Looking at the code, it seems like the 'Stop Sound' option only appears when sound is playing. Which means that there is nothing left to be done before the enqueuing bit.</p></pre>
</blockquote>
</blockquote>
<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;">After reading the code and thinking a bit more about it, I am not sure the enqueuing makes sense. Okular is not a music player in which you're going to create a playlist, if you have a sound you'll want to play it or not, i'd say the use case for "I want to play this but after this finishes" is quite small and not sure we should worry about it.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">What do you think?</p></pre>
<br />
<p>- Albert</p>
<br />
<p>On desembre 1st, 2015, 12:33 p.m. UTC, Sumit Sahrawat 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 Okular and Albert Astals Cid.</div>
<div>By Sumit Sahrawat.</div>
<p style="color: grey;"><i>Updated des. 1, 2015, 12:33 p.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Bugs: </b>
<a href="http://bugs.kde.org/show_bug.cgi?id=326536">326536</a>
</div>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
okular
</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;">I understood the bug as asking for a feature to enqueue sounds if sound is currently playing.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I did not understand why a QHash was being used to manage playbacks, which confused me:</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Q: Does okular support simultaneous playback of multiple sounds?</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">If so, I don't think both simultaneous output and enqueuing can be supported in an intuitive way using the right click menu. I've replaced the old QHash with a QQueue, which acts as a playlist of sounds to be played.</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;">No testing done. I couldn't find a document with many sound annotations. I thought I'd create one using Okular, but it doesn't support that yet.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Albert: What do you use to test this? Can you provide me a link to the same document?</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>core/audioplayer.h <span style="color: grey">(b335b5e)</span></li>
<li>core/audioplayer.cpp <span style="color: grey">(ab735f3)</span></li>
<li>core/audioplayer_p.h <span style="color: grey">(32fe4de)</span></li>
<li>core/document.cpp <span style="color: grey">(6953b1f)</span></li>
<li>ui/pageview.cpp <span style="color: grey">(3ebf7dc)</span></li>
<li>ui/presentationwidget.cpp <span style="color: grey">(e6ecdc5)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/126214/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>