From 40ecba4172722533916c359fcfe5a43dcd0801ea Mon Sep 17 00:00:00 2001 From: Jay Berkenbilt Date: Thu, 7 Sep 2017 17:46:55 -0400 Subject: 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. --- libqpdf/Pl_DCT.cc | 152 +++++++++++++++++++++++++++++++++++------ libtests/libtests.testcov | 2 + libtests/qtest/dct.test | 51 ++++++++------ libtests/qtest/dct/big-rawdata | Bin 0 -> 1769472 bytes 4 files changed, 161 insertions(+), 44 deletions(-) create mode 100644 libtests/qtest/dct/big-rawdata diff --git a/libqpdf/Pl_DCT.cc b/libqpdf/Pl_DCT.cc index b341939e..0853f01d 100644 --- a/libqpdf/Pl_DCT.cc +++ b/libqpdf/Pl_DCT.cc @@ -1,6 +1,7 @@ #include #include +#include #include #include #include @@ -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(&cinfo_compress), b); + if (this->action == a_compress) + { + compress(reinterpret_cast(&cinfo_compress), b); + } + else + { + decompress(reinterpret_cast(&cinfo_decompress), b); + } } - else + catch (std::exception& e) { - decompress(reinterpret_cast(&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(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(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( + (*cinfo->mem->alloc_small)(reinterpret_cast(cinfo), + JPOOL_PERMANENT, + sizeof(dct_pipeline_dest))); + dct_pipeline_dest* dest = + reinterpret_cast(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(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( + (*cinfo->mem->alloc_small)(reinterpret_cast(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 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); diff --git a/libtests/libtests.testcov b/libtests/libtests.testcov index 01ca9efe..87216b61 100644 --- a/libtests/libtests.testcov +++ b/libtests/libtests.testcov @@ -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 diff --git a/libtests/qtest/dct.test b/libtests/qtest/dct.test index f3b28581..666f6df8 100644 --- a/libtests/qtest/dct.test +++ b/libtests/qtest/dct.test @@ -16,38 +16,45 @@ my $td = new TestDriver('dct'); 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 { diff --git a/libtests/qtest/dct/big-rawdata b/libtests/qtest/dct/big-rawdata new file mode 100644 index 00000000..1f1c9333 Binary files /dev/null and b/libtests/qtest/dct/big-rawdata differ -- cgit v1.2.3-54-g00ecf