Remove for loop in QPDFWriter::writeObjectStream

m-holger
1 parent ae9e65f3
Showing 2 changed files with 68 additions and 67 deletions
libqpdf/QPDFWriter.cc
manual/release-notes.rst
@@ -1646,81 +1646,79 @@ QPDFWriter::writeObjectStream(QPDFObjectHandle object)
     std::shared_ptr<Buffer> stream_buffer;
     int first_obj = -1;
     bool compressed = false;
-    for (int pass: {1, 2}) {
-        PipelinePopper pp_ostream(this, &stream_buffer);
-        if (pass == 1) {
-            pushPipeline(new Pl_Buffer("object stream"));
-            activatePipelineStack(pp_ostream);
-        } else {
-            // Adjust offsets to skip over comment before first object
-            first = offsets.at(0);
-            for (auto& iter: offsets) {
-                iter -= first;
+    {
+        // Pass 1
+        PipelinePopper pp_ostream_pass1(this, &stream_buffer);
+
+        pushPipeline(new Pl_Buffer("object stream"));
+        activatePipelineStack(pp_ostream_pass1);
+
+        int count = -1;
+        for (auto const& obj: m->object_stream_to_objects[old_id]) {
+            ++count;
+            int new_obj = m->obj[obj].renumber;
+            if (first_obj == -1) {
+                first_obj = new_obj;
             }
-
-            // Take one pass at writing pairs of numbers so we can get their size information
-            {
-                PipelinePopper pp_discard(this);
-                pushDiscardFilter(pp_discard);
-                writeObjectStreamOffsets(offsets, first_obj);
-                first += m->pipeline->getCount();
+            if (m->qdf_mode) {
+                writeString(
+                    "%% Object stream: object " + std::to_string(new_obj) + ", index " +
+                    std::to_string(count));
+                if (!m->suppress_original_object_ids) {
+                    writeString("; original object ID: " + std::to_string(obj.getObj()));
+                    // For compatibility, only write the generation if non-zero.  While object
+                    // streams only allow objects with generation 0, if we are generating object
+                    // streams, the old object could have a non-zero generation.
+                    if (obj.getGen() != 0) {
+                        QTC::TC("qpdf", "QPDFWriter original obj non-zero gen");
+                        writeString(" " + std::to_string(obj.getGen()));
+                    }
+                }
+                writeString("\n");
             }
-            // Set up a stream to write the stream data into a buffer.
-            Pipeline* next = pushPipeline(new Pl_Buffer("object stream"));
-            if (m->compress_streams && !m->qdf_mode) {
-                compressed = true;
-                next =
-                    pushPipeline(new Pl_Flate("compress object stream", next, Pl_Flate::a_deflate));
+            offsets.push_back(m->pipeline->getCount());
+            // To avoid double-counting objects being written in object streams for progress
+            // reporting, decrement in pass 1.
+            indicateProgress(true, false);
+
+            QPDFObjectHandle obj_to_write = m->pdf.getObject(obj);
+            if (obj_to_write.isStream()) {
+                // This condition occurred in a fuzz input. Ideally we should block it at parse
+                // time, but it's not clear to me how to construct a case for this.
+                obj_to_write.warnIfPossible("stream found inside object stream; treating as null");
+                obj_to_write = QPDFObjectHandle::newNull();
             }
-            activatePipelineStack(pp_ostream);
-            writeObjectStreamOffsets(offsets, first_obj);
-            writeBuffer(stream_buffer);
+            writeObject(obj_to_write, count);
+
+            m->new_obj[new_obj].xref = QPDFXRefEntry(new_stream_id, count);
+        }
+    }
+    {
+        PipelinePopper pp_ostream(this, &stream_buffer);
+        // Adjust offsets to skip over comment before first object
+        first = offsets.at(0);
+        for (auto& iter: offsets) {
+            iter -= first;
         }
-        if (pass == 1) {
-            int count = -1;
-            for (auto const& obj: m->object_stream_to_objects[old_id]) {
-                ++count;
-                int new_obj = m->obj[obj].renumber;
-                if (first_obj == -1) {
-                    first_obj = new_obj;
-                }
-                if (m->qdf_mode) {
-                    writeString(
-                        "%% Object stream: object " + std::to_string(new_obj) + ", index " +
-                        std::to_string(count));
-                    if (!m->suppress_original_object_ids) {
-                        writeString("; original object ID: " + std::to_string(obj.getObj()));
-                        // For compatibility, only write the generation if non-zero.  While object
-                        // streams only allow objects with generation 0, if we are generating object
-                        // streams, the old object could have a non-zero generation.
-                        if (obj.getGen() != 0) {
-                            QTC::TC("qpdf", "QPDFWriter original obj non-zero gen");
-                            writeString(" " + std::to_string(obj.getGen()));
-                        }
-                    }
-                    writeString("\n");
-                }
-                if (pass == 1) {
-                    offsets.push_back(m->pipeline->getCount());
-                    // To avoid double-counting objects being written in object streams for progress
-                    // reporting, decrement in pass 1.
-                    indicateProgress(true, false);
-                }
-                QPDFObjectHandle obj_to_write = m->pdf.getObject(obj);
-                if (obj_to_write.isStream()) {
-                    // This condition occurred in a fuzz input. Ideally we should block it at parse
-                    // time, but it's not clear to me how to construct a case for this.
-                    obj_to_write.warnIfPossible(
-                        "stream found inside object stream; treating as null");
-                    obj_to_write = QPDFObjectHandle::newNull();
-                }
-                writeObject(obj_to_write, count);
+        // Take one pass at writing pairs of numbers so we can get their size information
+        {
+            PipelinePopper pp_discard(this);
+            pushDiscardFilter(pp_discard);
+            writeObjectStreamOffsets(offsets, first_obj);
+            first += m->pipeline->getCount();
+        }
-                m->new_obj[new_obj].xref = QPDFXRefEntry(new_stream_id, count);
-            }
+        // Set up a stream to write the stream data into a buffer.
+        Pipeline* next = pushPipeline(new Pl_Buffer("object stream"));
+        if (m->compress_streams && !m->qdf_mode) {
+            compressed = true;
+            next = pushPipeline(new Pl_Flate("compress object stream", next, Pl_Flate::a_deflate));
         }
+        activatePipelineStack(pp_ostream);
+        writeObjectStreamOffsets(offsets, first_obj);
+        writeBuffer(stream_buffer);
     }
     // Write the object
@@ -26,6 +26,9 @@ more detail.
     - There have been further enhancements to how files with damaged xref
       tables are recovered.
+    - There has been some refactoring of how object streams are written with
+      some performance improvement.
+
 .. cSpell:ignore substract
 .. _r12-0-0: