[PATCH 1/2] gnu: Add libjpeg-turbo.

[PATCH 1/2] gnu: Add libjpeg-turbo.

[Tobias Geerinckx-Rice]

* gnu/packages/image.scm (libjpeg-turbo): New variable.
---
 gnu/packages/image.scm | 31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/gnu/packages/image.scm b/gnu/packages/image.scm
index 526c87c..d513684 100644
--- a/gnu/packages/image.scm
+++ b/gnu/packages/image.scm
@@ -31,6 +31,7 @@
 (define-module (gnu packages image)
   #:use-module (gnu packages)
   #:use-module (gnu packages algebra)
+  #:use-module (gnu packages assembly)
   #:use-module (gnu packages autotools)
   #:use-module (gnu packages boost)
   #:use-module (gnu packages compression)
@@ -1016,3 +1017,33 @@ also converts external formats (BMP, GIF, PNM and TIFF) to optimized
 PNG, and performs PNG integrity checks and corrections.")
     (home-page "http://optipng.sourceforge.net/")
     (license license:zlib)))
+
+(define-public libjpeg-turbo
+  (package
+    (name "libjpeg-turbo")
+    (version "1.5.1")
+    (source (origin
+              (method url-fetch)
+              (uri (string-append "mirror://sourceforge/" name "/" version "/"
+                                  name "-" version ".tar.gz"))
+              (sha256
+               (base32
+                "0v365hm6z6lddcqagjj15wflk66rqyw75m73cqzl65rh4lyrshj1"))))
+    (build-system gnu-build-system)
+    (native-inputs
+     `(("nasm" ,nasm)))
+    (arguments
+     `(#:test-target "test"))
+    (home-page "http://www.libjpeg-turbo.org/")
+    (synopsis "SIMD-accelerated JPEG image handling library")
+    (description "libjpeg-turbo is a JPEG image codec that accelerates baseline
+JPEG compression and decompression using SIMD instructions: MMX on x86, SSE2 on
+x86-64, NEON on ARM, and AltiVec on PowerPC processors.  Even on other systems,
+its highly-optimized Huffman coding routines allow it to outperform libjpeg by
+a significant amount.
+libjpeg-turbo implements both the traditional libjpeg API and the less powerful
+but more straightforward TurboJPEG API, and provides a full-featured Java
+interface.  It supports color space extensions that allow it to compress from
+and decompress to 32-bit and big-endian pixel buffers (RGBX, XBGR, etc.).")
+    (license (list license:bsd-3        ; jsimd*.[ch] and most of simd/
+                   license:ijg))))      ; the rest
-- 
2.9.3

[PATCH 2/2] gnu: icecat: Use libjpeg-turbo instead of bundled libjpeg.

[Tobias Geerinckx-Rice]

* gnu/packages/gnuzilla.scm (icecat)[source]: Remove bundled libjpeg.
[inputs]: Add libjpeg-turbo.
[arguments]: Use it.
---
 gnu/packages/gnuzilla.scm | 13 +++----------
 1 file changed, 3 insertions(+), 10 deletions(-)

diff --git a/gnu/packages/gnuzilla.scm b/gnu/packages/gnuzilla.scm
index 3b00d3c..f7a8c7d 100644
--- a/gnu/packages/gnuzilla.scm
+++ b/gnu/packages/gnuzilla.scm
@@ -403,6 +403,7 @@ standards.")
                       "modules/zlib"
                       "modules/libbz2"
                       "ipc/chromium/src/third_party/libevent"
+                      "media/libjpeg"
                       "media/libvpx"
                       "security/nss"
                       "gfx/cairo"
@@ -432,6 +433,7 @@ standards.")
        ("hunspell" ,hunspell)
        ("libcanberra" ,libcanberra)
        ("libgnome" ,libgnome)
+       ("libjpeg-turbo" ,libjpeg-turbo)
        ("libxft" ,libxft)
        ("libevent" ,libevent)
        ("libxinerama" ,libxinerama)
@@ -497,6 +499,7 @@ standards.")
                            ;; Avoid bundled libraries.
                            "--with-system-zlib"
                            "--with-system-bz2"
+                           "--with-system-jpeg"        ; must be libjpeg-turbo
                            "--with-system-libevent"
                            "--with-system-libvpx"
                            "--with-system-icu"
@@ -517,16 +520,6 @@ standards.")
                            ;; Network Graphics (PNG) format";
                            ;; we probably do not wish to support it.
                            ;; "--with-system-png"
-
-                           ;; Fails with "libjpeg-turbo JCS_EXTENSIONS
-                           ;; required".
-                           ;; According to
-                           ;; http://sourceforge.net/projects/libjpeg-turbo/ ,
-                           ;; "libjpeg-turbo is a derivative of libjpeg that
-                           ;; uses MMX, SSE, SSE2, and NEON SIMD instructions
-                           ;; to accelerate baseline JPEG compression/
-                           ;; decompression", so we had better not use it
-                           ;; "--with-system-jpeg"
                            )
 
        #:modules ((ice-9 ftw)
-- 
2.9.3

Re: [PATCH 1/2] gnu: Add libjpeg-turbo.

[Leo Famulari]

On Sun, Nov 27, 2016 at 09:17:06PM +0100, Tobias Geerinckx-Rice wrote:
> * gnu/packages/image.scm (libjpeg-turbo): New variable.

Can you double-check that there are no unpatched named bugs in this
version?

https://cve.mitre.org/cgi-bin/cvekey.cgi?keyword=turbo

Otherwise LGTM

Re: [PATCH 2/2] gnu: icecat: Use libjpeg-turbo instead of bundled libjpeg.

[Leo Famulari]

On Sun, Nov 27, 2016 at 09:17:07PM +0100, Tobias Geerinckx-Rice wrote:
> * gnu/packages/gnuzilla.scm (icecat)[source]: Remove bundled libjpeg.
> [inputs]: Add libjpeg-turbo.
> [arguments]: Use it.

> -                           ;; Fails with "libjpeg-turbo JCS_EXTENSIONS
> -                           ;; required".
> -                           ;; According to
> -                           ;; http://sourceforge.net/projects/libjpeg-turbo/ ,
> -                           ;; "libjpeg-turbo is a derivative of libjpeg that
> -                           ;; uses MMX, SSE, SSE2, and NEON SIMD instructions
> -                           ;; to accelerate baseline JPEG compression/
> -                           ;; decompression", so we had better not use it
> -                           ;; "--with-system-jpeg"

Will this work on nachines that don't support SSE? My understanding is
that we don't wish to require it.

Re: [PATCH 2/2] gnu: icecat: Use libjpeg-turbo instead of bundled libjpeg.

[Tobias Geerinckx-Rice]

[-- Attachment #1.1: Type: text/plain, Size: 381 bytes --]

Leo,

On 27/11/16 21:56, Leo Famulari wrote:
> Will this work on nachines that don't support SSE? My understanding is
> that we don't wish to require it.

Why wouldn't it?

I don't often use Icecat, but when I do... I don't see why it would
require SSE by default. ;-) And it's not like we ever patched it to use
IJG's libjpeg to begin with.

Kind regards,

T G-R


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 476 bytes --]

Re: [PATCH 1/2] gnu: Add libjpeg-turbo.

[Tobias Geerinckx-Rice]

[-- Attachment #1.1: Type: text/plain, Size: 438 bytes --]

Leo,

On 27/11/16 21:55, Leo Famulari wrote:
> Can you double-check that there are no unpatched named bugs in this
> version?

Already did ;-)

According to [1], there are three known CVEs, all fixed by 1.4.2.

I didn't find any third party security patches being applied anywhere,
either.

Do you have suspicions otherwise?

Kind regards,

T G-R

[1]: https://cve.mitre.org/cgi-bin/cvekey.cgi?keyword=libjpeg-turbo


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 476 bytes --]

