Rename the variable 'quality' to 'brotli_quality'
Right now the quality is only used in the brotli compressor. And it
would be ambiguous if we add another compressor in the future; since the
compressors may have different interpretion on the quality value.
Test: unit tests pass; run bsdiff
Change-Id: I23af06a135b2b47df5209171f7db0773ac02a326
diff --git a/bsdiff_arguments.cc b/bsdiff_arguments.cc
index ebcddf1..ae23fe7 100644
--- a/bsdiff_arguments.cc
+++ b/bsdiff_arguments.cc
@@ -31,7 +31,7 @@
{"format", required_argument, nullptr, 0},
{"minlen", required_argument, nullptr, 0},
{"type", required_argument, nullptr, 0},
- {"quality", required_argument, nullptr, 0},
+ {"brotli_quality", required_argument, nullptr, 0},
{nullptr, 0, nullptr, 0},
};
@@ -43,8 +43,8 @@
bool BsdiffArguments::IsValid() const {
if (compressor_type_ == CompressorType::kBrotli &&
- (compression_quality_ < BROTLI_MIN_QUALITY ||
- compression_quality_ > BROTLI_MAX_QUALITY)) {
+ (brotli_quality_ < BROTLI_MIN_QUALITY ||
+ brotli_quality_ > BROTLI_MAX_QUALITY)) {
return false;
}
@@ -81,8 +81,9 @@
if (!ParseCompressorType(optarg, &compressor_type_)) {
return false;
}
- } else if (name == "quality") {
- if (!ParseQuality(optarg, &compression_quality_)) {
+ } else if (name == "brotli_quality") {
+ if (!ParseQuality(optarg, &brotli_quality_, BROTLI_MIN_QUALITY,
+ BROTLI_MAX_QUALITY)) {
return false;
}
} else {
@@ -93,13 +94,12 @@
// If quality is uninitialized for brotli, set it to default value.
if (format_ != BsdiffFormat::kLegacy &&
- compressor_type_ == CompressorType::kBrotli &&
- compression_quality_ == -1) {
- compression_quality_ = kBrotliDefaultQuality;
+ compressor_type_ == CompressorType::kBrotli && brotli_quality_ == -1) {
+ brotli_quality_ = kBrotliDefaultQuality;
} else if (compressor_type_ != CompressorType::kBrotli &&
- compression_quality_ != -1) {
- std::cerr << "Warning: Compression quality is only used in the brotli"
- " compressor." << endl;
+ brotli_quality_ != -1) {
+ std::cerr << "Warning: Brotli quality is only used in the brotli"
+ " compressor.\n";
}
return true;
@@ -161,7 +161,10 @@
return false;
}
-bool BsdiffArguments::ParseQuality(const string& str, int* quality) {
+bool BsdiffArguments::ParseQuality(const string& str,
+ int* quality,
+ int min,
+ int max) {
errno = 0;
char* end;
const char* s = str.c_str();
@@ -170,7 +173,7 @@
return false;
}
- if (result < BROTLI_MIN_QUALITY || result > BROTLI_MAX_QUALITY) {
+ if (result < min || result > max) {
std::cerr << "Compression quality out of range " << str << endl;
return false;
}
diff --git a/bsdiff_arguments.h b/bsdiff_arguments.h
index b330834..1907e73 100644
--- a/bsdiff_arguments.h
+++ b/bsdiff_arguments.h
@@ -14,18 +14,19 @@
namespace bsdiff {
-// Class to store the patch writer options about format, type and quality.
+// Class to store the patch writer options about format, type and
+// brotli_quality.
class BsdiffArguments {
public:
BsdiffArguments()
: format_(BsdiffFormat::kLegacy),
compressor_type_(CompressorType::kBZ2),
- compression_quality_(-1) {}
+ brotli_quality_(-1) {}
- BsdiffArguments(BsdiffFormat format, CompressorType type, int quality)
+ BsdiffArguments(BsdiffFormat format, CompressorType type, int brotli_quality)
: format_(format),
compressor_type_(type),
- compression_quality_(quality) {}
+ brotli_quality_(brotli_quality) {}
// Check if the compressor type is compatible with the bsdiff format.
bool IsValid() const;
@@ -37,7 +38,7 @@
CompressorType compressor_type() const { return compressor_type_; }
- int compression_quality() const { return compression_quality_; }
+ int brotli_quality() const { return brotli_quality_; }
// Parse the command line arguments of the main function and set all the
// fields accordingly.
@@ -52,8 +53,12 @@
// Parse the bsdiff format from string.
static bool ParseBsdiffFormat(const std::string& str, BsdiffFormat* format);
- // Parse the compression quality (for brotli) from string.
- static bool ParseQuality(const std::string& str, int* quality);
+ // Parse the compression quality (for brotli) from string; also check if the
+ // value is within the valid range.
+ static bool ParseQuality(const std::string& str,
+ int* quality,
+ int min,
+ int max);
private:
// Current format supported are the legacy "BSDIFF40" or "BSDF2".
@@ -62,9 +67,8 @@
// The algorithm to compress the patch, i.e. BZ2 or Brotli.
CompressorType compressor_type_;
- // The quality of compression, only valid when using brotli as the
- // compression algorithm.
- int compression_quality_;
+ // The quality of brotli compressor.
+ int brotli_quality_;
size_t min_length_{0};
};
diff --git a/bsdiff_arguments_unittest.cc b/bsdiff_arguments_unittest.cc
index 412b8e9..51e1038 100644
--- a/bsdiff_arguments_unittest.cc
+++ b/bsdiff_arguments_unittest.cc
@@ -40,13 +40,13 @@
TEST(BsdiffArgumentsTest, ParseQualityTest) {
int quality;
- EXPECT_TRUE(BsdiffArguments::ParseQuality("9", &quality));
+ EXPECT_TRUE(BsdiffArguments::ParseQuality("9", &quality, 0, 11));
EXPECT_EQ(9, quality);
// Check the out of range quality values.
- EXPECT_FALSE(BsdiffArguments::ParseQuality("30", &quality));
- EXPECT_FALSE(BsdiffArguments::ParseQuality("1234567890", &quality));
- EXPECT_FALSE(BsdiffArguments::ParseQuality("aabb", &quality));
+ EXPECT_FALSE(BsdiffArguments::ParseQuality("30", &quality, 0, 11));
+ EXPECT_FALSE(BsdiffArguments::ParseQuality("1234567890", &quality, 0, 1000));
+ EXPECT_FALSE(BsdiffArguments::ParseQuality("aabb", &quality, 0, 1000));
}
TEST(BsdiffArgumentsTest, ParseMinLengthTest) {
@@ -80,7 +80,7 @@
TEST(BsdiffArgumentsTest, ParseArgumentsSmokeTest) {
std::vector<const char*> args = {"bsdiff", "--format=bsdf2", "--type=brotli",
- "--quality=9", "--minlen=12"};
+ "--brotli_quality=9", "--minlen=12"};
BsdiffArguments arguments;
EXPECT_TRUE(
@@ -88,7 +88,7 @@
EXPECT_EQ(BsdiffFormat::kBsdf2, arguments.format());
EXPECT_EQ(CompressorType::kBrotli, arguments.compressor_type());
- EXPECT_EQ(9, arguments.compression_quality());
+ EXPECT_EQ(9, arguments.brotli_quality());
EXPECT_EQ(12, arguments.min_length());
}
diff --git a/bsdiff_main.cc b/bsdiff_main.cc
index bed38b6..c4753c7 100644
--- a/bsdiff_main.cc
+++ b/bsdiff_main.cc
@@ -79,13 +79,12 @@
if (arguments.format() == bsdiff::BsdiffFormat::kLegacy) {
patch_writer = bsdiff::CreateBsdiffPatchWriter(patch_filename);
} else if (arguments.format() == bsdiff::BsdiffFormat::kBsdf2) {
- patch_writer = bsdiff::CreateBSDF2PatchWriter(
- patch_filename, arguments.compressor_type(),
- arguments.compression_quality());
+ patch_writer = bsdiff::CreateBSDF2PatchWriter(patch_filename,
+ arguments.compressor_type(),
+ arguments.brotli_quality());
} else if (arguments.format() == bsdiff::BsdiffFormat::kEndsley) {
- patch_writer =
- bsdiff::CreateEndsleyPatchWriter(&raw_data, arguments.compressor_type(),
- arguments.compression_quality());
+ patch_writer = bsdiff::CreateEndsleyPatchWriter(
+ &raw_data, arguments.compressor_type(), arguments.brotli_quality());
} else {
std::cerr << "unexpected bsdiff format." << std::endl;
return 1;
@@ -122,8 +121,8 @@
"required to consider a match in the algorithm.\n"
<< " --type <bz2|brotli|nocompression> The algorithm to compress "
"the patch, bsdf2 format only.\n"
- << " --quality Quality of the patch "
- "compression, brotli only.\n";
+ << " --brotli_quality Quality of the brotli "
+ "compressor.\n";
}
} // namespace
diff --git a/endsley_patch_writer.cc b/endsley_patch_writer.cc
index e40406d..0e8602c 100644
--- a/endsley_patch_writer.cc
+++ b/endsley_patch_writer.cc
@@ -42,7 +42,7 @@
patch_->reserve(new_size);
break;
case CompressorType::kBrotli:
- compressor_.reset(new BrotliCompressor(quality_));
+ compressor_.reset(new BrotliCompressor(brotli_quality_));
if (!compressor_) {
LOG(ERROR) << "Error creating brotli compressor.";
return false;
diff --git a/endsley_patch_writer.h b/endsley_patch_writer.h
index a3170f6..e991d4a 100644
--- a/endsley_patch_writer.h
+++ b/endsley_patch_writer.h
@@ -42,8 +42,10 @@
// compressed using the compressor type |type|.
EndsleyPatchWriter(std::vector<uint8_t>* patch,
CompressorType type,
- int quality)
- : patch_(patch), compressor_type_(type), quality_(quality) {}
+ int brotli_quality)
+ : patch_(patch),
+ compressor_type_(type),
+ brotli_quality_(brotli_quality) {}
// PatchWriterInterface overrides.
bool Init(size_t new_size) override;
@@ -67,7 +69,7 @@
// The compressor type to use and its quality (if any).
CompressorType compressor_type_;
- int quality_;
+ int brotli_quality_;
std::unique_ptr<CompressorInterface> compressor_;
diff --git a/include/bsdiff/patch_writer_factory.h b/include/bsdiff/patch_writer_factory.h
index 64ec4fa..82abafd 100644
--- a/include/bsdiff/patch_writer_factory.h
+++ b/include/bsdiff/patch_writer_factory.h
@@ -22,12 +22,13 @@
const std::string& patch_filename);
// Create a patch writer using the "BSDF2" patch format. It uses the compressor
-// specified in |type| with compression quality |quality|.
+// specified in |type|. And the brotli compressor will have the compression
+// quality |brotli_quality|.
BSDIFF_EXPORT
std::unique_ptr<PatchWriterInterface> CreateBSDF2PatchWriter(
const std::string& patch_filename,
CompressorType type,
- int quality);
+ int brotli_quality);
// Create a patch writer compatible with Android Play Store bsdiff patches.
// The data will be written to the passed |patch| vector, which must be valid
@@ -38,7 +39,7 @@
std::unique_ptr<PatchWriterInterface> CreateEndsleyPatchWriter(
std::vector<uint8_t>* patch,
CompressorType type,
- int quality);
+ int brotli_quality);
// Helper function to create an Endsley patch writer with no compression.
BSDIFF_EXPORT
diff --git a/patch_writer.cc b/patch_writer.cc
index 7bed2ca..a4fee4a 100644
--- a/patch_writer.cc
+++ b/patch_writer.cc
@@ -35,16 +35,16 @@
BsdiffPatchWriter::BsdiffPatchWriter(const std::string& patch_filename,
CompressorType type,
- int quality)
+ int brotli_quality)
: patch_filename_(patch_filename), format_(BsdiffFormat::kBsdf2) {
if (type == CompressorType::kBZ2) {
ctrl_stream_.reset(new BZ2Compressor());
diff_stream_.reset(new BZ2Compressor());
extra_stream_.reset(new BZ2Compressor());
} else if (type == CompressorType::kBrotli) {
- ctrl_stream_.reset(new BrotliCompressor(quality));
- diff_stream_.reset(new BrotliCompressor(quality));
- extra_stream_.reset(new BrotliCompressor(quality));
+ ctrl_stream_.reset(new BrotliCompressor(brotli_quality));
+ diff_stream_.reset(new BrotliCompressor(brotli_quality));
+ extra_stream_.reset(new BrotliCompressor(brotli_quality));
}
}
diff --git a/patch_writer.h b/patch_writer.h
index 0733205..7ce57fd 100644
--- a/patch_writer.h
+++ b/patch_writer.h
@@ -23,11 +23,11 @@
explicit BsdiffPatchWriter(const std::string& patch_filename);
// Create the patch writer using the "BSDF2" format. It uses the compressor
- // with algorithm |type| and quality |quality|. This writer also writes the
- // patch data to the file |patch_filename|.
+ // with algorithm |type|; and quality |brotli_quality| if it's brotli. This
+ // writer also writes the patch data to the file |patch_filename|.
BsdiffPatchWriter(const std::string& patch_filename,
CompressorType type,
- int quality);
+ int brotli_quality);
// PatchWriterInterface overrides.
bool Init(size_t new_size) override;
diff --git a/patch_writer_factory.cc b/patch_writer_factory.cc
index 8c29bf2..b9a41f3 100644
--- a/patch_writer_factory.cc
+++ b/patch_writer_factory.cc
@@ -18,17 +18,17 @@
std::unique_ptr<PatchWriterInterface> CreateBSDF2PatchWriter(
const std::string& patch_filename,
CompressorType type,
- int quality) {
+ int brotli_quality) {
return std::unique_ptr<PatchWriterInterface>(
- new BsdiffPatchWriter(patch_filename, type, quality));
+ new BsdiffPatchWriter(patch_filename, type, brotli_quality));
}
std::unique_ptr<PatchWriterInterface> CreateEndsleyPatchWriter(
std::vector<uint8_t>* patch,
CompressorType type,
- int quality) {
+ int brotli_quality) {
return std::unique_ptr<PatchWriterInterface>(
- new EndsleyPatchWriter(patch, type, quality));
+ new EndsleyPatchWriter(patch, type, brotli_quality));
}
std::unique_ptr<PatchWriterInterface> CreateEndsleyPatchWriter(