Skip to content

Commit

Permalink
Cast: Remove the closure callback from InsertRawVideoFrame
Browse files Browse the repository at this point in the history 
Cleanup; closue no longer needed after the switch to media::VideoFrame

Should be submitted after: 
https://codereview.chromium.org/108403004/

Review URL: https://codereview.chromium.org/97993003

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@239654 0039d316-1c4b-4281-b951-d872f2087c98
  • Loading branch information
pwestin@google.com committed Dec 10, 2013
1 parent 3810678 commit 807f21f
Show file tree
Hide file tree
Showing 10 changed files with 32 additions and 54 deletions.
3 changes: 1 addition & 2 deletions media/cast/cast_sender.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,7 @@ class FrameInput : public base::RefCountedThreadSafe<FrameInput> {
// has been sent out.
virtual void InsertRawVideoFrame(
const scoped_refptr<media::VideoFrame>& video_frame,
const base::TimeTicks& capture_time,
const base::Closure& callback) = 0;
const base::TimeTicks& capture_time) = 0;

// The video_frame must be valid until the callback is called.
// The callback is called from the main cast thread as soon as
Expand Down
5 changes: 2 additions & 3 deletions media/cast/cast_sender_impl.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,10 @@ class LocalFrameInput : public FrameInput {

virtual void InsertRawVideoFrame(
const scoped_refptr<media::VideoFrame>& video_frame,
const base::TimeTicks& capture_time,
const base::Closure& callback) OVERRIDE {
const base::TimeTicks& capture_time) OVERRIDE {
cast_environment_->PostTask(CastEnvironment::MAIN, FROM_HERE,
base::Bind(&VideoSender::InsertRawVideoFrame, video_sender_,
video_frame, capture_time, callback));
video_frame, capture_time));
}