Re: [PATCH 1/2] gnu: Add libjpeg-turbo.

[Leo Famulari]

[-- Attachment #1: Type: text/plain, Size: 462 bytes --]

On Sun, Nov 27, 2016 at 10:12:13PM +0100, Tobias Geerinckx-Rice wrote:
> On 27/11/16 21:55, Leo Famulari wrote:
> > Can you double-check that there are no unpatched named bugs in this
> > version?
> 
> Already did ;-)
> 
> According to [1], there are three known CVEs, all fixed by 1.4.2.
> 
> I didn't find any third party security patches being applied anywhere,
> either.
> 
> Do you have suspicions otherwise?

Nope! Just wanted to be sure :)

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 2/2] gnu: icecat: Use libjpeg-turbo instead of bundled libjpeg.

[Leo Famulari]

[-- Attachment #1: Type: text/plain, Size: 901 bytes --]

On Sun, Nov 27, 2016 at 10:02:35PM +0100, Tobias Geerinckx-Rice wrote:
> Leo,
> 
> On 27/11/16 21:56, Leo Famulari wrote:
> > Will this work on nachines that don't support SSE? My understanding is
> > that we don't wish to require it.
> 
> Why wouldn't it?
> 
> I don't often use Icecat, but when I do... I don't see why it would
> require SSE by default. ;-) And it's not like we ever patched it to use
> IJG's libjpeg to begin with.

