Simple PMP reader to parse Picasa's metadata

webkit/fileapi/media/picasa/pmp_column_reader.cc

Index: webkit/fileapi/media/picasa/pmp_column_reader.cc
diff --git a/webkit/fileapi/media/picasa/pmp_column_reader.cc b/webkit/fileapi/media/picasa/pmp_column_reader.cc
new file mode 100644
index 0000000000000000000000000000000000000000..82a82625eb28035d596d9920b8081f4cc2e75cff
--- /dev/null
+++ b/webkit/fileapi/media/picasa/pmp_column_reader.cc
@@ -0,0 +1,211 @@
+// Copyright (c) 2013 The Chromium 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 "webkit/fileapi/media/picasa/pmp_column_reader.h"
+
+#include <cstring>
+
+#include "base/files/file_path.h"
+#include "base/files/memory_mapped_file.h"
+#include "base/logging.h"
+#include "webkit/fileapi/media/picasa/pmp_constants.h"
+
+namespace fileapi {
+
+PmpColumnReader::PmpColumnReader(std::string column_name)
+    : column_name_(column_name),
+      data_(NULL),
+      length_(0),
+      rows_(0),
+      strings_(),
+      fieldsize_(0) {
+

[vandebo (ex-Chrome), 2013/03/26 01:12:14]

nit: extra line

[tommycli, 2013/03/26 22:30:43]

Done.

+}
+
+PmpColumnReader::~PmpColumnReader() {
+  delete[] data_;

[vandebo (ex-Chrome), 2013/03/26 01:12:14]

This means you probably want to use a scoped object as the data member instead. 
Maybe scoped_array<T> in base/memory/scoped_ptr.h.

[tommycli, 2013/03/26 22:30:43]

Done.

+}
+
+bool PmpColumnReader::ReadFromDisk(uint32* rows_read,
+                                   const base::FilePath& filepath) {
+  base::MemoryMappedFile file;
+  bool file_initialized = file.Initialize(filepath);
+
+  // If file is not initialized, too small, or too large (50MB), fail out.
+  if (!file_initialized ||
+      file.length() < kPmpHeaderSize ||
+      file.length() > 50*1024*1024) {

[vandebo (ex-Chrome), 2013/03/26 01:12:14]

Make this a constant as well.

[tommycli, 2013/03/26 22:30:43]

Done.

+    return false;
+  }
+
+  length_ = file.length();
+  if(data_ != NULL) {
+    delete[] data_;
+  }
+  data_ = new uint8[length_];
+  memcpy(data_, file.data(), length_);
+
+  return ParseData(rows_read);
+}
+
+bool PmpColumnReader::ReadFromMemory(uint32* rows_read, const uint8* data,
+                                     const size_t len) {
+  if(len < kPmpHeaderSize || data == NULL) {
+    return false;
+  }
+
+  length_ = len;
+  if(data_ != NULL) {
+    delete[] data_;
+  }
+  data_ = new uint8[length_];
+  memcpy(data_, data, length_);
+
+  return ParseData(rows_read);
+}
+
+template<class T>
+bool PmpColumnReader::Read(uint32 row, T* target) {
+  DCHECK(data_ != NULL && length_ > kPmpHeaderSize);
+
+  if(sizeof(T) != fieldsize_) {
+    return false;
+  }
+
+  if(row < rows_) {
+    memcpy(target, data_ + kPmpHeaderSize + row*fieldsize_, fieldsize_);

[vandebo (ex-Chrome), 2013/03/26 01:12:14]

You probably want an explicit reinterpret_cast in here to make it clear what's
going on instead of hiding it in the memcpy.
http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml#Casting

[tommycli, 2013/03/26 22:30:43]

Done.

+    return true;
+  } else {
+    return false;
+  }
+}
+
+template bool PmpColumnReader::Read<uint8>(uint32 row, uint8* target);
+
+template bool PmpColumnReader::Read<uint16>(uint32 row, uint16* target);
+
+template bool PmpColumnReader::Read<uint32>(uint32 row, uint32* target);
+
+template bool PmpColumnReader::Read<uint64>(uint32 row, uint64* target);
+
+template bool PmpColumnReader::Read<double>(uint32 row, double* target);
+
+template<>
+bool PmpColumnReader::Read(uint32 row, std::string* target) {
+  DCHECK(data_ != NULL && length_ > kPmpHeaderSize);
+
+  if(row < rows_) {
+    *target = strings_[row];
+    return true;
+  } else {
+    return false;
+  }
+}
+
+bool PmpColumnReader::ParseData(uint32* rows_read) {
+  DCHECK(data_ != NULL);
+  DCHECK(length_ >= kPmpHeaderSize);
+
+  // Check all magic bytes
+  if (memcmp(kPmpMagicO0S4, data_, 4) != 0||

[vandebo (ex-Chrome), 2013/03/26 01:12:14]

You may want to make a packed struct of the different header fields.

[tommycli, 2013/03/26 22:30:43]

That's a good idea. I may need some help figuring out how to do this in a
Chromium/x-platform way, since the unittests are currently run on every
platform.

+      memcmp(kPmpMagicO6S2, data_ + 6, 2) != 0 ||
+      memcmp(kPmpMagicO8S4, data_ + 8, 4) != 0 ||
+      memcmp(kPmpMagicO14S2, data_ + 14, 2) != 0) {
+    return false;
+  }
+
+  // Verify row field type matches
+  if (data_[kPmpFieldTypeFirstOffset] != data_[kPmpFieldTypeSecondOffset]) {
+    return false;
+  }
+
+  // Read the field type and number of rows expected
+  uint16 field_type = data_[kPmpFieldTypeFirstOffset];
+  memcpy(&rows_, &data_[kPmpRowCountOffset], sizeof(uint32));
+
+  // Set the number of rows read. Though it's not valid unless we return true.
+  if(rows_read != NULL) {
+    *rows_read = rows_;
+  }
+
+  switch (field_type) {
+    case 0x00:

[vandebo (ex-Chrome), 2013/03/26 01:12:14]

Make these constants.

[tommycli, 2013/03/26 22:30:43]

Done.

+      return IndexStrings();
+      break;
+    case 0x01:
+      return SetAndValidateFieldsize<uint32>();
+      break;
+    case 0x02:
+      return SetAndValidateFieldsize<double>();
+      break;
+    case 0x03:
+      return SetAndValidateFieldsize<uint8>();
+      break;
+    case 0x04:
+      return SetAndValidateFieldsize<uint64>();
+      break;
+    case 0x05:
+      return SetAndValidateFieldsize<uint16>();
+      break;
+    case 0x06:
+      return IndexStrings();
+      break;
+    case 0x07:
+      return SetAndValidateFieldsize<uint32>();
+      break;
+    default:
+      return false;
+      break;
+  }
+}
+
+bool PmpColumnReader::IndexStrings() {
+  DCHECK(data_ != NULL);
+  DCHECK(length_ >= kPmpHeaderSize);
+
+  strings_.clear();
+  strings_.reserve(rows_);
+
+  size_t bytes_parsed = kPmpHeaderSize;
+  const uint8* itr = data_ + kPmpHeaderSize;

[vandebo (ex-Chrome), 2013/03/26 01:12:14]

How is the string encoded? UTF8? unspecified?

[tommycli, 2013/03/26 22:30:43]

Unspecified. I've sent an email to the Picasa ppl asking though.

+
+  while (strings_.size() < rows_) {
+    const uint8* ptr_to_str_end = static_cast<const uint8*>(
+        memchr(itr, '\0', length_-bytes_parsed));

[vandebo (ex-Chrome), 2013/03/26 01:12:14]

spaces around math operators.

[tommycli, 2013/03/26 22:30:43]

Done.

+
+    // fail if cannot find null termination. String runs on past file end.

[vandebo (ex-Chrome), 2013/03/26 01:12:14]

Capitalize

[tommycli, 2013/03/26 22:30:43]

Done.

+    if(ptr_to_str_end == NULL) {

[vandebo (ex-Chrome), 2013/03/26 01:12:14]

{}'s around single line conditionals with single line bodies are optional. 
Precedent is set by other instances in the file or the directory. 
webkit/fileapi doesn't seem to use them.

[tommycli, 2013/03/26 22:30:43]

Done.

+      return false;
+    }
+
+    // Length of string. (+1 to include the termination character)
+    ptrdiff_t len_bytes = ptr_to_str_end - itr + 1;
+
+    strings_.push_back(reinterpret_cast<const char*>(itr));
+    itr += len_bytes;
+    bytes_parsed += len_bytes;
+  }
+
+  // This means the file has more bytes at the end we haven't parsed
+  if(bytes_parsed != length_) {
+    return false;
+  }
+
+  return true;
+}
+
+template<class T>
+bool PmpColumnReader::SetAndValidateFieldsize() {
+  unsigned long calculated_n_rows =
+      (length_ - kPmpHeaderSize)/sizeof(T);
+
+  if(calculated_n_rows == rows_) {
+    fieldsize_ = sizeof(T);
+    return true;
+  } else {

[vandebo (ex-Chrome), 2013/03/26 01:12:14]

No else in this case.

[tommycli, 2013/03/26 22:30:43]

Done.

+    return false;
+  }
+}
+
+}  // namespace