Isolate register post encode callback in video coding module to simplify code and critical sections.

R=marpan@webrtc.org

Review URL: https://webrtc-codereview.appspot.com/6659004

git-svn-id: http://webrtc.googlecode.com/svn/trunk@5357 4adac7df-926f-26a2-2b94-8c16560cd09d

webrtc/modules/video_coding/main/source/generic_encoder.cc

@@ -142,15 +142,15 @@ VCMGenericEncoder::InternalSource() const
  /***************************
   * Callback Implementation
   ***************************/
-VCMEncodedFrameCallback::VCMEncodedFrameCallback():
+VCMEncodedFrameCallback::VCMEncodedFrameCallback(
+    EncodedImageCallback* post_encode_callback):
 _sendCallback(),
 _mediaOpt(NULL),
 _encodedBytes(0),
 _payloadType(0),
 _codecType(kVideoCodecUnknown),
 _internalSource(false),
-post_encode_callback_lock_(CriticalSectionWrapper::CreateCriticalSection()),
-post_encode_callback_(NULL)
+post_encode_callback_(post_encode_callback)
 #ifdef DEBUG_ENCODER_BIT_STREAM
 , _bitStreamAfterEncoder(NULL)
 #endif
@@ -180,12 +180,8 @@ VCMEncodedFrameCallback::Encoded(
     const CodecSpecificInfo* codecSpecificInfo,
     const RTPFragmentationHeader* fragmentationHeader)
 {
-    {
-      CriticalSectionScoped cs(post_encode_callback_lock_.get());
-      if (post_encode_callback_) {
-        post_encode_callback_->Encoded(encodedImage);
-      }
-    }
+    post_encode_callback_->Encoded(encodedImage);
+
     FrameType frameType = VCMEncodedFrame::ConvertFrameType(encodedImage._frameType);
 
     uint32_t encodedBytes = 0;
@@ -280,10 +276,4 @@ void VCMEncodedFrameCallback::CopyCodecSpecific(const CodecSpecificInfo& info,
       return;
   }
 }
-
-void VCMEncodedFrameCallback::RegisterPostEncodeImageCallback(
-    EncodedImageCallback* callback) {
-  CriticalSectionScoped cs(post_encode_callback_lock_.get());
-  post_encode_callback_ = callback;
-}
 }  // namespace webrtc

webrtc/modules/video_coding/main/source/generic_encoder.h

@@ -17,9 +17,7 @@
 
 #include "webrtc/system_wrappers/interface/scoped_ptr.h"
 
