martinbenes1996/jpeglib

Swapped sampling factors

Closed this issue · 2 comments

I am confused about the samp_factor property. im.samp_factor gives a 3x2 array. Intuitively, I would expect that the three rows correspond to the three channels and the two columns represent the horizontal and vertical directions. I wonder whether the horizontal and vertical directions were swapped accidentally.

Example:

  1. Compress an image using the cjpeg command line tool. With -sample 2x1, chroma pixels are 2 pixels wide and 1 pixel tall (horizontal subsampling only, compare this great blog post).
cjpeg -sample 2x1 -quality 75 testimg.ppm > /tmp/testimg.jpg
  1. Use libjpeg to retrieve and print the sampling factors.

The following code is based on libjpeg's example.c.

#include <stdio.h>
#include <stdlib.h>
#include "jpeglib.h"
#include <setjmp.h>


struct my_error_mgr {
  struct jpeg_error_mgr pub;	/* "public" fields */

  jmp_buf setjmp_buffer;	/* for return to caller */
};

typedef struct my_error_mgr * my_error_ptr;


METHODDEF(void)
my_error_exit (j_common_ptr cinfo)
{
  /* cinfo->err really points to a my_error_mgr struct, so coerce pointer */
  my_error_ptr myerr = (my_error_ptr) cinfo->err;

  /* Always display the message. */
  /* We could postpone this until after returning, if we chose. */
  (*cinfo->err->output_message) (cinfo);

  /* Return control to the setjmp point */
  longjmp(myerr->setjmp_buffer, 1);
}


GLOBAL(int) read_JPEG_file (char * filename)
{
  struct jpeg_decompress_struct cinfo;
  struct my_error_mgr jerr;
  FILE * infile;

  if ((infile = fopen(filename, "rb")) == NULL) {
    fprintf(stderr, "can't open %s\n", filename);
    return 0;
  }
  cinfo.err = jpeg_std_error(&jerr.pub);
  jerr.pub.error_exit = my_error_exit;
  if (setjmp(jerr.setjmp_buffer)) {
    jpeg_destroy_decompress(&cinfo);
    fclose(infile);
    return 0;
  }

  jpeg_create_decompress(&cinfo);

  jpeg_stdio_src(&cinfo, infile);

  (void) jpeg_read_header(&cinfo, TRUE);

  for (unsigned int ci=0; ci < 3; ci++) {
    printf("Channel %u: h_samp_factor = %d, v_samp_factor = %d\n",
      ci,
      cinfo.comp_info[ci].h_samp_factor,
      cinfo.comp_info[ci].v_samp_factor);
  }

  fclose(infile);

  return 0;
}


int main(int argc, char* argv[]) {
  read_JPEG_file("/tmp/testimg.jpg");

  return 0;
}

Compile and run

# This assumes you are the libjpeg source directory and compiled libjpeg to ./build
gcc -g -O2 -o example example.c -I`pwd`/build/include -L`pwd`/build/lib -ljpeg
LD_LIBRARY_PATH=`pwd`/build/lib ./example

This gives the expected output:

Channel 0: h_samp_factor = 2, v_samp_factor = 1
Channel 1: h_samp_factor = 1, v_samp_factor = 1
Channel 2: h_samp_factor = 1, v_samp_factor = 1
  1. Conversely, jpeglib reports the values in different order.
import jpeglib
im = jpeglib.read_spatial("/tmp/testimg.jpg")
print(im.samp_factor)

Output:

[[1 2]
 [1 1]
 [1 1]]

Since the default order to specify the sampling factors is h x v, I would have expected the jpeglib output in the same order.

It would be great if you can double-check whether the current behavior is intended. In this case, please consider updating the documentation.

After browsing the code, I found that jpeglib intentionally puts the vertical sampling factors first, presumably to match the common [height, width] order in Python. Nevertheless, I swapped the order so that the horizontal sampling factor comes before the vertical factor, which matches the commonly used notation in other JPEG tools. The changes and updated test cases can be found in the dev branch.

Thanks for pointing out. As all the other objects (spatial, Y, Cb, Cr) are in height-width order, it makes sense to keep it as-is and be consistent with our own interface.

I am extending the documentation to make sure the order in jpeglib is described well.