Merge "Add an argument parser in bsdiff" am: 922cdaab26

am: 937a978882

Change-Id: I3073a6f02311a8be228074c9e3cfe6ff57019320

Android.bp

@@ -60,7 +60,6 @@ cc_library_static {
         "bsdiff.cc",
         "bz2_compressor.cc",
         "compressor_buffer.cc",
-        "compressor_interface.cc",
         "diff_encoder.cc",
         "endsley_patch_writer.cc",
         "logging.cc",
@@ -94,7 +93,10 @@ cc_binary_host {
     name: "bsdiff",
     defaults: ["bsdiff_defaults"],
 
-    srcs: ["bsdiff_main.cc"],
+    srcs: [
+        "bsdiff_arguments.cc",
+        "bsdiff_main.cc",
+    ],
     static_libs: [
         "libbsdiff",
         "libdivsufsort64",
@@ -111,6 +113,8 @@ cc_test {
     test_suites: ["device-tests"],
     srcs: [
         "brotli_compressor_unittest.cc",
+        "bsdiff_arguments.cc",
+        "bsdiff_arguments_unittest.cc",
         "bsdiff_unittest.cc",
         "bspatch_unittest.cc",
         "diff_encoder_unittest.cc",

Makefile

@@ -37,7 +37,6 @@ bsdiff_src_files := \
     bsdiff.cc \
     bz2_compressor.cc \
     compressor_buffer.cc \
-    compressor_interface.cc \
     diff_encoder.cc \
     endsley_patch_writer.cc \
     logging.cc \
@@ -65,6 +64,8 @@ bspatch_src_files := \
 # Unit test files.
 bsdiff_common_unittests := \
     brotli_compressor_unittest.cc \
+    bsdiff_arguments.cc \
+    bsdiff_arguments_unittest.cc \
     bsdiff_unittest.cc \
     bspatch_unittest.cc \
     diff_encoder_unittest.cc \
@@ -81,14 +82,13 @@ bsdiff_common_unittests := \
 
 BSDIFF_LIBS := -lbz2 -lbrotlienc -ldivsufsort -ldivsufsort64
 BSDIFF_OBJS := $(bsdiff_src_files:.cc=.o)
-
 BSPATCH_LIBS := -lbz2 -lbrotlidec
 BSPATCH_OBJS := $(bspatch_src_files:.cc=.o)
 
 UNITTEST_LIBS = -lgmock -lgtest -lpthread
 UNITTEST_OBJS := $(bsdiff_common_unittests:.cc=.o)
 
-bsdiff: $(BSDIFF_OBJS) bsdiff_main.o
+bsdiff: $(BSDIFF_OBJS) bsdiff_arguments.o bsdiff_main.o
 bsdiff: LDLIBS += $(BSDIFF_LIBS)
 libbsdiff.so: $(BSDIFF_OBJS)
 libbsdiff.so: LDLIBS += $(BSDIFF_LIBS)

brotli_compressor.cc

@@ -7,25 +7,31 @@
 #include "bsdiff/logging.h"
 
 namespace {
-// TODO(xunchang) set brotli compression parameters based on input options.
+
 const size_t kBufferSize = 1024 * 1024;
-const uint32_t kBrotliQuality = 11;
-const uint32_t kBrotliLgwin = 20;
+const uint32_t kBrotliDefaultLgwin = 20;
 
 }  // namespace
 
 namespace bsdiff {
-
-BrotliCompressor::BrotliCompressor() : comp_buffer_(kBufferSize) {
+BrotliCompressor::BrotliCompressor(int quality) : comp_buffer_(kBufferSize) {
   brotli_encoder_state_ =
       BrotliEncoderCreateInstance(nullptr, nullptr, nullptr);
   if (!brotli_encoder_state_) {
     LOG(ERROR) << "Failed to initialize brotli decoder state";
   } else {
+    int compression_quality = quality;
+    if (compression_quality > BROTLI_MAX_QUALITY ||
+        compression_quality < BROTLI_MIN_QUALITY) {
+      LOG(ERROR) << "Invalid quality value: " << quality
+                 << ", using default quality instead.";
+      compression_quality = BROTLI_MAX_QUALITY;
+    }
+
     BrotliEncoderSetParameter(brotli_encoder_state_, BROTLI_PARAM_QUALITY,
-                              kBrotliQuality);
+                              compression_quality);
     BrotliEncoderSetParameter(brotli_encoder_state_, BROTLI_PARAM_LGWIN,
-                              kBrotliLgwin);
+                              kBrotliDefaultLgwin);
   }
 }
 

brotli_compressor.h

@@ -19,7 +19,11 @@ namespace bsdiff {
 
 class BrotliCompressor : public CompressorInterface {
  public:
-  BrotliCompressor();
+  // Create a brotli compressor with the compression quality |quality|. As the
+  // value of quality increases, the compression becomes better but slower.
+  // The valid range of quality is between BROTLI_MIN_QUALITY and
+  // BROTLI_MAX_QUALITY; and the caller is responsible for the validity check.
+  explicit BrotliCompressor(int quality);
   ~BrotliCompressor() override;
 
   // CompressorInterface overrides.

brotli_compressor_unittest.cc

@@ -18,7 +18,7 @@ constexpr uint8_t kHelloWorld[] = {
 namespace bsdiff {
 
 TEST(BrotliCompressorTest, BrotliCompressorSmoke) {
-  BrotliCompressor brotli_compressor;
+  BrotliCompressor brotli_compressor(11);
   EXPECT_TRUE(brotli_compressor.Write(kHelloWorld, sizeof(kHelloWorld)));
   EXPECT_TRUE(brotli_compressor.Finish());
   std::vector<uint8_t> compressed_data = brotli_compressor.GetCompressedData();
@@ -39,7 +39,7 @@ TEST(BrotliCompressorTest, BrotliCompressorSmoke) {
 
 TEST(BrotliCompressorTest, BrotliCompressorEmptyStream) {
   uint8_t empty_buffer[] = {};
-  BrotliCompressor brotli_compressor;
+  BrotliCompressor brotli_compressor(11);
   EXPECT_TRUE(brotli_compressor.Write(empty_buffer, sizeof(empty_buffer)));
   EXPECT_TRUE(brotli_compressor.Finish());
 

bsdiff.cc

@@ -49,7 +49,7 @@ namespace bsdiff {
 int bsdiff(const uint8_t* old_buf, size_t oldsize, const uint8_t* new_buf,
            size_t newsize, const char* patch_filename,
            SuffixArrayIndexInterface** sai_cache) {
-	BsdiffPatchWriter patch(patch_filename, BsdiffFormat::kLegacy);
+	BsdiffPatchWriter patch(patch_filename);
 	return bsdiff(old_buf, oldsize, new_buf, newsize, &patch, sai_cache);
 }
 

bsdiff.gyp

@@ -53,7 +53,6 @@
         'bsdiff.cc',
         'bz2_compressor.cc',
         'compressor_buffer.cc',
-        'compressor_interface.cc',
         'diff_encoder.cc',
         'endsley_patch_writer.cc',
         'logging.cc',
@@ -79,6 +78,7 @@
         'libbsdiff',
       ],
       'sources': [
+        'bsdiff_arguments.cc',
         'bsdiff_main.cc',
       ],
     },
@@ -151,6 +151,8 @@
           'includes': ['../common-mk/common_test.gypi'],
           'sources': [
             'brotli_compressor_unittest.cc',
+            'bsdiff_arguments.cc',
+            'bsdiff_arguments_unittest.cc',
             'bsdiff_unittest.cc',
             'bspatch_unittest.cc',
             'diff_encoder_unittest.cc',

bsdiff_arguments.cc

@@ -0,0 +1,147 @@
+#include "bsdiff/bsdiff_arguments.h"
+
+#include <getopt.h>
+
+#include <algorithm>
+#include <iostream>
+
+#include "brotli/encode.h"
+
+using std::endl;
+using std::string;
+
+namespace {
+
+// The name in string for the compression algorithms.
+constexpr char kNoCompressionString[] = "nocompression";
+constexpr char kBZ2String[] = "bz2";
+constexpr char kBrotliString[] = "brotli";
+
+// The name in string for the bsdiff format.
+constexpr char kLegacyString[] = "legacy";
+constexpr char kBsdf2String[] = "bsdf2";
+constexpr char kBsdiff40String[] = "bsdiff40";
+
+const struct option OPTIONS[] = {
+    {"format", required_argument, nullptr, 0},
+    {"type", required_argument, nullptr, 0},
+    {"quality", required_argument, nullptr, 0},
+    {nullptr, 0, nullptr, 0},
+};
+
+const uint32_t kBrotliDefaultQuality = BROTLI_MAX_QUALITY;
+
+}  // namespace
+
+namespace bsdiff {
+
+bool BsdiffArguments::IsValid() const {
+  if (format_ == BsdiffFormat::kLegacy) {
+    return (compressor_type_ == CompressorType::kBZ2);
+  } else if (format_ == BsdiffFormat::kBsdf2) {
+    if (compressor_type_ == CompressorType::kBZ2) {
+      return true;
+    }
+    if (compressor_type_ == CompressorType::kBrotli) {
+      return (compression_quality_ >= BROTLI_MIN_QUALITY &&
+              compression_quality_ <= BROTLI_MAX_QUALITY);
+    }
+  }
+  return false;
+}
+
+bool BsdiffArguments::ParseCommandLine(int argc, char** argv) {
+  int opt;
+  int option_index;
+  while ((opt = getopt_long(argc, argv, "", OPTIONS, &option_index)) != -1) {
+    if (opt != 0) {
+      return false;
+    }
+
+    std::string name = OPTIONS[option_index].name;
+    if (name == "format") {
+      if (!ParseBsdiffFormat(optarg, &format_)) {
+        return false;
+      }
+    } else if (name == "type") {
+      if (!ParseCompressorType(optarg, &compressor_type_)) {
+        return false;
+      }
+    } else if (name == "quality") {
+      if (!ParseQuality(optarg, &compression_quality_)) {
+        return false;
+      }
+    } else {
+      std::cerr << "Unrecognized options: " << name << endl;
+      return false;
+    }
+  }
+
+  // If quality is uninitialized for brotli, set it to default value.
+  if (format_ == BsdiffFormat::kBsdf2 &&
+      compressor_type_ == CompressorType::kBrotli &&
+      compression_quality_ == -1) {
+    compression_quality_ = kBrotliDefaultQuality;
+  } else if (compressor_type_ != CompressorType::kBrotli &&
+             compression_quality_ != -1) {
+    std::cerr << "Warning: Compression quality is only used in the brotli"
+                 " compressor." << endl;
+  }
+
+  return true;
+}
+
+bool BsdiffArguments::ParseCompressorType(const string& str,
+                                          CompressorType* type) {
+  string type_string = str;
+  std::transform(type_string.begin(), type_string.end(), type_string.begin(),
+                 ::tolower);
+  if (type_string == kNoCompressionString) {
+    *type = CompressorType::kNoCompression;
+    return true;
+  } else if (type_string == kBZ2String) {
+    *type = CompressorType::kBZ2;
+    return true;
+  } else if (type_string == kBrotliString) {
+    *type = CompressorType::kBrotli;
+    return true;
+  }
+  std::cerr << "Failed to parse compressor type in " << str << endl;
+  return false;
+}
+
+bool BsdiffArguments::ParseBsdiffFormat(const string& str,
+                                        BsdiffFormat* format) {
+  string format_string = str;
+  std::transform(format_string.begin(), format_string.end(),
+                 format_string.begin(), ::tolower);
+  if (format_string == kLegacyString || format_string == kBsdiff40String) {
+    *format = BsdiffFormat::kLegacy;
+    return true;
+  } else if (format_string == kBsdf2String) {
+    *format = BsdiffFormat::kBsdf2;
+    return true;
+  }
+  std::cerr << "Failed to parse bsdiff format in " << str << endl;
+  return false;
+}
+
+bool BsdiffArguments::ParseQuality(const string& str, int* quality) {
+  errno = 0;
+  char* end;
+  const char* s = str.c_str();
+  long result = strtol(s, &end, 10);
+  if (errno != 0 || s == end || *end != '\0') {
+    return false;
+  }
+
+  if (result < BROTLI_MIN_QUALITY || result > BROTLI_MAX_QUALITY) {
+    std::cerr << "Compression quality out of range " << str << endl;
+    return false;
+  }
+
+  *quality = result;
+  return true;
+}
+
+}  // namespace bsdiff

bsdiff_arguments.h

@@ -0,0 +1,69 @@
+// Copyright 2017 The Chromium OS Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef _BSDIFF_BSDIFF_ARGUMENTS_H_
+#define _BSDIFF_BSDIFF_ARGUMENTS_H_
+
+#include <stdint.h>
+
+#include <string>
+
+#include "bsdiff/constants.h"
+#include "bsdiff/patch_writer_interface.h"
+
+using std::string;
+
+namespace bsdiff {
+
+// Class to store the patch writer options about format, type and quality.
+class BsdiffArguments {
+ public:
+  BsdiffArguments()
+      : format_(BsdiffFormat::kLegacy),
+        compressor_type_(CompressorType::kBZ2),
+        compression_quality_(-1) {}
+
+  BsdiffArguments(BsdiffFormat format, CompressorType type, int quality)
+      : format_(format),
+        compressor_type_(type),
+        compression_quality_(quality) {}
+
+  // Check if the compressor type is compatible with the bsdiff format.
+  bool IsValid() const;
+
+  // Getter functions.
+  BsdiffFormat format() const { return format_; }
+
+  CompressorType compressor_type() const { return compressor_type_; }
+
+  int compression_quality() const { return compression_quality_; }
+
+  // Parse the command line arguments of the main function and set all the
+  // fields accordingly.
+  bool ParseCommandLine(int argc, char** argv);
+
+  // Parse the compression type from string.
+  static bool ParseCompressorType(const string& str, CompressorType* type);
+
+  // Parse the bsdiff format from string.
+  static bool ParseBsdiffFormat(const string& str, BsdiffFormat* format);
+
+  // Parse the compression quality (for brotli) from string.
+  static bool ParseQuality(const string& str, int* quality);
+
+ private:
+  // Current format supported are the legacy "BSDIFF40" or "BSDF2".
+  BsdiffFormat format_;
+
+  // 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_;
+};
+
+}  // namespace bsdiff
+
+#endif  //_BSDIFF_BSDIFF_ARGUMENTS_H_

bsdiff_arguments_unittest.cc

@@ -0,0 +1,80 @@
+// Copyright 2017 The Chromium OS Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "bsdiff_arguments.h"
+
+#include <vector>
+
+#include <gtest/gtest.h>
+
+namespace bsdiff {
+
+TEST(BsdiffArgumentsTest, ParseCompressorTypeTest) {
+  CompressorType type;
+  EXPECT_TRUE(BsdiffArguments::ParseCompressorType("Brotli", &type));
+  EXPECT_EQ(CompressorType::kBrotli, type);
+
+  EXPECT_TRUE(BsdiffArguments::ParseCompressorType("bz2", &type));
+  EXPECT_EQ(CompressorType::kBZ2, type);
+
+  EXPECT_FALSE(BsdiffArguments::ParseCompressorType("invalid", &type));
+}
+
+TEST(BsdiffArgumentsTest, ParseBsdiffFormatTest) {
+  BsdiffFormat format;
+  EXPECT_TRUE(BsdiffArguments::ParseBsdiffFormat("bsdf2", &format));
+  EXPECT_EQ(BsdiffFormat::kBsdf2, format);
+
+  EXPECT_TRUE(BsdiffArguments::ParseBsdiffFormat("Legacy", &format));
+  EXPECT_EQ(BsdiffFormat::kLegacy, format);
+
+  EXPECT_TRUE(BsdiffArguments::ParseBsdiffFormat("bsdiff40", &format));
+  EXPECT_EQ(BsdiffFormat::kLegacy, format);
+
+  EXPECT_FALSE(BsdiffArguments::ParseBsdiffFormat("Other", &format));
+}
+
+TEST(BsdiffArgumentsTest, ParseQualityTest) {
+  int quality;
+  EXPECT_TRUE(BsdiffArguments::ParseQuality("9", &quality));
+  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));
+}
+
+TEST(BsdiffArgumentsTest, ArgumentsValidTest) {
+  // Default arguments using BsdiffFormat::kLegacy and CompressorType::kBZ2
+  // should be valid.
+  EXPECT_TRUE(BsdiffArguments().IsValid());
+
+  // brotli is not supported for BsdiffFormat::kLegacy.
+  EXPECT_FALSE(
+      BsdiffArguments(BsdiffFormat::kLegacy, CompressorType::kBrotli, -1)
+          .IsValid());
+
+  EXPECT_TRUE(BsdiffArguments(BsdiffFormat::kBsdf2, CompressorType::kBrotli, 9)
+                  .IsValid());
+
+  // Compression quality out of range for brotli.
+  EXPECT_FALSE(
+      BsdiffArguments(BsdiffFormat::kBsdf2, CompressorType::kBrotli, 20)
+          .IsValid());
+}
+
+TEST(BsdiffArgumentsTest, ParseArgumentsSmokeTest) {
+  std::vector<const char*> args = {"bsdiff", "--format=bsdf2", "--type=brotli",
+                                   "--quality=9"};
+
+  BsdiffArguments arguments;
+  EXPECT_TRUE(arguments.ParseCommandLine(4, const_cast<char**>(args.data())));
+
+  EXPECT_EQ(BsdiffFormat::kBsdf2, arguments.format());
+  EXPECT_EQ(CompressorType::kBrotli, arguments.compressor_type());
+  EXPECT_EQ(9, arguments.compression_quality());
+}
+
+}  // namespace bsdiff

bsdiff_main.cc

@@ -4,6 +4,7 @@
 
 #include <err.h>
 #include <fcntl.h>
+#include <getopt.h>
 #include <sys/mman.h>
 #include <sys/stat.h>
 #include <sys/types.h>
@@ -13,9 +14,12 @@
 #include <stdio.h>
 #include <stdlib.h>
 
+#include <iostream>
 #include <limits>
 
 #include "bsdiff/bsdiff.h"
+#include "bsdiff/bsdiff_arguments.h"
+#include "bsdiff/constants.h"
 #include "bsdiff/patch_writer_factory.h"
 
 namespace {
@@ -50,38 +54,69 @@ void* MapFile(const char* filename, size_t* filesize) {
 }
 
 // Generate bsdiff patch from the |old_filename| file to the |new_filename|
-// file storing the resulting patch in a new |patch_filename| file.
+// file with options in |arguments|. Store the resulting patch in a new
+// |patch_filename| file. Returns 0 on success.
 int GenerateBsdiffFromFiles(const char* old_filename,
                             const char* new_filename,
-                            const char* patch_filename) {
-  size_t oldsize, newsize;
-  int ret = 0;
-
+                            const char* patch_filename,
+                            const bsdiff::BsdiffArguments& arguments) {
+  size_t oldsize;
   uint8_t* old_buf = static_cast<uint8_t*>(MapFile(old_filename, &oldsize));
-  uint8_t* new_buf = static_cast<uint8_t*>(MapFile(new_filename, &newsize));
-
-  if (old_buf && new_buf) {
-    auto patch_writer = bsdiff::CreateBsdiffPatchWriter(patch_filename);
-
-    ret = bsdiff::bsdiff(old_buf, oldsize, new_buf, newsize, patch_writer.get(),
-                         nullptr);
-  } else {
-    ret = 1;
+  if (!old_buf) {
+    return 1;
   }
 
-  if (old_buf)
+  size_t newsize;
+  uint8_t* new_buf = static_cast<uint8_t*>(MapFile(new_filename, &newsize));
+  if (!new_buf) {
     munmap(old_buf, oldsize);
-  if (new_buf)
-    munmap(new_buf, newsize);
+    return 1;
+  }
 
-  return ret;
+  std::unique_ptr<bsdiff::PatchWriterInterface> patch_writer;
+  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());
+  } else {
+    std::cerr << "unexpected bsdiff format." << std::endl;
+    return 1;
+  }
+
+  return bsdiff::bsdiff(old_buf, oldsize, new_buf, newsize, patch_writer.get(),
+                        nullptr);
+}
+
+void PrintUsage(const std::string& proc_name) {
+  std::cerr << "usage: " << proc_name
+            << " [options] oldfile newfile patchfile\n";
+  std::cerr << "  --format <legacy|bsdiff40|bsdf2>  The format of the bsdiff"
+               " patch.\n"
+            << "  --type <bz2|brotli>      The algorithm to compress the "
+               "patch, bsdf2 format only.\n"
+            << "  --quality                Quality of the patch compression,"
+               " brotli only.\n";
 }
 
 }  // namespace
 
 int main(int argc, char* argv[]) {
-  if (argc != 4)
-    errx(1, "usage: %s oldfile newfile patchfile\n", argv[0]);
+  bsdiff::BsdiffArguments arguments;
 
-  return GenerateBsdiffFromFiles(argv[1], argv[2], argv[3]);
+  if (!arguments.ParseCommandLine(argc, argv)) {
+    PrintUsage(argv[0]);
+    return 1;
+  }
+
+  // The optind will be updated in ParseCommandLine to parse the options; and
+  // we expect the rest of the arguments to be oldfile, newfile, patchfile.
+  if (!arguments.IsValid() || argc - optind != 3) {
+    PrintUsage(argv[0]);
+    return 1;
+  }
+
+  return GenerateBsdiffFromFiles(argv[optind], argv[optind + 1],
+                                 argv[optind + 2], arguments);
 }

compressor_interface.cc

@@ -1,26 +0,0 @@
-// Copyright 2017 The Chromium OS Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-#include "bsdiff/compressor_interface.h"
-
-#include "bsdiff/brotli_compressor.h"
-#include "bsdiff/bz2_compressor.h"
-#include "bsdiff/logging.h"
-
-namespace bsdiff {
-
-std::unique_ptr<CompressorInterface> CreateCompressor(CompressorType type) {
-  switch (type) {
-    case CompressorType::kBZ2:
-      return std::unique_ptr<CompressorInterface>(new BZ2Compressor());
-    case CompressorType::kBrotli:
-      return std::unique_ptr<CompressorInterface>(new BrotliCompressor());
-    default:
-      LOG(ERROR) << "unsupported compressor type: "
-                 << static_cast<uint8_t>(type);
-      return nullptr;
-  }
-}
-
-}  // namespace bsdiff

compressor_interface.h

@@ -35,8 +35,6 @@ class CompressorInterface {
   CompressorInterface() = default;
 };
 
-std::unique_ptr<CompressorInterface> CreateCompressor(CompressorType type);
-
 }  // namespace bsdiff
 
 #endif  // _BSDIFF_COMPRESSOR_INTERFACE_H_

include/bsdiff/patch_writer_factory.h

@@ -10,6 +10,7 @@
 #include <vector>
 
 #include "bsdiff/common.h"
+#include "bsdiff/constants.h"
 #include "bsdiff/patch_writer_interface.h"
 
 namespace bsdiff {
@@ -20,6 +21,14 @@ BSDIFF_EXPORT
 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|.
+BSDIFF_EXPORT
+std::unique_ptr<PatchWriterInterface> CreateBSDF2PatchWriter(
+    const std::string& patch_filename,
+    CompressorType type,
+    int quality);
+
 // Create a patch writer compatible with Android Play Store bsdiff patches,
 // uncompressed. The data will be written to the passed |patch| vector, which
 // must be valid until Close() is called or this patch is destroyed.

patch_reader_unittest.cc

@@ -125,8 +125,8 @@ TEST_F(PatchReaderTest, PatchReaderLegacyFormatSmoke) {
 TEST_F(PatchReaderTest, PatchReaderNewFormatSmoke) {
   // Compress the data with one bz2 and two brotli compressors.
   ctrl_stream_.reset(new BZ2Compressor());
-  diff_stream_.reset(new BrotliCompressor());
-  extra_stream_.reset(new BrotliCompressor());
+  diff_stream_.reset(new BrotliCompressor(11));
+  extra_stream_.reset(new BrotliCompressor(11));
 
   CompressData();
 

patch_writer.cc

@@ -26,19 +26,25 @@ void EncodeInt64(int64_t x, uint8_t* buf) {
 
 namespace bsdiff {
 
+BsdiffPatchWriter::BsdiffPatchWriter(const std::string& patch_filename)
+    : patch_filename_(patch_filename), format_(BsdiffFormat::kLegacy) {
+  ctrl_stream_.reset(new BZ2Compressor());
+  diff_stream_.reset(new BZ2Compressor());
+  extra_stream_.reset(new BZ2Compressor());
+}
+
 BsdiffPatchWriter::BsdiffPatchWriter(const std::string& patch_filename,
-                                     BsdiffFormat format)
-    : patch_filename_(patch_filename), format_(format) {
-  if (format_ == BsdiffFormat::kLegacy) {
-    ctrl_stream_ = CreateCompressor(CompressorType::kBZ2);
-    diff_stream_ = CreateCompressor(CompressorType::kBZ2);
-    extra_stream_ = CreateCompressor(CompressorType::kBZ2);
-  } else {
-    // TODO(xunchang) set different compression method according to the
-    // compressed size for these streams.
-    ctrl_stream_ = CreateCompressor(CompressorType::kBrotli);
-    diff_stream_ = CreateCompressor(CompressorType::kBrotli);
-    extra_stream_ = CreateCompressor(CompressorType::kBrotli);
+                                     CompressorType type,
+                                     int 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));
   }
 }
 

patch_writer.h

@@ -14,13 +14,20 @@
 
 namespace bsdiff {
 
-// A PatchWriterInterface class using the upstream's BSDIFF40 format: three
-// BZ2-compressors and a 32-byte header.
+// A PatchWriterInterface class with three compressors and a 32-byte header.
 class BsdiffPatchWriter : public PatchWriterInterface {
  public:
-  // Create the patch writer using |type| as the compression algorithm and the
-  // file |patch_filename| to write the patch data.
-  BsdiffPatchWriter(const std::string& patch_filename, BsdiffFormat format);
+  // Create the patch writer using the upstream's "BSDIFF40" format. It uses
+  // bz2 as the compression algorithm and the file |patch_filename| to write
+  // the patch data.
+  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|.
+  BsdiffPatchWriter(const std::string& patch_filename,
+                    CompressorType type,
+                    int quality);
 
   // PatchWriterInterface overrides.
   bool Init(size_t new_size) override;

patch_writer_factory.cc

@@ -9,11 +9,18 @@
 
 namespace bsdiff {
 
-// TODO(xunchang) choose the bsdiff format based on the input parameter.
 std::unique_ptr<PatchWriterInterface> CreateBsdiffPatchWriter(
     const std::string& patch_filename) {
   return std::unique_ptr<PatchWriterInterface>(
-      new BsdiffPatchWriter(patch_filename, BsdiffFormat::kLegacy));
+      new BsdiffPatchWriter(patch_filename));
+}
+
+std::unique_ptr<PatchWriterInterface> CreateBSDF2PatchWriter(
+    const std::string& patch_filename,
+    CompressorType type,
+    int quality) {
+  return std::unique_ptr<PatchWriterInterface>(
+      new BsdiffPatchWriter(patch_filename, type, quality));
 }
 
 std::unique_ptr<PatchWriterInterface> CreateEndsleyPatchWriter(

patch_writer_unittest.cc

@@ -53,8 +53,7 @@ class BsdiffPatchWriterTest : public testing::Test {
   }
 
   test_utils::ScopedTempFile patch_file_{"bsdiff_newfile.XXXXXX"};
-  BsdiffPatchWriter patch_writer_{patch_file_.filename(),
-                                  BsdiffFormat::kLegacy};
+  BsdiffPatchWriter patch_writer_{patch_file_.filename()};
 };
 
 TEST_F(BsdiffPatchWriterTest, CreateEmptyPatchTest) {

utils.cc

@@ -1,10 +1,7 @@
-#include <stdint.h>
-
 #include "bsdiff/utils.h"
 
 namespace bsdiff {
 
-
 int64_t ParseInt64(const uint8_t* buf) {
   int64_t result = buf[7] & 0x7F;
   for (int i = 6; i >= 0; i--) {
@@ -17,4 +14,4 @@ int64_t ParseInt64(const uint8_t* buf) {
   return result;
 }
 
-}  // namespace bsdiff
+}  // namespace bsdiff