Subject: bug#74476: closed (Re: bug#74476: [PATCH] Explore JPEG loading without quantization)

From: [email-masked] (GNU bug Tracking System)
Date: Mon, 02 Dec 2024 10:49:02 +0000

Your bug report

#74476: [PATCH] Explore JPEG loading without quantization

which was filed against the emacs package, has been closed.

The explanation is attached below, along with your original report.
If you require more details, please reply to [email-masked].

-- 
74476: https://debbugs.gnu.org/cgi/bugreport.cgi?bug=74476
GNU Bug Tracking System
Contact [email-masked] with problems

Subject: Re: bug#74476: [PATCH] Explore JPEG loading without quantization
From: Alan Third <[email-masked]>
To: Manuel Giraud <[email-masked]>
Cc: [email-masked], Eli Zaretskii <[email-masked]>
Date: Mon, 2 Dec 2024 10:47:42 +0000
List-Id: <debbugs-submit.debbugs.gnu.org>

On Sun, Dec 01, 2024 at 12:13:53PM +0100, Manuel Giraud wrote:
> Alan Third <[email-masked]> writes:
> 
> > I think it should be done for each load. And I just noticed you need
> > to add a free_color_table call after you're done with it.
> 
> Yes, I have re-add it in the attached new version of the patch.  I also
> re-add the ir, ig and ib indices computation.  WDYT?

Looks good to me.

I've pushed this to master. Thanks!
-- 
Alan Third

Subject: [PATCH] Explore JPEG loading without quantization
From: Manuel Giraud <[email-masked]>
To: [email-masked]
Date: Fri, 22 Nov 2024 15:53:18 +0100
List-Id: <debbugs-submit.debbugs.gnu.org>

Tags: patch

Hi,

While trying to speed up "emacs as an image viewer", I found that emacs
is using libjpeg with color quantization and it seems that removing this
quantization could speed up JPEG loading a bit.

My simple limited benchmark:

          - M-: (clear-image-cache)
          - Open an image in folder with some large enough pictures in
            it (4000x3000 here)
          - M-: (benchmark-run 10 (image-next-file 1))

Here are the results I get:

without this path: (5.415405491 1 0.09232176400000025)
with: (3.079911418 1 0.0751190459999993)

I don't think that this patch could be applied as is (it is rather
ugly).  And I also think that I probably have missed some (many?) use
case (where color quantization is mandatory).  But I'm submitting this
patch anyway as a conversation starter on the subject.

Thanks,

In GNU Emacs 31.0.50 (build 9, x86_64-unknown-openbsd7.6, X toolkit) of
 2024-11-22 built on computer
Repository revision: c66c0942ea9ac10e6d6324e472150de403a03b69
Repository branch: master
Windowing system distributor 'The X.Org Foundation', version 11.0.12101014
System Description: OpenBSD computer 7.6 GENERIC.MP#437 amd64

Configured using:
 'configure CC=egcc CPPFLAGS=-I/usr/local/include
 LDFLAGS=-L/usr/local/lib MAKEINFO=gmakeinfo --prefix=/home/manuel/emacs
 --bindir=/home/manuel/bin --with-x-toolkit=lucid
 --with-toolkit-scroll-bars=no --without-cairo
 --without-compress-install'

>From e360545b65427d6f0eec05cdeb26b8a05d8a0d61 Mon Sep 17 00:00:00 2001
From: Manuel Giraud <[email-masked]>
Date: Thu, 21 Nov 2024 17:19:59 +0100
Subject: [PATCH] Explore JPEG loading without quantization

---
 src/image.c | 63 +++++++++++++----------------------------------------
 1 file changed, 15 insertions(+), 48 deletions(-)

