<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/119201/">https://git.reviewboard.kde.org/r/119201/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On Июль 9th, 2014, 9:52 п.п. MSK, <b>Gilles Caulier</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;">Which CmakeFiles do you patch exactly ?</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">There are 3 parts in digiKam which use OpenCV :</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">1/ digiKam core<br style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;" />
2/ Kipi-plugins (RemoveRedEyes tool)<br style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;" />
3/ libkface</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">For 1 and 2 i think it's fine. But for libkface, i'm sure no, because i tried to reduce OpenCV fingerprint to speed up compilation on my computer (2.4.9 version), and digiKam crash due to a side effect in OpenCV (through a simple std::memcpy() call). With all default components, OpenCV do not crash.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">See my comment here :</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">https://bugs.kde.org/show_bug.cgi?id=336650#c3</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Gilles Caulier</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;">I agree that OpenCV components are a mess. I was not aware about a problem with libkface and will take a closer look then.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">In OpenBSD, we have separate packages for (1), (2) and (3), so it still makes sense for us to have explicit dependencies. But it looks like we need to separate OpenCV detection somehow. Probably calling find_package(OpenCV [...]) on lower layers should be enough, but this needs verification. I'll update review after some experiments.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Thank you for input!</p></pre>
<br />
<p>- Vadim</p>
<br />
<p>On Июль 9th, 2014, 9:26 п.п. MSK, Vadim Zhukov 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 Digikam.</div>
<div>By Vadim Zhukov.</div>
<p style="color: grey;"><i>Updated Июль 9, 2014, 9:26 п.п.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
digikam
</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;">This makes Digikam being explicit in what OpenCV components are needed. This results in less number of libraries to be linked to. As a result, Digikam starts a bit faster, eats a bit less memory and makes downstream (packagers) a bit more happy. Also it helps in cases when OpenCV package is split (for example, with non-free components separated) - without explicit mentioning of what components are needed, it's not clear will Digikam link to some components or not.</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;">OpenBSD/i386-CURRENT, KDE 4.11.5+Digikam 3.x and KDE 4.13.2+Digikam 4.x</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>CMakeLists.txt <span style="color: grey">(d591a2f)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/119201/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>