[prev in list] [next in list] [prev in thread] [next in thread]
List: kde-release-team
Subject: Re: Review Request 110962: Switch to an external LibRaw
From: "Pino Toscano" <pino () kde ! org>
Date: 2013-06-11 20:44:58
Message-ID: 20130611204458.30437.18446 () vidsolbach ! de
[Download RAW message or body]
[Attachment #2 (multipart/alternative)]
> On June 11, 2013, 6:38 p.m., Gilles Caulier wrote:
> > libraw detection can be not enough.
> >
> > if you look in libkdcraw, you will see RawSpeed code backported too.
> >
> > http://rawstudio.org/blog/?p=800
> >
> > This code is to speed up raw data extraction. It's not to process demosaicing.
> >
> > Here on my linux Mageia, librawspeed is not available. I don't know if other distro support this \
> > library.
> > LibRaw < 0.15 will not support RawSpeed. You must take a care about. In this case RawSpeed must be \
> > disabled.
> > Also, settings from raw decoding container and widget have been validated with libraw 0.15.x. So do \
> > not accept an older version of libraw, else there will be serious problems...
> > Gilles Caulier
>
> Pino Toscano wrote:
> > if you look in libkdcraw, you will see RawSpeed code backported too.
>
> Yet another copy of some 3rd party source?! Sigh.... It would be really helpful if your development way \
> would not start by embedding some 3rd party library in the sources of some KDE application/library...
> For the rest: OK, it is fine to accept only LibRaw >= 0.15. (I suspect they would work the same, but oh \
> well...)
> Gilles Caulier wrote:
> The reason to include libraw is simple : libraw API change agian and again, and i don't won't to puzzle \
> libkdcraw source code with an amount of conditional rules. As i'm in contact with libraw team, i'm \
> aware of plan and this is why i take a care to not have a shared component as libkdcraw which provide a \
> broken binary compatibility with a lots of crash... Just my experience.
> Typicially, libraw as experiental code which are tested and removed if to much problem are reported...
>
> There is also another problem : how to be sure that 3rd party codec from libraw are compiled in shared \
> version of system : There are 2 packs : one GPL2, one GPL3. These packages add new demosaicing methods \
> and camera support. If libraw is not compiled with these components some parts of libkdcraw will not \
> work as well. This will be very problematic for end users...
> This is another reason about libraw inclusion in libkdcraw. Like this i'm sure that all features are \
> included...
> Libraw is a big puzzle. Take a care...
>
> Gilles Caulier
>
> Pino Toscano wrote:
> If the API changes, then the version macros can be used to keep compatibility with multiple versions; \
> it seems the libraw people are not breaking the API much lately, so hopefully the libkdcraw code won't \
> be filled with a lot of these checks.
> Regarding the binary compatibility: if the libraw people bump SONAME/SOVERSION whenever the ABI \
> changes, that's perfectly fine and will not cause issues (maybe just to self-compiling users which have \
> no clue on what they are doing). If they break ABI without handling SONAME/SOVERSION bumps, that's \
> their fault which they ought to solve.
> Regarding the extra packs: are those compiled/provided by default to libraw users? If they could cause \
> licensing issues/incompatibilities, then compiling them as part of libkdcraw will *not* change their \
> situation.
> Gilles Caulier wrote:
> Extra GPL2/GPL3 pack are not included in libraw in standard due to mixing licensing problem. It's \
> separated packages. So distro packagers must take a care about and compile all as well.
> It's the same problem with RawSpeed which use another licensing than libraw core...
>
> Gilles Caulier
So the inclusion of these packs in libkdcraw *is* creating a licensing issue to distro packagers already? \
Great...
Gilles: putting everything in one source (libkdcraw) does not make
a) licensing issues
b) incompatibilities
c) shipping issue
go away at the same time, as you're creating new issues instead.
Now, if you could help testing this in the way you like, we could cleanup all this embedding mess in \
libkdcraw. Thank you.
- Pino
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/110962/#review34173
-----------------------------------------------------------
On June 11, 2013, 5:50 p.m., Pino Toscano wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/110962/
> -----------------------------------------------------------
>
> (Updated June 11, 2013, 5:50 p.m.)
>
>
> Review request for KDE Graphics, Release Team and Gilles Caulier.
>
>
> Description
> -------
>
> Instead of using an embedded copy of LibRaw, look for an external LibRaw as mandatory dependency with a \
> new CMake module and using its variables.
> Considering some LibRaw versions seem to be underlinked and not linking to OpenMP, link it manually in \
> libkdcraw to overcome such lack.
> Switch back to the MAKE_KDCRAW_LIB define (i.e. the default set by KDE4_ADD_LIBRARY) as the one used to \
> check whether it is being built, as otherwise LIBRAW_BUILDLIB would conflict with LibRaw.
> Once this RR is approved, I will remove the libraw code copy and the CMake modules (FindLCMS2.cmake and \
> FindPthreads.cmake) needed for it.
>
> This addresses bug 307146.
> http://bugs.kde.org/show_bug.cgi?id=307146
>
>
> Diffs
> -----
>
> CMakeLists.txt f2f269609feb10947ec3bac10125b379c6c821dd
> cmake/modules/FindLibRaw.cmake PRE-CREATION
> libkdcraw/CMakeLists.txt cce5d6dba690fb5182638ccd1f10488bbd6ec2ce
> libkdcraw/libkdcraw_export.h 1a222a03502a0e068bdba4f03b7ff4961c4a8f2b
>
> Diff: http://git.reviewboard.kde.org/r/110962/diff/
>
>
> Testing
> -------
>
> Compiles fine with both LibRaw 0.14.7 and 0.15.1.
>
>
> Thanks,
>
> Pino Toscano
>
>
[Attachment #5 (text/html)]
<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 solid;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="http://git.reviewboard.kde.org/r/110962/">http://git.reviewboard.kde.org/r/110962/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On June 11th, 2013, 6:38 p.m. UTC, <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;">libraw detection can be not enough.
if you look in libkdcraw, you will see RawSpeed code backported too.
http://rawstudio.org/blog/?p=800
This code is to speed up raw data extraction. It's not to process demosaicing.
Here on my linux Mageia, librawspeed is not available. I don't know if other distro support this \
library.
LibRaw < 0.15 will not support RawSpeed. You must take a care about. In this case RawSpeed must be \
disabled.
Also, settings from raw decoding container and widget have been validated with libraw 0.15.x. So do not \
accept an older version of libraw, else there will be serious problems...
Gilles Caulier</pre>
</blockquote>
<p>On June 11th, 2013, 7:26 p.m. UTC, <b>Pino Toscano</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;">> if you look in libkdcraw, you will see RawSpeed code backported \
too.
Yet another copy of some 3rd party source?! Sigh.... It would be really helpful if your development way \
would not start by embedding some 3rd party library in the sources of some KDE application/library...
For the rest: OK, it is fine to accept only LibRaw >= 0.15. (I suspect they would work the same, but \
oh well...)</pre> </blockquote>
<p>On June 11th, 2013, 8:08 p.m. UTC, <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;">The reason to include libraw is simple : libraw API change agian and \
again, and i don't won't to puzzle libkdcraw source code with an amount of conditional rules. As \
i'm in contact with libraw team, i'm aware of plan and this is why i take a care to not have a \
shared component as libkdcraw which provide a broken binary compatibility with a lots of crash... Just my \
experience.
Typicially, libraw as experiental code which are tested and removed if to much problem are reported...
There is also another problem : how to be sure that 3rd party codec from libraw are compiled in shared \
version of system : There are 2 packs : one GPL2, one GPL3. These packages add new demosaicing methods \
and camera support. If libraw is not compiled with these components some parts of libkdcraw will not work \
as well. This will be very problematic for end users...
This is another reason about libraw inclusion in libkdcraw. Like this i'm sure that all features are \
included...
Libraw is a big puzzle. Take a care...
Gilles Caulier</pre>
</blockquote>
<p>On June 11th, 2013, 8:23 p.m. UTC, <b>Pino Toscano</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;">If the API changes, then the version macros can be used to keep \
compatibility with multiple versions; it seems the libraw people are not breaking the API much lately, so \
hopefully the libkdcraw code won't be filled with a lot of these checks.
Regarding the binary compatibility: if the libraw people bump SONAME/SOVERSION whenever the ABI changes, \
that's perfectly fine and will not cause issues (maybe just to self-compiling users which have no \
clue on what they are doing). If they break ABI without handling SONAME/SOVERSION bumps, that's their \
fault which they ought to solve.
Regarding the extra packs: are those compiled/provided by default to libraw users? If they could cause \
licensing issues/incompatibilities, then compiling them as part of libkdcraw will *not* change their \
situation.</pre> </blockquote>
<p>On June 11th, 2013, 8:35 p.m. UTC, <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;">Extra GPL2/GPL3 pack are not included in libraw in standard due to \
mixing licensing problem. It's separated packages. So distro packagers must take a care about and \
compile all as well.
It's the same problem with RawSpeed which use another licensing than libraw core...
Gilles Caulier</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;">So the inclusion of these packs in libkdcraw *is* creating a \
licensing issue to distro packagers already? Great...
Gilles: putting everything in one source (libkdcraw) does not make
a) licensing issues
b) incompatibilities
c) shipping issue
go away at the same time, as you're creating new issues instead.
Now, if you could help testing this in the way you like, we could cleanup all this embedding mess in \
libkdcraw. Thank you.</pre> <br />
<p>- Pino</p>
<br />
<p>On June 11th, 2013, 5:50 p.m. UTC, Pino Toscano wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: \
url('http://git.reviewboard.kde.org/static/rb/images/review_request_box_top_bg.ab6f3b1072c9.png'); \
background-position: left top; background-repeat: repeat-x; border: 1px black solid;"> <tr>
<td>
<div>Review request for KDE Graphics, Release Team and Gilles Caulier.</div>
<div>By Pino Toscano.</div>
<p style="color: grey;"><i>Updated June 11, 2013, 5:50 p.m.</i></p>
<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;">Instead of using an embedded copy of LibRaw, \
look for an external LibRaw as mandatory dependency with a new CMake module and using its variables.
Considering some LibRaw versions seem to be underlinked and not linking to OpenMP, link it manually in \
libkdcraw to overcome such lack.
Switch back to the MAKE_KDCRAW_LIB define (i.e. the default set by KDE4_ADD_LIBRARY) as the one used to \
check whether it is being built, as otherwise LIBRAW_BUILDLIB would conflict with LibRaw.
Once this RR is approved, I will remove the libraw code copy and the CMake modules (FindLCMS2.cmake and \
FindPthreads.cmake) needed for it.</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;">Compiles fine with both LibRaw 0.14.7 and \
0.15.1.</pre> </td>
</tr>
</table>
<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=307146">307146</a>
</div>
<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">(f2f269609feb10947ec3bac10125b379c6c821dd)</span></li>
<li>cmake/modules/FindLibRaw.cmake <span style="color: grey">(PRE-CREATION)</span></li>
<li>libkdcraw/CMakeLists.txt <span style="color: \
grey">(cce5d6dba690fb5182638ccd1f10488bbd6ec2ce)</span></li>
<li>libkdcraw/libkdcraw_export.h <span style="color: \
grey">(1a222a03502a0e068bdba4f03b7ff4961c4a8f2b)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/110962/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>
_______________________________________________
release-team mailing list
release-team@kde.org
https://mail.kde.org/mailman/listinfo/release-team
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic