Pl_DCT: Use custom source and destination managers (fixes #153)

Avoid calling jpeg_mem_src and jpeg_mem_dest. The custom destination manager writes to the pipeline in smaller chunks to avoid having the whole image in memory at once. The source manager works directly with the Buffer object. Using customer managers avoids use of memory source and destination managers, which are not present in older versions of libjpeg still in use by some Linux distributions.

Pl_DCT: Use custom source and destination managers (fixes #153)
Avoid calling jpeg_mem_src and jpeg_mem_dest. The custom destination manager writes to the pipeline in smaller chunks to avoid having the whole image in memory at once. The source manager works directly with the Buffer object. Using customer managers avoids use of memory source and destination managers, which are not present in older versions of libjpeg still in use by some Linux distributions.
Jay Berkenbilt
1 parent cbb26149
Showing 4 changed files with 161 additions and 44 deletions
libqpdf/Pl_DCT.cc
libtests/libtests.testcov
libtests/qtest/dct.test
libtests/qtest/dct/big-rawdata
 #include <qpdf/Pl_DCT.hh>
 #include <qpdf/QUtil.hh>
+#include <qpdf/QTC.hh>
 #include <setjmp.h>
 #include <string>
 #include <stdexcept>
@@ -80,15 +81,28 @@ Pl_DCT::finish()
     // and decompress causes a memory leak with setjmp/longjmp. Just
     // use a pointer and delete it.
     Buffer* b = this->buf.getBuffer();
+    // The jpeg library is a "C" library, so we use setjmp and longjmp
+    // for exceptoin handling.
     if (setjmp(jerr.jmpbuf) == 0)
     {
-        if (this->action == a_compress)
+        try
         {
-            compress(reinterpret_cast<void*>(&cinfo_compress), b);
+            if (this->action == a_compress)
+            {
+                compress(reinterpret_cast<void*>(&cinfo_compress), b);
+            }
+            else
+            {
+                decompress(reinterpret_cast<void*>(&cinfo_decompress), b);
+            }
         }
-        else
+        catch (std::exception& e)
         {
-            decompress(reinterpret_cast<void*>(&cinfo_decompress), b);
+            // Convert an exception back to a longjmp so we can ensure
+            // that the right cleanup happens. This will get converted
+            // back to an exception.
+            jerr.msg = e.what();
+            longjmp(jerr.jmpbuf, 1);
         }
     }
     else
@@ -111,24 +125,118 @@ Pl_DCT::finish()
     }
 }
