<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 />
<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;">You are right, the code seems to be architected around the fact that more than one sound can be played at the same time and sounds can be repeated when finished.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">You're removed the possibility of the repeating as far as i can see.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">For now let's make this simpler and make it so that stop sounds actually only appears if it's playing, and we will think the enqueue a bit more after landing this. ok?</p></pre>
<br />
<div>
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="https://git.reviewboard.kde.org/r/126214/diff/1/?file=420308#file420308line69" style="color: black; font-weight: bold; text-decoration: underline;">core/audioplayer.h</a>
<span style="font-weight: normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">class OKULAR_EXPORT AudioPlayer : public QObject</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">68</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="kt">void</span> <span class="nf">stopPlayback<span class="hl">s</span></span><span class="p">();</span></pre></td>
<th bgcolor="#e9eaa8" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">69</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="kt">void</span> <span class="nf">stopPlayback</span><span class="p">();</span></pre></td>
</tr>
</tbody>
</table>
<div style="margin-left: 2em;">
<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;">Do not change public API without a good reason (it's a typo is not a good reason :D)</p></pre>
</div>
</div>
<br />
<p>- Albert Astals Cid</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>