virtual void InsertCodedVideoFrame(const EncodedVideoFrame* video_frame,
Expand Down
3 changes: 1 addition & 2 deletions media/cast/test/end2end_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -435,8 +435,7 @@ class End2EndTest : public ::testing::Test {
media::VideoFrame::CreateFrame(
VideoFrame::I420, size, gfx::Rect(size), size, time_diff);
PopulateVideoFrame(video_frame, start_value);
frame_input_->InsertRawVideoFrame(video_frame, capture_time,
base::Bind(base::DoNothing));
frame_input_->InsertRawVideoFrame(video_frame, capture_time);
}

void RunTasks(int during_ms) {
Expand Down
11 changes: 3 additions & 8 deletions media/cast/test/sender.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -225,11 +225,6 @@ class SendProcess {
fclose(video_file_);
}

void ReleaseVideoFrame(const scoped_refptr<media::VideoFrame>&) {
test_app_thread_proxy_->PostTask(FROM_HERE,
base::Bind(&SendProcess::SendFrame, base::Unretained(this)));
}

void SendFrame() {
// Make sure that we don't drift.
int num_10ms_blocks = audio_diff_ / 10;
Expand Down Expand Up @@ -280,9 +275,9 @@ class SendProcess {

void SendVideoFrameOnTime(scoped_refptr<media::VideoFrame> video_frame) {
send_time_ = clock_->NowTicks();
frame_input_->InsertRawVideoFrame(video_frame, send_time_,
base::Bind(&SendProcess::ReleaseVideoFrame, weak_factory_.GetWeakPtr(),
video_frame));
frame_input_->InsertRawVideoFrame(video_frame, send_time_);
test_app_thread_proxy_->PostTask(FROM_HERE,
base::Bind(&SendProcess::SendFrame, base::Unretained(this)));
}

private:
Expand Down
12 changes: 3 additions & 9 deletions media/cast/video_sender/video_encoder.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,7 @@ VideoEncoder::~VideoEncoder() {}
bool VideoEncoder::EncodeVideoFrame(
const scoped_refptr<media::VideoFrame>& video_frame,
const base::TimeTicks& capture_time,
const FrameEncodedCallback& frame_encoded_callback,
const base::Closure frame_release_callback) {
const FrameEncodedCallback& frame_encoded_callback) {
DCHECK(cast_environment_->CurrentlyOn(CastEnvironment::MAIN));
if (video_config_.codec != kVp8) return false;

Expand All @@ -58,8 +57,7 @@ bool VideoEncoder::EncodeVideoFrame(
cast_environment_->PostTask(CastEnvironment::VIDEO_ENCODER, FROM_HERE,
base::Bind(&VideoEncoder::EncodeVideoFrameEncoderThread,
base::Unretained(this), video_frame, capture_time,
dynamic_config_, frame_encoded_callback,
frame_release_callback));
dynamic_config_, frame_encoded_callback));

dynamic_config_.key_frame_requested = false;
return true;
Expand All @@ -69,8 +67,7 @@ void VideoEncoder::EncodeVideoFrameEncoderThread(
const scoped_refptr<media::VideoFrame>& video_frame,
const base::TimeTicks& capture_time,
const CodecDynamicConfig& dynamic_config,
const FrameEncodedCallback& frame_encoded_callback,
const base::Closure frame_release_callback) {
const FrameEncodedCallback& frame_encoded_callback) {
DCHECK(cast_environment_->CurrentlyOn(CastEnvironment::VIDEO_ENCODER));
if (dynamic_config.key_frame_requested) {
vp8_encoder_->GenerateKeyFrame();
Expand All @@ -84,9 +81,6 @@ void VideoEncoder::EncodeVideoFrameEncoderThread(

cast_environment_->PostTask(CastEnvironment::MAIN, FROM_HERE,
base::Bind(LogFrameEncodedEvent, cast_environment_, capture_time));
// We are done with the video frame release it.
cast_environment_->PostTask(CastEnvironment::MAIN, FROM_HERE,
frame_release_callback);

if (!retval) {
VLOG(1) << "Encoding failed";
Expand Down
6 changes: 2 additions & 4 deletions media/cast/video_sender/video_encoder.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,7 @@ class VideoEncoder : public VideoEncoderController {
// Once the encoded frame is ready the frame_encoded_callback is called.
bool EncodeVideoFrame(const scoped_refptr<media::VideoFrame>& video_frame,
const base::TimeTicks& capture_time,
const FrameEncodedCallback& frame_encoded_callback,
const base::Closure frame_release_callback);
const FrameEncodedCallback& frame_encoded_callback);

protected:
struct CodecDynamicConfig {
Expand All @@ -56,8 +55,7 @@ class VideoEncoder : public VideoEncoderController {
const scoped_refptr<media::VideoFrame>& video_frame,
const base::TimeTicks& capture_time,
const CodecDynamicConfig& dynamic_config,
const FrameEncodedCallback& frame_encoded_callback,
const base::Closure frame_release_callback);
const FrameEncodedCallback& frame_encoded_callback);

// The following functions are called from the main cast thread.
virtual void SetBitRate(int new_bit_rate) OVERRIDE;
Expand Down
28 changes: 14 additions & 14 deletions media/cast/video_sender/video_encoder_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -119,21 +119,21 @@ TEST_F(VideoEncoderTest, EncodePattern30fpsRunningOutOfAck) {
capture_time += base::TimeDelta::FromMilliseconds(33);
test_video_encoder_callback_->SetExpectedResult(true, 0, 0, capture_time);
EXPECT_TRUE(video_encoder_->EncodeVideoFrame(video_frame_, capture_time,
frame_encoded_callback, base::Bind(base::DoNothing)));
frame_encoded_callback));
task_runner_->RunTasks();

capture_time += base::TimeDelta::FromMilliseconds(33);
video_encoder_controller_->LatestFrameIdToReference(0);
test_video_encoder_callback_->SetExpectedResult(false, 1, 0, capture_time);
EXPECT_TRUE(video_encoder_->EncodeVideoFrame(video_frame_, capture_time,
frame_encoded_callback, base::Bind(base::DoNothing)));
frame_encoded_callback));
task_runner_->RunTasks();

capture_time += base::TimeDelta::FromMilliseconds(33);
video_encoder_controller_->LatestFrameIdToReference(1);
test_video_encoder_callback_->SetExpectedResult(false, 2, 1, capture_time);
EXPECT_TRUE(video_encoder_->EncodeVideoFrame(video_frame_, capture_time,
frame_encoded_callback, base::Bind(base::DoNothing)));
frame_encoded_callback));
task_runner_->RunTasks();

video_encoder_controller_->LatestFrameIdToReference(2);
Expand All @@ -142,7 +142,7 @@ TEST_F(VideoEncoderTest, EncodePattern30fpsRunningOutOfAck) {
capture_time += base::TimeDelta::FromMilliseconds(33);
test_video_encoder_callback_->SetExpectedResult(false, i, 2, capture_time);
EXPECT_TRUE(video_encoder_->EncodeVideoFrame(video_frame_, capture_time,
frame_encoded_callback, base::Bind(base::DoNothing)));
frame_encoded_callback));
task_runner_->RunTasks();
}
}
Expand All @@ -161,21 +161,21 @@ TEST_F(VideoEncoderTest,DISABLED_EncodePattern60fpsRunningOutOfAck) {
capture_time += base::TimeDelta::FromMilliseconds(33);
test_video_encoder_callback_->SetExpectedResult(true, 0, 0, capture_time);
EXPECT_TRUE(video_encoder_->EncodeVideoFrame(video_frame_, capture_time,
frame_encoded_callback, base::Bind(base::DoNothing)));
frame_encoded_callback));
task_runner_->RunTasks();

