Skip to content

Commit

Permalink
Fixed wrap-around in MapTo32bitsFrameId and added unit tests.
Browse files Browse the repository at this point in the history
BUG=326625

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

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@239316 0039d316-1c4b-4281-b951-d872f2087c98
  • Loading branch information
hguihot@google.com committed Dec 7, 2013
1 parent 94cf31c commit 372145b
Show file tree
Hide file tree
Showing 3 changed files with 92 additions and 18 deletions.
6 changes: 2 additions & 4 deletions media/cast/cast.gyp
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
'sources': [
'cast_config.cc',
'cast_config.h',
'cast_defines.h',
'cast_environment.cc',
'cast_environment.h',
'logging/logging_defines.cc',
Expand Down Expand Up @@ -88,14 +89,11 @@
'test/crypto_utility.h',
'test/encode_decode_test.cc',
'test/end2end_unittest.cc',
'test/frame_id_wrap_helper_test.cc',
'video_receiver/video_decoder_unittest.cc',
'video_receiver/video_receiver_unittest.cc',
'video_sender/mock_video_encoder_controller.cc',
'video_sender/mock_video_encoder_controller.h',
'pacing/paced_sender_unittest.cc',
'rtcp/rtcp_receiver_unittest.cc',
'rtcp/rtcp_sender_unittest.cc',
'rtcp/rtcp_unittest.cc',
'video_sender/video_encoder_unittest.cc',
'video_sender/video_sender_unittest.cc',
], # source
Expand Down
56 changes: 42 additions & 14 deletions media/cast/cast_defines.h
Original file line number Diff line number Diff line change
Expand Up @@ -139,8 +139,8 @@ class FrameIdWrapHelper {
public:
FrameIdWrapHelper()
: first_(true),
can_we_wrap_(false),
frame_id_wrap_count_(0) {}
frame_id_wrap_count_(0),
range_(kLowRange) {}

uint32 MapTo32bitsFrameId(const uint8 over_the_wire_frame_id) {
if (first_) {
Expand All @@ -150,24 +150,52 @@ class FrameIdWrapHelper {
return kStartFrameId;
}
}
if (can_we_wrap_) {
if (over_the_wire_frame_id < 0x0f) {
// Disable wrap check until we are closer to the max of uint8.
can_we_wrap_ = false;
}
} else {
if (over_the_wire_frame_id > 0xf0) {
// Enable wrap check until we have wrapped.
can_we_wrap_ = true;
}

uint32 wrap_count = frame_id_wrap_count_;
switch (range_) {
case kLowRange:
if (over_the_wire_frame_id > kLowRangeThreshold &&
over_the_wire_frame_id < kHighRangeThreshold) {
range_ = kMiddleRange;
}
if (over_the_wire_frame_id > kHighRangeThreshold) {
// Wrap count was incremented in High->Low transition, but this frame
// is 'old', actually from before the wrap count got incremented.
--wrap_count;
}
break;
case kMiddleRange:
if (over_the_wire_frame_id > kHighRangeThreshold) {
range_ = kHighRange;
}
break;
case kHighRange:
if (over_the_wire_frame_id < kLowRangeThreshold) {
// Wrap-around detected.
range_ = kLowRange;
++frame_id_wrap_count_;
// Frame triggering wrap-around so wrap count should be incremented as
// as well to match |frame_id_wrap_count_|.
++wrap_count;
}
break;
}
return (frame_id_wrap_count_ << 8) + over_the_wire_frame_id;
return (wrap_count << 8) + over_the_wire_frame_id;
}

private:
enum Range {
kLowRange,
kMiddleRange,
kHighRange,
};

static const uint8 kLowRangeThreshold = 0x0f;
static const uint8 kHighRangeThreshold = 0xf0;

bool first_;
bool can_we_wrap_;
uint32 frame_id_wrap_count_;
Range range_;
};

inline std::string GetAesNonce(uint32 frame_id, const std::string& iv_mask) {
Expand Down
48 changes: 48 additions & 0 deletions media/cast/test/frame_id_wrap_helper_test.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
// Copyright 2013 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.

#include <gtest/gtest.h>
#include <media/cast/cast_defines.h>

namespace media {
namespace cast {

class FrameIdWrapHelperTest : public ::testing::Test {
protected:
FrameIdWrapHelperTest() {}
virtual ~FrameIdWrapHelperTest() {}

FrameIdWrapHelper frame_id_wrap_helper_;
};

TEST_F(FrameIdWrapHelperTest, FirstFrame) {
EXPECT_EQ(kStartFrameId, frame_id_wrap_helper_.MapTo32bitsFrameId(255u));
}

TEST_F(FrameIdWrapHelperTest, Rollover) {
uint32 new_frame_id = 0u;
for (int i = 0; i <= 256; ++i) {
new_frame_id = frame_id_wrap_helper_.MapTo32bitsFrameId(
static_cast<uint8>(i));
}
EXPECT_EQ(256u, new_frame_id);
}

TEST_F(FrameIdWrapHelperTest, OutOfOrder) {
uint32 new_frame_id = 0u;
for (int i = 0; i < 255; ++i) {
new_frame_id = frame_id_wrap_helper_.MapTo32bitsFrameId(
static_cast<uint8>(i));
}
EXPECT_EQ(254u, new_frame_id);
new_frame_id = frame_id_wrap_helper_.MapTo32bitsFrameId(0u);
EXPECT_EQ(256u, new_frame_id);
new_frame_id = frame_id_wrap_helper_.MapTo32bitsFrameId(255u);
EXPECT_EQ(255u, new_frame_id);
new_frame_id = frame_id_wrap_helper_.MapTo32bitsFrameId(1u);
EXPECT_EQ(257u, new_frame_id);
}

} // namespace cast
} // namespace media

0 comments on commit 372145b

Please sign in to comment.