Motown in-proc streaming framework used to implement media services.

services/media/framework/models/lpcm_frames.h

Index: services/media/framework/models/lpcm_frames.h
diff --git a/services/media/framework/models/lpcm_frames.h b/services/media/framework/models/lpcm_frames.h
new file mode 100644
index 0000000000000000000000000000000000000000..120e331c966dd2ca19d1df138dfb04d7039b56c8
--- /dev/null
+++ b/services/media/framework/models/lpcm_frames.h
@@ -0,0 +1,90 @@
+// 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.
+
+#ifndef MOJO_MEDIA_MODELS_LPCM_FRAMES_H_
+#define MOJO_MEDIA_MODELS_LPCM_FRAMES_H_
+
+#include <cstdint>
+
+#include "base/logging.h"
+#include "services/media/framework/packet.h"
+
+namespace mojo {
+namespace media {
+
+// References an LPCM frame buffer and implements advancement through it.
+class LpcmFrames {
+ public:
+  LpcmFrames() :
+      bytes_per_frame_(0),
+      remaining_buffer_(nullptr),
+      remaining_frame_count_(0) {}
+
+  void set_bytes_per_frame(uint32_t bytes_per_frame) {
+    bytes_per_frame_ = bytes_per_frame;
+  }
+
+  uint32_t bytes_per_frame() {

[johngro, 2016/01/26 01:32:39]

this method is const.  Same goes for the other accessors below (buffer and
frame_count)

[dalesat, 2016/01/26 21:17:51]

Good catch. Still not in the habit of looking for these.

+    return bytes_per_frame_;
+  }
+
+  // The remaining frame buffer.
+  void* buffer() {
+    return remaining_buffer_;
+  }
+
+  // The remaining number of frames accommodated by the frame buffer.
+  uint64_t frame_count() {
+    return remaining_frame_count_;
+  }
+
+  // Resets the buffer and frame.
+  void reset() {
+    remaining_buffer_ = nullptr;
+    remaining_frame_count_ = 0;
+    reset_when_exhausted_ = nullptr;
+  }
+
+  // Sets the buffer and frame count.
+  void set(void* buffer, uint64_t frame_count) {
+    remaining_buffer_ = buffer;
+    remaining_frame_count_ = frame_count;
+    reset_when_exhausted_ = nullptr;
+  }
+
+  void set(PacketPtr& packet) {
+    DCHECK(packet);
+    remaining_buffer_ = packet->payload();
+    remaining_frame_count_ = packet->duration();
+    if (remaining_frame_count_ == 0) {
+      packet.reset();
+      reset_when_exhausted_ = nullptr;
+    } else {
+      reset_when_exhausted_ = &packet;
+    }
+  }
+
+  // Updates buffer and frame_count to reflect use of the buffer.
+  void advance(uint64_t frame_count) {
+    DCHECK(remaining_buffer_);
+    DCHECK(frame_count <= remaining_frame_count_);
+    remaining_buffer_ = reinterpret_cast<uint8_t*>(remaining_buffer_) +
+        (frame_count * bytes_per_frame_);
+    remaining_frame_count_ -= frame_count;
+    if (remaining_frame_count_ == 0 && reset_when_exhausted_ != nullptr) {
+      (*reset_when_exhausted_).reset();
+    }
+  }
+
+ private:
+  uint32_t bytes_per_frame_;
+  void* remaining_buffer_;
+  uint64_t remaining_frame_count_;
+  PacketPtr* reset_when_exhausted_;

[johngro, 2016/01/26 01:32:39]

I'm unclear on why we need reset_when_exhausted_ here.

From what I can tell, PacketPtr is a std::unique_ptr with a non-standard
deleter.  When a PacketPtr instance is reset (or has nullptr assigned to it),
the non-standard deleter will be called, and presumably the packet memory will
be returned to the allocator and the packet object itself perhaps placed on a
free list somewhere.

Why not just have a PacketPtr member which does this job, instead of a pointer
to one?  The LpcmFrames instance has ownership of the Packet, and when
remaining_frame_count hits zero, you can just packet_ptr_.reset().  You get the
same behavior in the destructor for free.

Doing it like this seems to allow the LpcmFrames object to manipulate the
bookkeeping of its user which seems odd.  It also introduces a lifetime
management issue; how do we know that our pointer to our PacketPtr (IOW - our
pointer to our user's bookkeeping) is going to have a lifetime <= the PacketPtr
instance itself?  What prevents someone on the outside from resetting our
PacketPtr (presumably freeing the memory which backs it) and leaving us with a
stale buffer pointer (in the form of a non-null remaining_buffer_)

[dalesat, 2016/01/26 21:17:51]

This feature is used in LpcmStageInput, and the hazards you mention are avoided.
I take your point that this use of PacketPtr* constitutes an anti-pattern. I'll
address this by removing all references to PacketPtr here (making this class
more independent) and providing a callback mechanism so LpcmStageInput and other
callers can take action when the frames are exhausted as needed.

[johngro, 2016/01/27 22:35:22]

Acknowledged.

+};
+
+}  // namespace media
+}  // namespace mojo
+
+#endif // MOJO_MEDIA_MODELS_LPCM_FRAMES_H_