I'm not sure, but the comment seems to indicate that we don't want to
use it:

;; According to
;; http://sourceforge.net/projects/libjpeg-turbo/ ,
;; "libjpeg-turbo is a derivative of libjpeg that
;; uses MMX, SSE, SSE2, and NEON SIMD instructions
;; to accelerate baseline JPEG compression/
;; decompression", so we had better not use it

But I'm not sure how to interpret that, so I've copied Andreas on my
reply (he wrote that comment).

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 2/2] gnu: icecat: Use libjpeg-turbo instead of bundled libjpeg.

[Marius Bakke]

[-- Attachment #1: Type: text/plain, Size: 1136 bytes --]

Leo Famulari <leo@famulari.name> writes:

> On Sun, Nov 27, 2016 at 10:02:35PM +0100, Tobias Geerinckx-Rice wrote:
>> Leo,
>> 
>> On 27/11/16 21:56, Leo Famulari wrote:
>> > Will this work on nachines that don't support SSE? My understanding is
>> > that we don't wish to require it.
>> 
>> Why wouldn't it?
>> 
>> I don't often use Icecat, but when I do... I don't see why it would
>> require SSE by default. ;-) And it's not like we ever patched it to use
>> IJG's libjpeg to begin with.
>
> I'm not sure, but the comment seems to indicate that we don't want to
> use it:
>
> ;; According to
> ;; http://sourceforge.net/projects/libjpeg-turbo/ ,
> ;; "libjpeg-turbo is a derivative of libjpeg that
> ;; uses MMX, SSE, SSE2, and NEON SIMD instructions
> ;; to accelerate baseline JPEG compression/
> ;; decompression", so we had better not use it

SSE is pentium III, and SSE2 was introduced in Pentium IV. MMX is even
older. Are we really committed to supporting 15+ year old hardware?

The NEON SIMD instruction seems to be ARM-specific and is available at
least on Cortex A8, which is an ARMv7 design.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]

Re: [PATCH 2/2] gnu: icecat: Use libjpeg-turbo instead of bundled libjpeg.

[Marius Bakke]

[-- Attachment #1: Type: text/plain, Size: 1278 bytes --]

Marius Bakke <mbakke@fastmail.com> writes:

> Leo Famulari <leo@famulari.name> writes:
>
>> On Sun, Nov 27, 2016 at 10:02:35PM +0100, Tobias Geerinckx-Rice wrote:
>>> Leo,
>>> 
>>> On 27/11/16 21:56, Leo Famulari wrote:
>>> > Will this work on nachines that don't support SSE? My understanding is
>>> > that we don't wish to require it.
>>> 
>>> Why wouldn't it?
>>> 
>>> I don't often use Icecat, but when I do... I don't see why it would
>>> require SSE by default. ;-) And it's not like we ever patched it to use
>>> IJG's libjpeg to begin with.
>>
>> I'm not sure, but the comment seems to indicate that we don't want to
>> use it:
>>
>> ;; According to
>> ;; http://sourceforge.net/projects/libjpeg-turbo/ ,
>> ;; "libjpeg-turbo is a derivative of libjpeg that
>> ;; uses MMX, SSE, SSE2, and NEON SIMD instructions
>> ;; to accelerate baseline JPEG compression/
>> ;; decompression", so we had better not use it
>
> SSE is pentium III, and SSE2 was introduced in Pentium IV. MMX is even
> older. Are we really committed to supporting 15+ year old hardware?

