[Mojo Video Capture] Split OnIncomingCapturedVideoFrame() to OnNewBuffer() and OnFrameReadyInBuffer(

media/capture/video/video_capture_device.h

 // Copyright (c) 2012 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.
 //
 // VideoCaptureDevice is the abstract base class for realizing video capture
 // device support in Chromium. It provides the interface for OS dependent
 // implementations.
 // The class is created and functions are invoked on a thread owned by
 // VideoCaptureManager. Capturing is done on other threads, depending on the OS
 // specific implementation.
@@ skipping 23 matching lines @@
 #include "media/capture/video/video_capture_device_descriptor.h"
 #include "media/capture/video_capture_types.h"
 #include "ui/gfx/gpu_memory_buffer.h"
 
 namespace tracked_objects {
 class Location;
 }  // namespace tracked_objects
 
 namespace media {
 
-class CAPTURE_EXPORT FrameBufferPool {
- public:
-  virtual ~FrameBufferPool() {}
-
-  virtual void SetBufferHold(int buffer_id) = 0;
-  virtual void ReleaseBufferHold(int buffer_id) = 0;
-};
-
 class CAPTURE_EXPORT VideoFrameConsumerFeedbackObserver {
  public:
   virtual ~VideoFrameConsumerFeedbackObserver() {}
 
   // During processing of a video frame, consumers may report back their
   // utilization level to the source device. The device may use this information
   // to adjust the rate of data it pushes out. Values are interpreted as
   // follows:
   // Less than 0.0 is meaningless and should be ignored.  1.0 indicates a
   // maximum sustainable utilization.  Greater than 1.0 indicates the consumer
@@ skipping 48 matching lines @@
 
       Buffer();
       Buffer(int buffer_id,
              int frame_feedback_id,
              std::unique_ptr<HandleProvider> handle_provider,
              std::unique_ptr<ScopedAccessPermission> access_permission);
       ~Buffer();
       Buffer(Buffer&& other);
       Buffer& operator=(Buffer&& other);
 
-      bool is_valid() const { return handle_provider_ != nullptr; }
+      bool is_valid() const { return handle_provider != nullptr; }
       int id() const { return id_; }
       int frame_feedback_id() const { return frame_feedback_id_; }
-      HandleProvider* handle_provider() const { return handle_provider_.get(); }
+
+      std::unique_ptr<HandleProvider> handle_provider;

[miu, 2017/02/14 00:41:27]

style: Classes shouldn't have public-access data members. Perhaps you could add
a TakeHandleProvider() method to transfer ownership from this class to the new
owner?

Also, since the ownership can be transferred, does that mean all other
codepoints accessing this object need to include null checks?

[chfremer, 2017/02/14 21:15:16]

I still feel this should be a plain struct. This was harder to argue for in the
previous CL, since it was passed around more and the ownership of the unique_ptr
members was never moved out. But with this CL this is really only used to bundle
input/output parameters in the communication between VideoCaptureDevice and its
Client.

I am proposing to make this a struct.

+      std::unique_ptr<ScopedAccessPermission> access_permission;

[miu, 2017/02/14 00:41:27]

ditto

[chfremer, 2017/02/14 21:15:16]

Acknowledged.

 
      private:
-      std::unique_ptr<HandleProvider> handle_provider_;
-      std::unique_ptr<ScopedAccessPermission> access_permission_;
       int id_;
       int frame_feedback_id_;
 
       DISALLOW_COPY_AND_ASSIGN(Buffer);
     };
 
     virtual ~Client() {}
 
     // Captured a new video frame, data for which is pointed to by |data|.
     //
@@ skipping 164 matching lines @@
 
  private:
   // Gets the power line frequency from the current system time zone if this is
   // defined, otherwise returns 0.
   PowerLineFrequency GetPowerLineFrequencyForLocation() const;
 };
 
 }  // namespace media
 
 #endif  // MEDIA_CAPTURE_VIDEO_VIDEO_CAPTURE_DEVICE_H_