Merge "Rename the variable 'quality' to 'brotli_quality'" am: a31cf35116

am: 5cd1cd51cb

Change-Id: Ib91006b05bbc156f98e1aa2ef20713dc78614561

bsdiff_arguments.cc

@@ -31,7 +31,7 @@ const struct option OPTIONS[] = {
     {"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 @@ namespace bsdiff {
 
 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 @@ bool BsdiffArguments::ParseCommandLine(int argc, char** argv) {
       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 @@ bool BsdiffArguments::ParseCommandLine(int argc, char** argv) {
 
   // 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 @@ bool BsdiffArguments::ParseBsdiffFormat(const string& str,
   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 @@ bool BsdiffArguments::ParseQuality(const string& str, int* quality) {
     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;
   }

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 @@ class BsdiffArguments {
 
   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 @@ class BsdiffArguments {
   // 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 @@ class BsdiffArguments {
   // 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};
 };

bsdiff_arguments_unittest.cc

@@ -40,13 +40,13 @@ TEST(BsdiffArgumentsTest, ParseBsdiffFormatTest) {
 
 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, ArgumentsValidTest) {
 
 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 @@ TEST(BsdiffArgumentsTest, ParseArgumentsSmokeTest) {
 
   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());
 }
 

bsdiff_main.cc

@@ -79,13 +79,12 @@ int GenerateBsdiffFromFiles(const char* old_filename,
   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 @@ void PrintUsage(const std::string& proc_name) {
                "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

endsley_patch_writer.cc

@@ -42,7 +42,7 @@ bool EndsleyPatchWriter::Init(size_t new_size) {
       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;

endsley_patch_writer.h

@@ -42,8 +42,10 @@ class EndsleyPatchWriter : public PatchWriterInterface {
   // 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 @@ class EndsleyPatchWriter : public PatchWriterInterface {
 
   // The compressor type to use and its quality (if any).
   CompressorType compressor_type_;
-  int quality_;
+  int brotli_quality_;
 
   std::unique_ptr<CompressorInterface> compressor_;
 

include/bsdiff/patch_writer_factory.h

@@ -22,12 +22,13 @@ std::unique_ptr<PatchWriterInterface> CreateBsdiffPatchWriter(
     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 @@ BSDIFF_EXPORT
 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

patch_writer.cc

@@ -35,16 +35,16 @@ BsdiffPatchWriter::BsdiffPatchWriter(const std::string& patch_filename)
 
 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));
   }
 }
 

patch_writer.h

@@ -23,11 +23,11 @@ class BsdiffPatchWriter : public PatchWriterInterface {
   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;

patch_writer_factory.cc

@@ -18,17 +18,17 @@ std::unique_ptr<PatchWriterInterface> CreateBsdiffPatchWriter(
 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(