-class Freer
+struct dct_pipeline_dest
 {
-  public:
-    Freer(unsigned char** p) :
-        p(p)
+    struct jpeg_destination_mgr pub; /* public fields */
+    unsigned char* buffer;
+    size_t size;
+    Pipeline* next;
+};
+
+static void
+init_pipeline_destination(j_compress_ptr)
+{
+}
+
+static int
+empty_pipeline_output_buffer(j_compress_ptr cinfo)
+{
+    QTC::TC("libtests", "Pl_DCT empty_pipeline_output_buffer");
+    dct_pipeline_dest* dest =
+        reinterpret_cast<dct_pipeline_dest*>(cinfo->dest);
+    dest->next->write(dest->buffer, dest->size);
+    dest->pub.next_output_byte = dest->buffer;
+    dest->pub.free_in_buffer = dest->size;
+    return TRUE;
+}
+
+static void
+term_pipeline_destination(j_compress_ptr cinfo)
+{
+    QTC::TC("libtests", "Pl_DCT term_pipeline_destination");
+    dct_pipeline_dest* dest =
+        reinterpret_cast<dct_pipeline_dest*>(cinfo->dest);
+    dest->next->write(dest->buffer, dest->size - dest->pub.free_in_buffer);
+}
+
+static void
+jpeg_pipeline_dest(j_compress_ptr cinfo,
+                   unsigned char* outbuffer, size_t size,
+                   Pipeline* next)
+{
+    cinfo->dest = static_cast<struct jpeg_destination_mgr *>(
+        (*cinfo->mem->alloc_small)(reinterpret_cast<j_common_ptr>(cinfo),
+                                   JPOOL_PERMANENT,
+                                   sizeof(dct_pipeline_dest)));
+    dct_pipeline_dest* dest =
+        reinterpret_cast<dct_pipeline_dest*>(cinfo->dest);
+    dest->pub.init_destination = init_pipeline_destination;
+    dest->pub.empty_output_buffer = empty_pipeline_output_buffer;
+    dest->pub.term_destination = term_pipeline_destination;
+    dest->pub.next_output_byte = dest->buffer = outbuffer;
+    dest->pub.free_in_buffer = dest->size = size;
+    dest->next = next;
+}
+
+static void
+init_buffer_source(j_decompress_ptr)
+{
+}
+
+static int
+fill_buffer_input_buffer(j_decompress_ptr)
+{
+    // The whole JPEG data is expected to reside in the supplied memory
+    // buffer, so any request for more data beyond the given buffer size
+    // is treated as an error.
+    throw std::runtime_error("invalid jpeg data reading from buffer");
+    return TRUE;
+}
+
+static void
+skip_buffer_input_data(j_decompress_ptr cinfo, long num_bytes)
+{
+    if (num_bytes < 0)
     {
+        throw std::runtime_error(
+            "reading jpeg: jpeg library requested"
+            " skipping a negative number of bytes");
     }
-    ~Freer()
+    size_t to_skip = static_cast<size_t>(num_bytes);
+    if ((to_skip > 0) && (to_skip <= cinfo->src->bytes_in_buffer))
     {
-        if (*p)
-        {
-            free(*p);
-        }
+        cinfo->src->next_input_byte += to_skip;
+        cinfo->src->bytes_in_buffer -= to_skip;
+    }
+    else if (to_skip != 0)
+    {
+        cinfo->src->next_input_byte += cinfo->src->bytes_in_buffer;
+        cinfo->src->bytes_in_buffer = 0;
     }
+}
-  private:
-    unsigned char** p;
-};
+static void
+term_buffer_source(j_decompress_ptr)
+{
+}
+
+static void
+jpeg_buffer_src(j_decompress_ptr cinfo, Buffer* buffer)
+{
+    cinfo->src = reinterpret_cast<jpeg_source_mgr *>(
+        (*cinfo->mem->alloc_small)(reinterpret_cast<j_common_ptr>(cinfo),
+                                   JPOOL_PERMANENT,
+                                   sizeof(jpeg_source_mgr)));
+
+    jpeg_source_mgr* src = cinfo->src;
+    src->init_source = init_buffer_source;
+    src->fill_input_buffer = fill_buffer_input_buffer;
+    src->skip_input_data = skip_buffer_input_data;
+    src->resync_to_restart = jpeg_resync_to_restart; /* use default method */
+    src->term_source = term_buffer_source;
+    src->bytes_in_buffer = buffer->getSize();
+    src->next_input_byte = buffer->getBuffer();
+}
 void
 Pl_DCT::compress(void* cinfo_p, Buffer* b)
@@ -146,10 +254,11 @@ Pl_DCT::compress(void* cinfo_p, Buffer* b)
      defined(__clang__))
 #       pragma GCC diagnostic pop
 #endif
-    unsigned char* outbuffer = 0;
-    Freer freer(&outbuffer);
-    unsigned long outsize = 0;
-    jpeg_mem_dest(cinfo, &outbuffer, &outsize);
+    static int const BUF_SIZE = 65536;
+    PointerHolder<unsigned char> outbuffer_ph(
+        true, new unsigned char[BUF_SIZE]);
+    unsigned char* outbuffer = outbuffer_ph.getPointer();
+    jpeg_pipeline_dest(cinfo, outbuffer, BUF_SIZE, this->getNext());
     cinfo->image_width = this->image_width;
     cinfo->image_height = this->image_height;