-namespace webrtc
-{
-
+namespace webrtc {
 class CriticalSectionWrapper;
 
 namespace media_optimization {
@@ -32,7 +30,7 @@ class MediaOptimization;
 class VCMEncodedFrameCallback : public EncodedImageCallback
 {
 public:
-    VCMEncodedFrameCallback();
+    VCMEncodedFrameCallback(EncodedImageCallback* post_encode_callback);
     virtual ~VCMEncodedFrameCallback();
 
     /*
@@ -59,8 +57,6 @@ public:
     void SetCodecType(VideoCodecType codecType) {_codecType = codecType;};
     void SetInternalSource(bool internalSource) { _internalSource = internalSource; };
 
-    void RegisterPostEncodeImageCallback(EncodedImageCallback* callback);
-
 private:
     /*
      * Map information from info into rtp. If no relevant information is found
@@ -76,7 +72,6 @@ private:
     VideoCodecType _codecType;
     bool _internalSource;
 
-    scoped_ptr<CriticalSectionWrapper> post_encode_callback_lock_;
     EncodedImageCallback* post_encode_callback_;
 
 #ifdef DEBUG_ENCODER_BIT_STREAM

webrtc/modules/video_coding/main/source/video_coding_impl.cc

@@ -45,6 +45,36 @@ VCMProcessTimer::Processed() {
 }  // namespace vcm
 
 namespace {
+// This wrapper provides a way to modify the callback without the need to expose
+// a register method all the way down to the function calling it.
+class EncodedImageCallbackWrapper : public EncodedImageCallback {
+ public:
+  EncodedImageCallbackWrapper()
+      : cs_(CriticalSectionWrapper::CreateCriticalSection()), callback_(NULL) {}
+
+  virtual ~EncodedImageCallbackWrapper() {}
+
+  void Register(EncodedImageCallback* callback) {
+    CriticalSectionScoped cs(cs_.get());
+    callback_ = callback;
+  }
+
+  // TODO(andresp): Change to void as return value is ignored.
+  virtual int32_t Encoded(EncodedImage& encoded_image,
+                          const CodecSpecificInfo* codec_specific_info,
+                          const RTPFragmentationHeader* fragmentation) {
+    CriticalSectionScoped cs(cs_.get());
+    if (callback_)
+      return callback_->Encoded(
+          encoded_image, codec_specific_info, fragmentation);
+    return 0;
+  }
+
+ private:
+  scoped_ptr<CriticalSectionWrapper> cs_;
+  EncodedImageCallback* callback_ GUARDED_BY(cs_);
+};
+
 class VideoCodingModuleImpl : public VideoCodingModule {
  public:
   VideoCodingModuleImpl(const int32_t id,
@@ -52,7 +82,7 @@ class VideoCodingModuleImpl : public VideoCodingModule {
                         EventFactory* event_factory,
                         bool owns_event_factory)
       : VideoCodingModule(),
-        sender_(new vcm::VideoSender(id, clock)),
+        sender_(new vcm::VideoSender(id, clock, &post_encode_callback_)),
         receiver_(new vcm::VideoReceiver(id, clock, event_factory)),
         own_event_factory_(owns_event_factory ? event_factory : NULL) {}
 
@@ -327,10 +357,11 @@ class VideoCodingModuleImpl : public VideoCodingModule {
 
   virtual void RegisterPostEncodeImageCallback(
       EncodedImageCallback* observer) OVERRIDE {
-    sender_->RegisterPostEncodeImageCallback(observer);
+    post_encode_callback_.Register(observer);
   }
 
  private:
+  EncodedImageCallbackWrapper post_encode_callback_;
   scoped_ptr<vcm::VideoSender> sender_;
   scoped_ptr<vcm::VideoReceiver> receiver_;
   scoped_ptr<EventFactory> own_event_factory_;

webrtc/modules/video_coding/main/source/video_coding_impl.h

@@ -54,7 +54,10 @@ class VideoSender {
  public:
   typedef VideoCodingModule::SenderNackMode SenderNackMode;
 
-  VideoSender(const int32_t id, Clock* clock);
+  VideoSender(const int32_t id,
+              Clock* clock,
+              EncodedImageCallback* post_encode_callback);
+
   ~VideoSender();
 
   int32_t InitializeSender();
@@ -103,9 +106,6 @@ class VideoSender {
   void SuspendBelowMinBitrate();
   bool VideoSuspended() const;
 
-  void RegisterPostEncodeImageCallback(
-      EncodedImageCallback* post_encode_callback);
-
   int32_t TimeUntilNextProcess();
   int32_t Process();
 

webrtc/modules/video_coding/main/source/video_sender.cc

@@ -57,14 +57,16 @@ class DebugRecorder {
   FILE* file_ GUARDED_BY(cs_);
 };
 
-VideoSender::VideoSender(const int32_t id, Clock* clock)
+VideoSender::VideoSender(const int32_t id,
+                         Clock* clock,
+                         EncodedImageCallback* post_encode_callback)
     : _id(id),
       clock_(clock),
       recorder_(new DebugRecorder()),
       process_crit_sect_(CriticalSectionWrapper::CreateCriticalSection()),
       _sendCritSect(CriticalSectionWrapper::CreateCriticalSection()),
       _encoder(),
-      _encodedFrameCallback(),
+      _encodedFrameCallback(post_encode_callback),
       _nextFrameTypes(1, kVideoFrameDelta),
       _mediaOpt(id, clock_),
       _sendStatsCallback(NULL),
@@ -471,12 +473,5 @@ bool VideoSender::VideoSuspended() const {
   CriticalSectionScoped cs(_sendCritSect);
   return _mediaOpt.IsVideoSuspended();
 }
-
-void VideoSender::RegisterPostEncodeImageCallback(
-    EncodedImageCallback* observer) {
-  CriticalSectionScoped cs(_sendCritSect);
-  _encodedFrameCallback.RegisterPostEncodeImageCallback(observer);
-}
-
 }  // namespace vcm
 }  // namespace webrtc

webrtc/modules/video_coding/main/source/video_sender_unittest.cc

@@ -173,7 +173,7 @@ class TestVideoSender : public ::testing::Test {
   TestVideoSender() : clock_(1000), packetization_callback_(&clock_) {}
 
   virtual void SetUp() {
-    sender_.reset(new VideoSender(0, &clock_));
+    sender_.reset(new VideoSender(0, &clock_, &post_encode_callback_));
     EXPECT_EQ(0, sender_->InitializeSender());
     EXPECT_EQ(0, sender_->RegisterTransportCallback(&packetization_callback_));
   }
@@ -185,6 +185,7 @@ class TestVideoSender : public ::testing::Test {
 
   SimulatedClock clock_;
   PacketizationCallback packetization_callback_;
+  MockEncodedImageCallback post_encode_callback_;
   scoped_ptr<VideoSender> sender_;
   scoped_ptr<FrameGenerator> generator_;
 };