Skip to content

Commit

Permalink
ppapi: VideoEncoder: prevent scheduling encoding multiple times
Browse files Browse the repository at this point in the history
The ScheduleNextEncode() can be called at multiple stages in the
example depending on whether it needs to reconfigure the
MediaVideoStreamTrack. This can lead the example to encode at twice
30fps. At this speed we can exhaust the 33ms lifespan of video
buffers, leading to errors in the example plugins which can't retrieve
new frames to respect the 30fps framerate.

BUG=503153
TEST=run video_encoder example from NaCl SDK with VP8 codec and verify there are no error messages after 1 minute

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

Cr-Commit-Position: refs/heads/master@{#336850}
  • Loading branch information
llandwerlin-intel authored and Commit bot committed Jun 30, 2015
1 parent 24e706d commit cd74d55
Show file tree
Hide file tree
Showing 2 changed files with 40 additions and 2 deletions.
21 changes: 20 additions & 1 deletion native_client_sdk/src/examples/api/video_encode/video_encode.cc
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,10 @@

namespace {

double clamp(double min, double max, double value) {
return std::max(std::min(value, max), min);
}

// IVF container writer. It is possible to parse H264 bitstream using
// NAL units but for VP8 we need a container to at least find encoded
// pictures as well as the picture sizes.
Expand Down Expand Up @@ -161,6 +165,7 @@ class VideoEncoderInstance : public pp::Instance {
VideoProfileToStringMap profile_to_string_;

bool is_encoding_;
bool is_encode_ticking_;
bool is_receiving_track_frames_;

pp::VideoEncoder video_encoder_;
Expand All @@ -186,6 +191,7 @@ VideoEncoderInstance::VideoEncoderInstance(PP_Instance instance,
pp::Module* module)
: pp::Instance(instance),
is_encoding_(false),
is_encode_ticking_(false),
callback_factory_(this),
#if defined(USE_VP8_INSTEAD_OF_H264)
video_profile_(PP_VIDEOPROFILE_VP8_ANY),
Expand Down Expand Up @@ -340,15 +346,28 @@ void VideoEncoderInstance::OnInitializedEncoder(int32_t result) {
}

void VideoEncoderInstance::ScheduleNextEncode() {
// Avoid scheduling more than once at a time.
if (is_encode_ticking_)
return;

PP_Time now = pp::Module::Get()->core()->GetTime();
PP_Time tick = 1.0 / 30;
// If the callback was triggered late, we need to account for that
// delay for the next tick.
PP_Time delta = tick - clamp(0, tick, now - last_encode_tick_ - tick);

pp::Module::Get()->core()->CallOnMainThread(
std::min(std::max(now - last_encode_tick_, 0.0), 1000.0 / 30),
delta * 1000,
callback_factory_.NewCallback(&VideoEncoderInstance::GetEncoderFrameTick),
0);

last_encode_tick_ = now;
is_encode_ticking_ = true;
}

void VideoEncoderInstance::GetEncoderFrameTick(int32_t result) {
is_encode_ticking_ = false;

if (is_encoding_) {
if (!current_track_frame_.is_null()) {
pp::VideoFrame frame = current_track_frame_;
Expand Down
21 changes: 20 additions & 1 deletion ppapi/examples/video_encode/video_encode.cc
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,10 @@

namespace {

double clamp(double min, double max, double value) {
return std::max(std::min(value, max), min);
}

// IVF container writer. It is possible to parse H264 bitstream using
// NAL units but for VP8 we need a container to at least find encoded
// pictures as well as the picture sizes.
Expand Down Expand Up @@ -160,6 +164,7 @@ class VideoEncoderInstance : public pp::Instance {
VideoProfileToStringMap profile_to_string_;

bool is_encoding_;
bool is_encode_ticking_;
bool is_receiving_track_frames_;

pp::VideoEncoder video_encoder_;
Expand All @@ -185,6 +190,7 @@ VideoEncoderInstance::VideoEncoderInstance(PP_Instance instance,
pp::Module* module)
: pp::Instance(instance),
is_encoding_(false),
is_encode_ticking_(false),
callback_factory_(this),
#if defined(USE_VP8_INSTEAD_OF_H264)
video_profile_(PP_VIDEOPROFILE_VP8_ANY),
Expand Down Expand Up @@ -343,15 +349,28 @@ void VideoEncoderInstance::OnInitializedEncoder(int32_t result) {
}

void VideoEncoderInstance::ScheduleNextEncode() {
// Avoid scheduling more than once at a time.
if (is_encode_ticking_)
return;

PP_Time now = pp::Module::Get()->core()->GetTime();
PP_Time tick = 1.0 / 30;
// If the callback was triggered late, we need to account for that
// delay for the next tick.
PP_Time delta = tick - clamp(0, tick, now - last_encode_tick_ - tick);

pp::Module::Get()->core()->CallOnMainThread(
std::min(std::max(now - last_encode_tick_, 0.0), 1000.0 / 30),
delta * 1000,
callback_factory_.NewCallback(&VideoEncoderInstance::GetEncoderFrameTick),
0);

last_encode_tick_ = now;
is_encode_ticking_ = true;
}

void VideoEncoderInstance::GetEncoderFrameTick(int32_t result) {
is_encode_ticking_ = false;

if (is_encoding_) {
if (!current_track_frame_.is_null()) {
pp::VideoFrame frame = current_track_frame_;
Expand Down

0 comments on commit cd74d55

Please sign in to comment.