@@ -182,7 +291,6 @@ Pl_DCT::compress(void* cinfo_p, Buffer* b)
         (void) jpeg_write_scanlines(cinfo, row_pointer, 1);
     }
     jpeg_finish_compress(cinfo);
-    this->getNext()->write(outbuffer, outsize);
     this->getNext()->finish();
 }
@@ -202,7 +310,7 @@ Pl_DCT::decompress(void* cinfo_p, Buffer* b)
      defined(__clang__))
 #       pragma GCC diagnostic pop
 #endif
-    jpeg_mem_src(cinfo, b->getBuffer(), b->getSize());
+    jpeg_buffer_src(cinfo, b);
     (void) jpeg_read_header(cinfo, TRUE);
     (void) jpeg_calc_output_dimensions(cinfo);
@@ -27,3 +27,5 @@ InputSource found match at buf[0] 0
 Pl_RunLength: switch to run 1
 Pl_RunLength flush full buffer 1
 Pl_RunLength flush empty buffer 0
+Pl_DCT empty_pipeline_output_buffer 0
+Pl_DCT term_pipeline_destination 0
@@ -16,38 +16,45 @@ my $td = new TestDriver(&#39;dct&#39;);
 cleanup();
-$td->runtest("compress",
-             {$td->COMMAND => "dct_compress rawdata a.jpg 400 256 gray"},
-             {$td->STRING => "", $td->EXIT_STATUS => 0});
-$td->runtest("decompress",
-             {$td->COMMAND => "dct_uncompress a.jpg out"},
-             {$td->STRING => "", $td->EXIT_STATUS => 0});
-# Compare
-my @raw = get_data('rawdata');
-my @processed = get_data('out');
 my $checked_data = 0;
-if ($td->runtest("bytes in data",
-                 {$td->STRING => scalar(@processed)},
-                 {$td->STRING => scalar(@raw)}))
+foreach my $d (['rawdata', '400 256 gray', 0],
+               ['big-rawdata', '1024 576 rgb', 0.2])
 {
-    my $mismatch = 0;
-    for (my $i = 0; $i < scalar(@raw); ++$i)
+    my ($in, $args, $mismatch_fraction) = @$d;
+    $td->runtest("compress",
+                 {$td->COMMAND => "dct_compress $in a.jpg $args"},
+                 {$td->STRING => "", $td->EXIT_STATUS => 0});
+    $td->runtest("decompress",
+                 {$td->COMMAND => "dct_uncompress a.jpg out"},
+                 {$td->STRING => "", $td->EXIT_STATUS => 0});
+    # Compare
+    my @raw = get_data($in);
+    my @processed = get_data('out');
+    my $bytes = scalar(@raw);
+    if ($td->runtest("bytes in data",
+                     {$td->STRING => scalar(@processed)},
+                     {$td->STRING => $bytes}))
     {
-        $checked_data = 1;
-        my $delta = abs(ord($raw[$i]) - ord($processed[$i]));
-        if ($delta > 10)
+        ++$checked_data;
+        my $mismatch = 0;
+        for (my $i = 0; $i < scalar(@raw); ++$i)
         {
-            ++$mismatch;
+            my $delta = abs(ord($raw[$i]) - ord($processed[$i]));
+            if ($delta > 10)
+            {
+                ++$mismatch;
+            }
         }
+        my $threshold = int($mismatch_fraction * $bytes);
+        $td->runtest("data is close enough",
+                     {$td->STRING => $mismatch <= $threshold ? 'pass' : 'fail'},
+                     {$td->STRING => 'pass'});
     }
-    $td->runtest("data is close enough",
-                 {$td->STRING => $mismatch},
-                 {$td->STRING => '0'});
 }
 cleanup();
-$td->report(3 + $checked_data);
+$td->report(6 + $checked_data);
 sub cleanup
 {