Commit 587177b175cfdb5fdce72b449b69bfcf860ce51c
Committed by
GitHub
Merge pull request #1514 from m-holger/imopt
Refactor `ImageOptimizer`: enforce `final` overrides, switch to `uniq…
Showing
1 changed file
with
21 additions
and
24 deletions
libqpdf/QPDFJob.cc
| ... | ... | @@ -6,7 +6,7 @@ |
| 6 | 6 | |
| 7 | 7 | #include <qpdf/ClosedFileInputSource.hh> |
| 8 | 8 | #include <qpdf/FileInputSource.hh> |
| 9 | -#include <qpdf/Pl_Count.hh> | |
| 9 | +#include <qpdf/Pipeline_private.hh> | |
| 10 | 10 | #include <qpdf/Pl_DCT.hh> |
| 11 | 11 | #include <qpdf/Pl_Discard.hh> |
| 12 | 12 | #include <qpdf/Pl_Flate.hh> |
| ... | ... | @@ -37,7 +37,7 @@ using namespace qpdf; |
| 37 | 37 | |
| 38 | 38 | namespace |
| 39 | 39 | { |
| 40 | - class ImageOptimizer: public QPDFObjectHandle::StreamDataProvider | |
| 40 | + class ImageOptimizer final: public QPDFObjectHandle::StreamDataProvider | |
| 41 | 41 | { |
| 42 | 42 | public: |
| 43 | 43 | ImageOptimizer( |
| ... | ... | @@ -47,12 +47,13 @@ namespace |
| 47 | 47 | size_t oi_min_area, |
| 48 | 48 | int quality, |
| 49 | 49 | QPDFObjectHandle& image); |
| 50 | - ~ImageOptimizer() override = default; | |
| 51 | - void provideStreamData(QPDFObjGen const&, Pipeline* pipeline) override; | |
| 52 | - std::shared_ptr<Pipeline> makePipeline(std::string const& description, Pipeline* next); | |
| 50 | + ~ImageOptimizer() final = default; | |
| 51 | + void provideStreamData(QPDFObjGen const&, Pipeline* pipeline) final; | |
| 53 | 52 | bool evaluate(std::string const& description); |
| 54 | 53 | |
| 55 | 54 | private: |
| 55 | + std::unique_ptr<Pipeline> makePipeline(std::string const& description, Pipeline* next); | |
| 56 | + | |
| 56 | 57 | QPDFJob& o; |
| 57 | 58 | size_t oi_min_width; |
| 58 | 59 | size_t oi_min_height; |
| ... | ... | @@ -113,10 +114,9 @@ ImageOptimizer::ImageOptimizer( |
| 113 | 114 | } |
| 114 | 115 | } |
| 115 | 116 | |
| 116 | -std::shared_ptr<Pipeline> | |
| 117 | +std::unique_ptr<Pipeline> | |
| 117 | 118 | ImageOptimizer::makePipeline(std::string const& description, Pipeline* next) |
| 118 | 119 | { |
| 119 | - std::shared_ptr<Pipeline> result; | |
| 120 | 120 | QPDFObjectHandle dict = image.getDict(); |
| 121 | 121 | QPDFObjectHandle w_obj = dict.getKey("/Width"); |
| 122 | 122 | QPDFObjectHandle h_obj = dict.getKey("/Height"); |
| ... | ... | @@ -128,10 +128,10 @@ ImageOptimizer::makePipeline(std::string const& description, Pipeline* next) |
| 128 | 128 | << ": not optimizing because image dictionary is missing required keys\n"; |
| 129 | 129 | }); |
| 130 | 130 | } |
| 131 | - return result; | |
| 131 | + return {}; | |
| 132 | 132 | } |
| 133 | 133 | QPDFObjectHandle components_obj = dict.getKey("/BitsPerComponent"); |
| 134 | - if (!(components_obj.isInteger() && (components_obj.getIntValue() == 8))) { | |
| 134 | + if (!(components_obj.isInteger() && components_obj.getIntValue() == 8)) { | |
| 135 | 135 | QTC::TC("qpdf", "QPDFJob image optimize bits per component"); |
| 136 | 136 | if (!description.empty()) { |
| 137 | 137 | o.doIfVerbose([&](Pipeline& v, std::string const& prefix) { |
| ... | ... | @@ -139,7 +139,7 @@ ImageOptimizer::makePipeline(std::string const& description, Pipeline* next) |
| 139 | 139 | << ": not optimizing because image has other than 8 bits per component\n"; |
| 140 | 140 | }); |
| 141 | 141 | } |
| 142 | - return result; | |
| 142 | + return {}; | |
| 143 | 143 | } |
| 144 | 144 | // Files have been seen in the wild whose width and height are floating point, which is goofy, |
| 145 | 145 | // but we can deal with it. |
| ... | ... | @@ -175,11 +175,10 @@ ImageOptimizer::makePipeline(std::string const& description, Pipeline* next) |
| 175 | 175 | << ": not optimizing because qpdf can't optimize images with this colorspace\n"; |
| 176 | 176 | }); |
| 177 | 177 | } |
| 178 | - return result; | |
| 178 | + return {}; | |
| 179 | 179 | } |
| 180 | - if (((oi_min_width > 0) && (w <= oi_min_width)) || | |
| 181 | - ((oi_min_height > 0) && (h <= oi_min_height)) || | |
| 182 | - ((oi_min_area > 0) && ((w * h) <= oi_min_area))) { | |
| 180 | + if ((oi_min_width > 0 && w <= oi_min_width) || (oi_min_height > 0 && h <= oi_min_height) || | |
| 181 | + (oi_min_area > 0 && (w * h) <= oi_min_area)) { | |
| 183 | 182 | QTC::TC("qpdf", "QPDFJob image optimize too small"); |
| 184 | 183 | if (!description.empty()) { |
| 185 | 184 | o.doIfVerbose([&](Pipeline& v, std::string const& prefix) { |
| ... | ... | @@ -188,11 +187,10 @@ ImageOptimizer::makePipeline(std::string const& description, Pipeline* next) |
| 188 | 187 | "dimensions\n"; |
| 189 | 188 | }); |
| 190 | 189 | } |
| 191 | - return result; | |
| 190 | + return {}; | |
| 192 | 191 | } |
| 193 | 192 | |
| 194 | - result = std::make_shared<Pl_DCT>("jpg", next, w, h, components, cs, config.get()); | |
| 195 | - return result; | |
| 193 | + return std::make_unique<Pl_DCT>("jpg", next, w, h, components, cs, config.get()); | |
| 196 | 194 | } |
| 197 | 195 | |
| 198 | 196 | bool |
| ... | ... | @@ -207,10 +205,9 @@ ImageOptimizer::evaluate(std::string const& description) |
| 207 | 205 | }); |
| 208 | 206 | return false; |
| 209 | 207 | } |
| 210 | - Pl_Discard d; | |
| 211 | - Pl_Count c("count", &d); | |
| 212 | - std::shared_ptr<Pipeline> p = makePipeline(description, &c); | |
| 213 | - if (p == nullptr) { | |
| 208 | + pl::Count c(0); | |
| 209 | + auto p = makePipeline(description, &c); | |
| 210 | + if (!p) { | |
| 214 | 211 | // message issued by makePipeline |
| 215 | 212 | return false; |
| 216 | 213 | } |
| ... | ... | @@ -236,8 +233,8 @@ ImageOptimizer::evaluate(std::string const& description) |
| 236 | 233 | void |
| 237 | 234 | ImageOptimizer::provideStreamData(QPDFObjGen const&, Pipeline* pipeline) |
| 238 | 235 | { |
| 239 | - std::shared_ptr<Pipeline> p = makePipeline("", pipeline); | |
| 240 | - if (p == nullptr) { | |
| 236 | + auto p = makePipeline("", pipeline); | |
| 237 | + if (!p) { | |
| 241 | 238 | // Should not be possible |
| 242 | 239 | image.warnIfPossible( |
| 243 | 240 | "unable to create pipeline after previous success; image data will be lost"); |
| ... | ... | @@ -836,7 +833,7 @@ QPDFJob::doShowObj(QPDF& pdf) |
| 836 | 833 | if (obj.isStream()) { |
| 837 | 834 | if (m->show_raw_stream_data || m->show_filtered_stream_data) { |
| 838 | 835 | bool filter = m->show_filtered_stream_data; |
| 839 | - if (filter && (!obj.pipeStreamData(nullptr, 0, qpdf_dl_all))) { | |
| 836 | + if (filter && !obj.pipeStreamData(nullptr, 0, qpdf_dl_all)) { | |
| 840 | 837 | QTC::TC("qpdf", "QPDFJob unable to filter"); |
| 841 | 838 | obj.warnIfPossible("unable to filter stream data"); |
| 842 | 839 | error = true; | ... | ... |