Digging a bit further, SSE2 is actually part of the AMD64
specification[0]. Perhaps we could make it conditional on architecture?

0: https://en.wikipedia.org/wiki/SSE2#CPU_support

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]

Re: [PATCH 2/2] gnu: icecat: Use libjpeg-turbo instead of bundled libjpeg.

[Leo Famulari]

[-- Attachment #1: Type: text/plain, Size: 1068 bytes --]

On Sun, Nov 27, 2016 at 10:50:35PM +0100, Marius Bakke wrote:
> Leo Famulari <leo@famulari.name> writes:
> > I'm not sure, but the comment seems to indicate that we don't want to
> > use it:
> >
> > ;; According to
> > ;; http://sourceforge.net/projects/libjpeg-turbo/ ,
> > ;; "libjpeg-turbo is a derivative of libjpeg that
> > ;; uses MMX, SSE, SSE2, and NEON SIMD instructions
> > ;; to accelerate baseline JPEG compression/
> > ;; decompression", so we had better not use it
> 
> SSE is pentium III, and SSE2 was introduced in Pentium IV. MMX is even
> older. Are we really committed to supporting 15+ year old hardware?

See this earlier thread:

http://lists.gnu.org/archive/html/guix-devel/2016-07/msg01534.html

I think it's fine to use libjpeg-turbo if it can run on those older
processors, or if we decide to drop support for them.

> The NEON SIMD instruction seems to be ARM-specific and is available at
> least on Cortex A8, which is an ARMv7 design.

The extremely popular Cortex A7 [0] also has NEON SIMD.

[0] Allwinner A20

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 2/2] gnu: icecat: Use libjpeg-turbo instead of bundled libjpeg.

[Leo Famulari]

[-- Attachment #1: Type: text/plain, Size: 1511 bytes --]

On Sun, Nov 27, 2016 at 11:28:45PM +0100, Marius Bakke wrote:
> Marius Bakke <mbakke@fastmail.com> writes:
> 
> > Leo Famulari <leo@famulari.name> writes:
> >
> >> On Sun, Nov 27, 2016 at 10:02:35PM +0100, Tobias Geerinckx-Rice wrote:
> >>> Leo,
> >>> 
> >>> On 27/11/16 21:56, Leo Famulari wrote:
> >>> > Will this work on nachines that don't support SSE? My understanding is
> >>> > that we don't wish to require it.
> >>> 
> >>> Why wouldn't it?
> >>> 
> >>> I don't often use Icecat, but when I do... I don't see why it would
> >>> require SSE by default. ;-) And it's not like we ever patched it to use
> >>> IJG's libjpeg to begin with.
> >>
> >> I'm not sure, but the comment seems to indicate that we don't want to
> >> use it:
> >>
> >> ;; According to
> >> ;; http://sourceforge.net/projects/libjpeg-turbo/ ,
> >> ;; "libjpeg-turbo is a derivative of libjpeg that
> >> ;; uses MMX, SSE, SSE2, and NEON SIMD instructions
> >> ;; to accelerate baseline JPEG compression/
> >> ;; decompression", so we had better not use it
> >
> > SSE is pentium III, and SSE2 was introduced in Pentium IV. MMX is even
> > older. Are we really committed to supporting 15+ year old hardware?
> 
> Digging a bit further, SSE2 is actually part of the AMD64
> specification[0]. Perhaps we could make it conditional on architecture?
> 
> 0: https://en.wikipedia.org/wiki/SSE2#CPU_support

Sure, if it requires SSE2 to run, and we keep the status quo regarding
i686, I don't see why not.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 2/2] gnu: icecat: Use libjpeg-turbo instead of bundled libjpeg.

[Tobias Geerinckx-Rice]

