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
diff --git a/webrtc/modules/video_coding/main/source/generic_encoder.cc b/webrtc/modules/video_coding/main/source/generic_encoder.cc
index 064470b..7fa0cc6 100644
--- a/webrtc/modules/video_coding/main/source/generic_encoder.cc
+++ b/webrtc/modules/video_coding/main/source/generic_encoder.cc
@@ -142,15 +142,15 @@
/***************************
* 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 @@
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 @@
return;
}
}
-
-void VCMEncodedFrameCallback::RegisterPostEncodeImageCallback(
- EncodedImageCallback* callback) {
- CriticalSectionScoped cs(post_encode_callback_lock_.get());
- post_encode_callback_ = callback;
-}
} // namespace webrtc
diff --git a/webrtc/modules/video_coding/main/source/generic_encoder.h b/webrtc/modules/video_coding/main/source/generic_encoder.h
index c5cfeab..83c6c46 100644
--- a/webrtc/modules/video_coding/main/source/generic_encoder.h
+++ b/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 VCMEncodedFrameCallback : public EncodedImageCallback
{
public:
- VCMEncodedFrameCallback();
+ VCMEncodedFrameCallback(EncodedImageCallback* post_encode_callback);
virtual ~VCMEncodedFrameCallback();
/*
@@ -59,8 +57,6 @@
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 @@
VideoCodecType _codecType;
bool _internalSource;
- scoped_ptr<CriticalSectionWrapper> post_encode_callback_lock_;
EncodedImageCallback* post_encode_callback_;
#ifdef DEBUG_ENCODER_BIT_STREAM
diff --git a/webrtc/modules/video_coding/main/source/video_coding_impl.cc b/webrtc/modules/video_coding/main/source/video_coding_impl.cc
index f1ec0eb..aea562d 100644
--- a/webrtc/modules/video_coding/main/source/video_coding_impl.cc
+++ b/webrtc/modules/video_coding/main/source/video_coding_impl.cc
@@ -45,6 +45,36 @@
} // 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 @@
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 @@
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_;
diff --git a/webrtc/modules/video_coding/main/source/video_coding_impl.h b/webrtc/modules/video_coding/main/source/video_coding_impl.h
index 9f22c0b..13fae2b 100644
--- a/webrtc/modules/video_coding/main/source/video_coding_impl.h
+++ b/webrtc/modules/video_coding/main/source/video_coding_impl.h
@@ -54,7 +54,10 @@
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 @@
void SuspendBelowMinBitrate();
bool VideoSuspended() const;
- void RegisterPostEncodeImageCallback(
- EncodedImageCallback* post_encode_callback);
-
int32_t TimeUntilNextProcess();
int32_t Process();
diff --git a/webrtc/modules/video_coding/main/source/video_sender.cc b/webrtc/modules/video_coding/main/source/video_sender.cc
index 5ac842e..52a0f86 100644
--- a/webrtc/modules/video_coding/main/source/video_sender.cc
+++ b/webrtc/modules/video_coding/main/source/video_sender.cc
@@ -57,14 +57,16 @@
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 @@
CriticalSectionScoped cs(_sendCritSect);
return _mediaOpt.IsVideoSuspended();
}
-
-void VideoSender::RegisterPostEncodeImageCallback(
- EncodedImageCallback* observer) {
- CriticalSectionScoped cs(_sendCritSect);
- _encodedFrameCallback.RegisterPostEncodeImageCallback(observer);
-}
-
} // namespace vcm
} // namespace webrtc
diff --git a/webrtc/modules/video_coding/main/source/video_sender_unittest.cc b/webrtc/modules/video_coding/main/source/video_sender_unittest.cc
index 513a99e..196eff3 100644
--- a/webrtc/modules/video_coding/main/source/video_sender_unittest.cc
+++ b/webrtc/modules/video_coding/main/source/video_sender_unittest.cc
@@ -173,7 +173,7 @@
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 @@
SimulatedClock clock_;
PacketizationCallback packetization_callback_;
+ MockEncodedImageCallback post_encode_callback_;
scoped_ptr<VideoSender> sender_;
scoped_ptr<FrameGenerator> generator_;
};