video_encoder_controller_->LatestFrameIdToReference(0);
capture_time += base::TimeDelta::FromMilliseconds(33);
test_video_encoder_callback_->SetExpectedResult(false, 1, 0, capture_time);
EXPECT_TRUE(video_encoder_->EncodeVideoFrame(video_frame_, capture_time,
frame_encoded_callback, base::Bind(base::DoNothing)));
frame_encoded_callback));
task_runner_->RunTasks();

video_encoder_controller_->LatestFrameIdToReference(1);
capture_time += base::TimeDelta::FromMilliseconds(33);
test_video_encoder_callback_->SetExpectedResult(false, 2, 0, capture_time);
EXPECT_TRUE(video_encoder_->EncodeVideoFrame(video_frame_, capture_time,
frame_encoded_callback, base::Bind(base::DoNothing)));
frame_encoded_callback));
task_runner_->RunTasks();

video_encoder_controller_->LatestFrameIdToReference(2);
Expand All @@ -184,7 +184,7 @@ TEST_F(VideoEncoderTest,DISABLED_EncodePattern60fpsRunningOutOfAck) {
capture_time += base::TimeDelta::FromMilliseconds(33);
test_video_encoder_callback_->SetExpectedResult(false, i, 2, capture_time);
EXPECT_TRUE(video_encoder_->EncodeVideoFrame(video_frame_, capture_time,
frame_encoded_callback, base::Bind(base::DoNothing)));
frame_encoded_callback));
task_runner_->RunTasks();
}
}
Expand All @@ -202,43 +202,43 @@ TEST_F(VideoEncoderTest, DISABLED_EncodePattern60fps200msDelayRunningOutOfAck) {
capture_time += base::TimeDelta::FromMilliseconds(33);
test_video_encoder_callback_->SetExpectedResult(true, 0, 0, capture_time);
EXPECT_TRUE(video_encoder_->EncodeVideoFrame(video_frame_, capture_time,
frame_encoded_callback, base::Bind(base::DoNothing)));
frame_encoded_callback));
task_runner_->RunTasks();

video_encoder_controller_->LatestFrameIdToReference(0);
capture_time += base::TimeDelta::FromMilliseconds(33);
test_video_encoder_callback_->SetExpectedResult(false, 1, 0, capture_time);
EXPECT_TRUE(video_encoder_->EncodeVideoFrame(video_frame_, capture_time,
frame_encoded_callback, base::Bind(base::DoNothing)));
frame_encoded_callback));
task_runner_->RunTasks();

video_encoder_controller_->LatestFrameIdToReference(1);
capture_time += base::TimeDelta::FromMilliseconds(33);
test_video_encoder_callback_->SetExpectedResult(false, 2, 0, capture_time);
EXPECT_TRUE(video_encoder_->EncodeVideoFrame(video_frame_, capture_time,
frame_encoded_callback, base::Bind(base::DoNothing)));
frame_encoded_callback));
task_runner_->RunTasks();