[-- Attachment #1.1: Type: text/plain, Size: 1319 bytes --]

Andreas, Leo,

Thanks for the feedback!

On 27/11/16 22:41, Leo Famulari wrote:
> I'm not sure, but the comment seems to indicate that we don't want to
> use it:
> 
> ;; According to
> ;; http://sourceforge.net/projects/libjpeg-turbo/ ,
> ;; "libjpeg-turbo is a derivative of libjpeg that
> ;; uses MMX, SSE, SSE2, and NEON SIMD instructions
> ;; to accelerate baseline JPEG compression/
> ;; decompression", so we had better not use it

There seems to be some misunderstanding here. Sorry if it wasn't clear:
this (libjpeg-turbo) was *already* bundled and used by icecat, as the
‘media/libjpeg’ (sic) that this patch removes. You'll find the same SIMD
code there. For the quote above is missing:

> ;; Fails with "libjpeg-turbo JCS_EXTENSIONS required".

Translation: icecat wouldn't build with Guix's ‘libjpeg’ even if we'd
want that.

And we don't. This isn't some dodgy go-faster fork of IJG's ‘real’
libjpeg. It's intended as a sane alternative to the IJG library, albeit
one that doesn't implement every recent extension. It detects CPU
features at runtime — although they're the status quo now, as Andreas
notes — and falls back to other algorithms that might still be faster.

Dang it. I thought I wrote good (description)s. :-(

Kind regards,

T G-R-turbo


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 476 bytes --]

Re: [PATCH 2/2] gnu: icecat: Use libjpeg-turbo instead of bundled libjpeg.

[Tobias Geerinckx-Rice]

[-- Attachment #1.1: Type: text/plain, Size: 352 bytes --]

On 28/11/16 00:15, Tobias Geerinckx-Rice wrote:
> this (libjpeg-turbo) was *already* bundled and used by icecat, [...]

Some fortuitous grepping revealed that our spice packages also #warns
about *not* being built with libjpeg-turbo. Absent any known drawbacks,
I'd follow upstreams' recommendations in these matters.

Kind regards,

T G-R


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 476 bytes --]

Re: [PATCH 2/2] gnu: icecat: Use libjpeg-turbo instead of bundled libjpeg.

[Leo Famulari]

[-- Attachment #1: Type: text/plain, Size: 1057 bytes --]

On Mon, Nov 28, 2016 at 12:15:01AM +0100, Tobias Geerinckx-Rice wrote:
> There seems to be some misunderstanding here. Sorry if it wasn't clear:
> this (libjpeg-turbo) was *already* bundled and used by icecat, as the
> ‘media/libjpeg’ (sic) that this patch removes. You'll find the same SIMD
> code there. For the quote above is missing:

Ah, thanks for the clarification.

> > ;; Fails with "libjpeg-turbo JCS_EXTENSIONS required".
> 
> Translation: icecat wouldn't build with Guix's ‘libjpeg’ even if we'd
> want that.
> 
> And we don't. This isn't some dodgy go-faster fork of IJG's ‘real’
> libjpeg. It's intended as a sane alternative to the IJG library, albeit
> one that doesn't implement every recent extension. It detects CPU
> features at runtime — although they're the status quo now, as Andreas
> notes — and falls back to other algorithms that might still be faster.
> 
> Dang it. I thought I wrote good (description)s. :-(

The description is excellent. Thanks for putting up with my
misunderstanding :)

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 2/2] gnu: icecat: Use libjpeg-turbo instead of bundled libjpeg.

[Leo Famulari]

[-- Attachment #1: Type: text/plain, Size: 478 bytes --]

On Mon, Nov 28, 2016 at 12:20:53AM +0100, Tobias Geerinckx-Rice wrote:
> On 28/11/16 00:15, Tobias Geerinckx-Rice wrote:
> > this (libjpeg-turbo) was *already* bundled and used by icecat, [...]
> 
> Some fortuitous grepping revealed that our spice packages also #warns
> about *not* being built with libjpeg-turbo. Absent any known drawbacks,
> I'd follow upstreams' recommendations in these matters.

Now that we have the package, I think we should make spice use it.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]