diff --git a/src/image.c b/src/image.c
index db7f6acd171..374d2f9c454 100644
--- a/src/image.c
+++ b/src/image.c
@@ -8949,9 +8949,9 @@ jpeg_load_body (struct frame *f, struct image *img,
   FILE *fp = NULL;
   JSAMPARRAY buffer;
   int row_stride, x, y;
-  int width, height;
-  int i, ir, ig, ib;
-  unsigned long *colors;
+  int width, height, ncomp;
+  int i, off;
+  unsigned long color;
   Emacs_Pix_Container volatile ximg_volatile = NULL;
 
   /* Open the JPEG file.  */
@@ -9049,12 +9049,11 @@ jpeg_load_body (struct frame *f, struct image *img,
 
   jpeg_read_header (&mgr->cinfo, 1);
 
-  /* Customize decompression so that color quantization will be used.
-	 Start decompression.  */
-  mgr->cinfo.quantize_colors = 1;
+  /* Start decompression.  */
   jpeg_start_decompress (&mgr->cinfo);
   width = img->width = mgr->cinfo.output_width;
   height = img->height = mgr->cinfo.output_height;
+  ncomp = mgr->cinfo.output_components;
 
   if (!check_image_size (f, width, height))
     {
@@ -9073,53 +9072,22 @@ jpeg_load_body (struct frame *f, struct image *img,
       sys_longjmp (mgr->setjmp_buffer, 1);
     }
 
-  /* Allocate colors.  When color quantization is used,
-     mgr->cinfo.actual_number_of_colors has been set with the number of
-     colors generated, and mgr->cinfo.colormap is a two-dimensional array
-     of color indices in the range 0..mgr->cinfo.actual_number_of_colors.
-     No more than 255 colors will be generated.  */
-  USE_SAFE_ALLOCA;
-  {
-    if (mgr->cinfo.out_color_components > 2)
-      ir = 0, ig = 1, ib = 2;
-    else if (mgr->cinfo.out_color_components > 1)
-      ir = 0, ig = 1, ib = 0;
-    else
-      ir = 0, ig = 0, ib = 0;
-
-    /* Use the color table mechanism because it handles colors that
-       cannot be allocated nicely.  Such colors will be replaced with
-       a default color, and we don't have to care about which colors
-       can be freed safely, and which can't.  */
-    init_color_table ();
-    SAFE_NALLOCA (colors, 1, mgr->cinfo.actual_number_of_colors);
-
-    for (i = 0; i < mgr->cinfo.actual_number_of_colors; ++i)
-      {
-	/* Multiply RGB values with 255 because X expects RGB values
-	   in the range 0..0xffff.  */
-	int r = mgr->cinfo.colormap[ir][i] << 8;
-	int g = mgr->cinfo.colormap[ig][i] << 8;
-	int b = mgr->cinfo.colormap[ib][i] << 8;
-	colors[i] = lookup_rgb_color (f, r, g, b);
-      }
-
-#ifdef COLOR_TABLE_SUPPORT
-    /* Remember those colors actually allocated.  */
-    img->colors = colors_in_color_table (&img->ncolors);
-    free_color_table ();
-#endif /* COLOR_TABLE_SUPPORT */
-  }
-
   /* Read pixels.  */
-  row_stride = width * mgr->cinfo.output_components;
+  row_stride = width * ncomp;
   buffer = mgr->cinfo.mem->alloc_sarray ((j_common_ptr) &mgr->cinfo,
 					 JPOOL_IMAGE, row_stride, 1);
   for (y = 0; y < height; ++y)
     {
       jpeg_read_scanlines (&mgr->cinfo, buffer, 1);
-      for (x = 0; x < mgr->cinfo.output_width; ++x)
-	PUT_PIXEL (ximg, x, y, colors[buffer[0][x]]);
+      for (x = 0; x < width; ++x)
+	{
+	  color = 0;
+	  off = x * ncomp;
+	  /* XXX I suck at bit twiddling.  */
+	  for (i = 0; i < ncomp; ++i)
+	    color += (buffer[0][off + i] << ((ncomp - 1 - i) * 8));
+	  PUT_PIXEL (ximg, x, y, color);
+	}
     }
 
   /* Clean up.  */
@@ -9135,7 +9103,6 @@ jpeg_load_body (struct frame *f, struct image *img,
 
   /* Put ximg into the image.  */
   image_put_x_image (f, img, ximg, 0);
-  SAFE_FREE ();
   return 1;
 }
 
-- 
2.47.0

-- 
Manuel Giraud