video_encoder_controller_->LatestFrameIdToReference(2);
capture_time += base::TimeDelta::FromMilliseconds(33);
test_video_encoder_callback_->SetExpectedResult(false, 3, 0, capture_time);
EXPECT_TRUE(video_encoder_->EncodeVideoFrame(video_frame_, capture_time,
frame_encoded_callback, base::Bind(base::DoNothing)));
frame_encoded_callback));
task_runner_->RunTasks();

video_encoder_controller_->LatestFrameIdToReference(3);
capture_time += base::TimeDelta::FromMilliseconds(33);
test_video_encoder_callback_->SetExpectedResult(false, 4, 0, capture_time);
EXPECT_TRUE(video_encoder_->EncodeVideoFrame(video_frame_, capture_time,
frame_encoded_callback, base::Bind(base::DoNothing)));
frame_encoded_callback));
task_runner_->RunTasks();

video_encoder_controller_->LatestFrameIdToReference(4);

for (int i = 5; i < 17; ++i) {
test_video_encoder_callback_->SetExpectedResult(false, i, 4, capture_time);
EXPECT_TRUE(video_encoder_->EncodeVideoFrame(video_frame_, capture_time,
frame_encoded_callback, base::Bind(base::DoNothing)));
frame_encoded_callback));
task_runner_->RunTasks();
}
}
Expand Down
6 changes: 2 additions & 4 deletions media/cast/video_sender/video_sender.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -129,17 +129,15 @@ void VideoSender::InitializeTimers() {

void VideoSender::InsertRawVideoFrame(
const scoped_refptr<media::VideoFrame>& video_frame,
const base::TimeTicks& capture_time,
const base::Closure& callback) {
const base::TimeTicks& capture_time) {
DCHECK(cast_environment_->CurrentlyOn(CastEnvironment::MAIN));
DCHECK(video_encoder_.get()) << "Invalid state";
cast_environment_->Logging()->InsertFrameEvent(kVideoFrameReceived,
GetVideoRtpTimestamp(capture_time), kFrameIdUnknown);

if (!video_encoder_->EncodeVideoFrame(video_frame, capture_time,
base::Bind(&VideoSender::SendEncodedVideoFrameMainThread,
weak_factory_.GetWeakPtr()), callback)) {
cast_environment_->PostTask(CastEnvironment::MAIN, FROM_HERE, callback);
weak_factory_.GetWeakPtr()))) {
}
}

Expand Down
3 changes: 1 addition & 2 deletions media/cast/video_sender/video_sender.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,7 @@ class VideoSender : public base::NonThreadSafe,
// has been sent out.
void InsertRawVideoFrame(
const scoped_refptr<media::VideoFrame>& video_frame,
const base::TimeTicks& capture_time,
const base::Closure& callback);
const base::TimeTicks& capture_time);

// The video_frame must be valid until the closure callback is called.
// The closure callback is called from the main thread as soon as
Expand Down
9 changes: 3 additions & 6 deletions media/cast/video_sender/video_sender_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -111,8 +111,7 @@ TEST_F(VideoSenderTest, BuiltInEncoder) {
scoped_refptr<media::VideoFrame> video_frame = GetNewVideoFrame();

base::TimeTicks capture_time;
video_sender_->InsertRawVideoFrame(video_frame, capture_time,
base::Bind(base::DoNothing));
video_sender_->InsertRawVideoFrame(video_frame, capture_time);

task_runner_->RunTasks();
}
Expand Down Expand Up @@ -171,8 +170,7 @@ TEST_F(VideoSenderTest, ResendTimer) {
scoped_refptr<media::VideoFrame> video_frame = GetNewVideoFrame();

base::TimeTicks capture_time;
video_sender_->InsertRawVideoFrame(video_frame, capture_time,
base::Bind(base::DoNothing));
video_sender_->InsertRawVideoFrame(video_frame, capture_time);

task_runner_->RunTasks();

Expand All @@ -183,8 +181,7 @@ TEST_F(VideoSenderTest, ResendTimer) {
video_sender_->OnReceivedCastFeedback(cast_feedback);

video_frame = GetNewVideoFrame();
video_sender_->InsertRawVideoFrame(video_frame, capture_time,
base::Bind(base::DoNothing));
video_sender_->InsertRawVideoFrame(video_frame, capture_time);

task_runner_->RunTasks();

Expand Down

0 comments on commit 807f21f

Please sign in to comment.