diff --git a/ChangeLog b/ChangeLog index 5e3092de..6fa4c45d 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,21 @@ +2012-07-07 Jay Berkenbilt + + * NOTE: BREAKING API CHANGE. Remove previously required length + parameter from the version QPDFObjectHandle::replaceStreamData + that uses a stream data provider. Prior to qpdf 3.0.0, you had to + compute the stream length in advance so that qpdf could internally + verify that the stream data had the same length every time the + provider was invoked. Now this requirement is enforced a + different way, and the length parameter is no longer required. + Note that I take API-breaking changes very seriously and only did + it in this case since the lack of need to know length in advance + could significantly simplify people's code. If you were + previously going to a lot of trouble to compute the length of the + new stream data in advance, you now no longer have to do that. + You can just drop the length parameter and remove any code that + was previously computing the length. Thanks to Tobias Hoffmann + for pointing out how annoying the original interface was. + 2012-07-05 Jay Berkenbilt * Add QPDFWriter methods to write to an already open stdio FILE*. diff --git a/TODO b/TODO index 40c113d6..9adbaa73 100644 --- a/TODO +++ b/TODO @@ -17,8 +17,8 @@ Next * Document that your compiler has to support long long. - * Figure out why we have to specify a stream's length in advance when - providing stream data, and remove this restriction if possible. + * Make sure that the release notes call attention to the one API + breaking change: removal of length from replaceStreamData. * Add a way to create new QPDFObjectHandles with a string representation of them, such as diff --git a/examples/pdf-create.cc b/examples/pdf-create.cc index e1d75759..a9ad2389 100644 --- a/examples/pdf-create.cc +++ b/examples/pdf-create.cc @@ -17,7 +17,6 @@ class ImageProvider: public QPDFObjectHandle::StreamDataProvider virtual ~ImageProvider(); virtual void provideStreamData(int objid, int generation, Pipeline* pipeline); - size_t getLength() const; private: int width; @@ -45,12 +44,6 @@ ImageProvider::provideStreamData(int objid, int generation, pipeline->finish(); } -size_t -ImageProvider::getLength() const -{ - return 3 * width * height; -} - void usage() { std::cerr << "Usage: " << whoami << " filename" << std::endl @@ -111,8 +104,7 @@ static void create_pdf(char const* filename) PointerHolder provider(p); image.replaceStreamData(provider, QPDFObjectHandle::newNull(), - QPDFObjectHandle::newNull(), - p->getLength()); + QPDFObjectHandle::newNull()); // Create direct objects as needed by the page dictionary. QPDFObjectHandle procset = QPDFObjectHandle::newArray(); diff --git a/examples/pdf-invert-images.cc b/examples/pdf-invert-images.cc index 2dc73251..997fc37c 100644 --- a/examples/pdf-invert-images.cc +++ b/examples/pdf-invert-images.cc @@ -141,8 +141,7 @@ int main(int argc, char* argv[]) image.replaceStreamData( p, QPDFObjectHandle::newNull(), - QPDFObjectHandle::newNull(), - inv->image_data[objid][gen]->getSize()); + QPDFObjectHandle::newNull()); } } } diff --git a/include/qpdf/QPDFObjectHandle.hh b/include/qpdf/QPDFObjectHandle.hh index a61277a9..421b0144 100644 --- a/include/qpdf/QPDFObjectHandle.hh +++ b/include/qpdf/QPDFObjectHandle.hh @@ -294,18 +294,31 @@ class QPDFObjectHandle // directly providing a buffer with the stream data, call the // given provider's provideStreamData method. See comments on the // StreamDataProvider class (defined above) for details on the - // method. The provider must write the number of bytes as - // indicated by the length parameter, and the data must be - // consistent with filter and decode_parms as provided. Although - // it is more complex to use this form of replaceStreamData than - // the one that takes a buffer, it makes it possible to avoid - // allocating memory for the stream data. Example programs are - // provided that use both forms of replaceStreamData. + // method. The data must be consistent with filter and + // decode_parms as provided. Although it is more complex to use + // this form of replaceStreamData than the one that takes a + // buffer, it makes it possible to avoid allocating memory for the + // stream data. Example programs are provided that use both forms + // of replaceStreamData. + + // Note about stream length: for any given stream, the provider + // must provide the same amount of data each time it is called. + // This is critical for making linearization work properly. + // Versions of qpdf before 3.0.0 required a length to be specified + // here. Starting with version 3.0.0, this is no longer necessary + // (or permitted). The first time the stream data provider is + // invoked for a given stream, the actual length is stored. + // Subsequent times, it is enforced that the length be the same as + // the first time. + + // If you have gotten a compile error here while building code + // that worked with older versions of qpdf, just omit the length + // parameter. You can also simplify your code by not having to + // compute the length in advance. QPDF_DLL void replaceStreamData(PointerHolder provider, QPDFObjectHandle const& filter, - QPDFObjectHandle const& decode_parms, - size_t length); + QPDFObjectHandle const& decode_parms); // return 0 for direct objects QPDF_DLL diff --git a/libqpdf/QPDFObjectHandle.cc b/libqpdf/QPDFObjectHandle.cc index 4d6c5f79..c8b67416 100644 --- a/libqpdf/QPDFObjectHandle.cc +++ b/libqpdf/QPDFObjectHandle.cc @@ -414,12 +414,11 @@ QPDFObjectHandle::replaceStreamData(PointerHolder data, void QPDFObjectHandle::replaceStreamData(PointerHolder provider, QPDFObjectHandle const& filter, - QPDFObjectHandle const& decode_parms, - size_t length) + QPDFObjectHandle const& decode_parms) { assertType("Stream", isStream()); dynamic_cast(obj.getPointer())->replaceStreamData( - provider, filter, decode_parms, length); + provider, filter, decode_parms); } int diff --git a/libqpdf/QPDF_Stream.cc b/libqpdf/QPDF_Stream.cc index ed5fb5be..c089bcc1 100644 --- a/libqpdf/QPDF_Stream.cc +++ b/libqpdf/QPDF_Stream.cc @@ -380,24 +380,33 @@ QPDF_Stream::pipeStreamData(Pipeline* pipeline, bool filter, this->stream_provider->provideStreamData( this->objid, this->generation, &count); qpdf_offset_t actual_length = count.getCount(); - qpdf_offset_t desired_length = - this->stream_dict.getKey("/Length").getIntValue(); - if (actual_length == desired_length) - { - QTC::TC("qpdf", "QPDF_Stream pipe use stream provider"); - } - else - { - QTC::TC("qpdf", "QPDF_Stream provider length mismatch"); - throw std::logic_error( - "stream data provider for " + - QUtil::int_to_string(this->objid) + " " + - QUtil::int_to_string(this->generation) + - " provided " + - QUtil::int_to_string(actual_length) + - " bytes instead of expected " + - QUtil::int_to_string(desired_length) + " bytes"); - } + qpdf_offset_t desired_length = 0; + if (this->stream_dict.hasKey("/Length")) + { + desired_length = this->stream_dict.getKey("/Length").getIntValue(); + if (actual_length == desired_length) + { + QTC::TC("qpdf", "QPDF_Stream pipe use stream provider"); + } + else + { + QTC::TC("qpdf", "QPDF_Stream provider length mismatch"); + throw std::logic_error( + "stream data provider for " + + QUtil::int_to_string(this->objid) + " " + + QUtil::int_to_string(this->generation) + + " provided " + + QUtil::int_to_string(actual_length) + + " bytes instead of expected " + + QUtil::int_to_string(desired_length) + " bytes"); + } + } + else + { + QTC::TC("qpdf", "QPDF_Stream provider length not provided"); + this->stream_dict.replaceKey( + "/Length", QPDFObjectHandle::newInteger(actual_length)); + } } else if (this->offset == 0) { @@ -430,12 +439,11 @@ void QPDF_Stream::replaceStreamData( PointerHolder provider, QPDFObjectHandle const& filter, - QPDFObjectHandle const& decode_parms, - size_t length) + QPDFObjectHandle const& decode_parms) { this->stream_provider = provider; this->stream_data = 0; - replaceFilterData(filter, decode_parms, length); + replaceFilterData(filter, decode_parms, 0); } void @@ -445,6 +453,14 @@ QPDF_Stream::replaceFilterData(QPDFObjectHandle const& filter, { this->stream_dict.replaceOrRemoveKey("/Filter", filter); this->stream_dict.replaceOrRemoveKey("/DecodeParms", decode_parms); - this->stream_dict.replaceKey("/Length", - QPDFObjectHandle::newInteger((int)length)); + if (length == 0) + { + QTC::TC("qpdf", "QPDF_Stream unknown stream length"); + this->stream_dict.removeKey("/Length"); + } + else + { + this->stream_dict.replaceKey( + "/Length", QPDFObjectHandle::newInteger((int)length)); + } } diff --git a/libqpdf/qpdf/QPDF_Stream.hh b/libqpdf/qpdf/QPDF_Stream.hh index e74ae201..ce46d994 100644 --- a/libqpdf/qpdf/QPDF_Stream.hh +++ b/libqpdf/qpdf/QPDF_Stream.hh @@ -30,8 +30,7 @@ class QPDF_Stream: public QPDFObject void replaceStreamData( PointerHolder provider, QPDFObjectHandle const& filter, - QPDFObjectHandle const& decode_parms, - size_t length); + QPDFObjectHandle const& decode_parms); // Replace object ID and generation. This may only be called if // object ID and generation are 0. It is used by QPDFObjectHandle diff --git a/qpdf/qpdf.testcov b/qpdf/qpdf.testcov index 93bbf88d..b2c98496 100644 --- a/qpdf/qpdf.testcov +++ b/qpdf/qpdf.testcov @@ -215,3 +215,5 @@ QPDFObjectHandle shallow copy dictionary 0 QPDFObjectHandle shallow copy scalar 0 QPDFObjectHandle newStream with string 0 QPDF unknown key not inherited 0 +QPDF_Stream provider length not provided 0 +QPDF_Stream unknown stream length 0 diff --git a/qpdf/qtest/qpdf/replaced-stream-data-flate.pdf b/qpdf/qtest/qpdf/replaced-stream-data-flate.pdf index 8da1663f..02931170 100644 Binary files a/qpdf/qtest/qpdf/replaced-stream-data-flate.pdf and b/qpdf/qtest/qpdf/replaced-stream-data-flate.pdf differ diff --git a/qpdf/test_driver.cc b/qpdf/test_driver.cc index 89b422ac..8373bd04 100644 --- a/qpdf/test_driver.cc +++ b/qpdf/test_driver.cc @@ -478,7 +478,17 @@ void runtest(int n, char const* filename) PointerHolder p = provider; qstream.replaceStreamData( p, QPDFObjectHandle::newName("/FlateDecode"), - QPDFObjectHandle::newNull(), b->getSize()); + QPDFObjectHandle::newNull()); + provider->badLength(false); + QPDFWriter w(pdf, "a.pdf"); + w.setStaticID(true); + // Linearize to force the provider to be called multiple times. + w.setLinearization(true); + w.setStreamDataMode(qpdf_s_preserve); + w.write(); + + // Every time a provider pipes stream data, it has to provide + // the same amount of data. provider->badLength(true); try { @@ -489,11 +499,6 @@ void runtest(int n, char const* filename) { std::cout << "exception: " << e.what() << std::endl; } - provider->badLength(false); - QPDFWriter w(pdf, "a.pdf"); - w.setStaticID(true); - w.setStreamDataMode(qpdf_s_preserve); - w.write(); } else if (n == 9) { diff --git a/qpdf/test_large_file.cc b/qpdf/test_large_file.cc index fc2c7bbc..f54adb33 100644 --- a/qpdf/test_large_file.cc +++ b/qpdf/test_large_file.cc @@ -109,7 +109,6 @@ class ImageProvider: public QPDFObjectHandle::StreamDataProvider virtual ~ImageProvider(); virtual void provideStreamData(int objid, int generation, Pipeline* pipeline); - size_t getLength() const; private: int n; @@ -142,12 +141,6 @@ ImageProvider::provideStreamData(int objid, int generation, pipeline->finish(); } -size_t -ImageProvider::getLength() const -{ - return width * height; -} - void usage() { std::cerr << "Usage: " << whoami << " {read|write} {large|small} outfile" @@ -229,8 +222,7 @@ static void create_pdf(char const* filename) PointerHolder provider(p); image.replaceStreamData(provider, QPDFObjectHandle::newNull(), - QPDFObjectHandle::newNull(), - p->getLength()); + QPDFObjectHandle::newNull()); QPDFObjectHandle xobject = QPDFObjectHandle::newDictionary(); xobject.replaceKey("/Im1", image);