Add offset support to BitstreamBuffer.

content/common/gpu/gpu_messages.cc

 // Copyright 2016 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 <stddef.h>
 
 #include <sstream>
 
 #include "base/logging.h"
 #include "base/numerics/safe_math.h"
 #include "content/common/gpu/gpu_messages.h"
 
 namespace IPC {
 
 void ParamTraits<media::BitstreamBuffer>::Write(base::Pickle* m,
                                                 const param_type& p) {
   WriteParam(m, p.id());
   WriteParam(m, static_cast<uint64_t>(p.size()));
+  WriteParam(m, static_cast<int64_t>(p.offset()));

[dcheng, 2016/03/01 01:48:43]

Does it make sense to DCHECK() that this is non-negative?

[Owen Lin, 2016/03/02 02:50:52]

I agree with Pawel that
"MapAt() will fail us if the offset is invalid, e.g. if we pass a negative value
for some reason. So I don't think we have to check for <0 explicitly."

[dcheng, 2016/03/02 05:58:20]

It's not in the contract of SharedMemory::MapAt(), and it's not actually obvious
that it will fail. In fact, here's the Windows implementation:

bool SharedMemory::MapAt(off_t offset, size_t bytes) {
  ...
  memory_ = MapViewOfFile(
      mapped_file_, read_only_ ? FILE_MAP_READ : FILE_MAP_READ | FILE_MAP_WRITE,
      static_cast<uint64_t>(offset) >> 32, static_cast<DWORD>(offset), bytes);
  ...
}

Will this fail or succeed with off_t = -1? We cast it to unsigned before we
shift, so we won't get sign extension...

   WriteParam(m, p.presentation_timestamp());
   WriteParam(m, p.key_id());
   if (!p.key_id().empty()) {
     WriteParam(m, p.iv());
     WriteParam(m, p.subsamples());
   }
   WriteParam(m, p.handle());
 }
 
 bool ParamTraits<media::BitstreamBuffer>::Read(const base::Pickle* m,
                                                base::PickleIterator* iter,
                                                param_type* r) {
   DCHECK(r);
   uint64_t size = 0;
+  int64_t offset = 0;
   if (!(ReadParam(m, iter, &r->id_) && ReadParam(m, iter, &size) &&
+        ReadParam(m, iter, &offset) &&
         ReadParam(m, iter, &r->presentation_timestamp_) &&
         ReadParam(m, iter, &r->key_id_)))
     return false;
 
   base::CheckedNumeric<size_t> checked_size(size);
   if (!checked_size.IsValid()) {
     DLOG(ERROR) << "Invalid size: " << size;
     return false;
   }
   r->size_ = checked_size.ValueOrDie();
 
+  base::CheckedNumeric<off_t> checked_offset(offset);
+  if (!checked_offset.IsValid()) {

[dcheng, 2016/03/01 01:48:43]

Similarly, does it make sense to assert that the value is non-negative?

[Owen Lin, 2016/03/02 02:50:52]

See above.

+    DLOG(ERROR) << "Invalid offset: " << offset;
+    return false;
+  }
+  r->offset_ = checked_offset.ValueOrDie();
+
   if (!r->key_id_.empty()) {
     if (!(ReadParam(m, iter, &r->iv_) && ReadParam(m, iter, &r->subsamples_)))
       return false;
   }
 
   return ReadParam(m, iter, &r->handle_);
 }
 
 void ParamTraits<media::BitstreamBuffer>::Log(const param_type& p,
                                               std::string* l) {
   std::ostringstream oss;
   oss << "id=" << p.id() << ", size=" << p.size() << ", presentation_timestamp="
       << p.presentation_timestamp().ToInternalValue();
   l->append(oss.str());
 }
 
 }  // namespace IPC