Skip to content

Commit

Permalink
Revert "SeqNumUnwrapper::Unwrap now returns int64_t instead of uint64…
Browse files Browse the repository at this point in the history
…_t."

This reverts commit b0f968a.

Reason for revert: Need to update DecodedFramesHistory to manage negative picture IDs.

Original change's description:
> SeqNumUnwrapper::Unwrap now returns int64_t instead of uint64_t.
> 
> Bug: webrtc:10263
> Change-Id: Idaeae6be01bd4eba0691226c958d70e114161ffd
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/127295
> Commit-Queue: Philip Eliasson <[email protected]>
> Reviewed-by: Johannes Kron <[email protected]>
> Reviewed-by: Karl Wiberg <[email protected]>
> Cr-Commit-Position: refs/heads/master@{#27129}

[email protected],[email protected],[email protected],[email protected],[email protected]

Change-Id: I529bb0475bd21a80fa244278aff1fd912a85c169
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: webrtc:10263
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/127885
Reviewed-by: Philip Eliasson <[email protected]>
Commit-Queue: Philip Eliasson <[email protected]>
Cr-Commit-Position: refs/heads/master@{#27135}
  • Loading branch information
Philipel-WebRTC authored and Commit Bot committed Mar 14, 2019
1 parent 38e6c66 commit b5207b4
Show file tree
Hide file tree
Showing 9 changed files with 449 additions and 410 deletions.
19 changes: 11 additions & 8 deletions logging/rtc_event_log/rtc_event_log_parser.cc
Original file line number Diff line number Diff line change
Expand Up @@ -57,10 +57,18 @@ constexpr size_t kStunOverhead = 4;
constexpr uint16_t kDefaultOverhead =
kUdpOverhead + kSrtpOverhead + kIpv4Overhead;

// Starting at a multiple of common audio sample rate (48000) and video tick
// rate (90000) to make a tick count of 0 to correspond to something without
// decimals in base 10. Starting at 0 is not safe as it would cause negative
// wraparound if the first timestamps are out of order.
constexpr uint64_t kStartingCaptureTimeTicks = 90 * 48 * 1000;

struct MediaStreamInfo {
MediaStreamInfo() = default;
MediaStreamInfo() : unwrap_capture_ticks(kStartingCaptureTimeTicks) {}
MediaStreamInfo(LoggedMediaType media_type, bool rtx)
: media_type(media_type), rtx(rtx) {}
: media_type(media_type),
rtx(rtx),
unwrap_capture_ticks(kStartingCaptureTimeTicks) {}
LoggedMediaType media_type = LoggedMediaType::kUnknown;
bool rtx = false;
SeqNumUnwrapper<uint32_t> unwrap_capture_ticks;
Expand Down Expand Up @@ -1945,12 +1953,7 @@ std::vector<LoggedPacketInfo> ParsedRtcEventLog::GetPacketInfos(
// RTX copy the timestamp of the retransmitted packets. This means that
// RTX streams don't have a unique clock offset and frequency, so
// the RTP timstamps can't be unwrapped.

// Add an offset to avoid |capture_ticks| to become negative in the case
// of reordering.
constexpr int64_t kStartingCaptureTimeTicks = 90 * 48 * 1000;
int64_t capture_ticks =
kStartingCaptureTimeTicks +
uint64_t capture_ticks =
stream->unwrap_capture_ticks.Unwrap(rtp.header.timestamp);
// TODO(srte): Use logged sample rate when it is added to the format.
capture_time = Timestamp::seconds(
Expand Down
5 changes: 4 additions & 1 deletion modules/video_coding/frame_buffer2.cc
Original file line number Diff line number Diff line change
Expand Up @@ -328,8 +328,11 @@ void FrameBuffer::UpdateRtt(int64_t rtt_ms) {
}

bool FrameBuffer::ValidReferences(const EncodedFrame& frame) const {
if (frame.id.picture_id < 0)
return false;

for (size_t i = 0; i < frame.num_references; ++i) {
if (frame.references[i] >= frame.id.picture_id)
if (frame.references[i] < 0 || frame.references[i] >= frame.id.picture_id)
return false;

for (size_t j = i + 1; j < frame.num_references; ++j) {
Expand Down
9 changes: 5 additions & 4 deletions modules/video_coding/loss_notification_controller.cc
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ void LossNotificationController::OnReceivedPacket(const VCMPacket& packet) {

if (packet.generic_descriptor->FirstPacketInSubFrame()) {
const uint16_t frame_id = packet.generic_descriptor->FrameId();
const int64_t unwrapped_frame_id = frame_id_unwrapper_.Unwrap(frame_id);
const uint64_t unwrapped_frame_id = frame_id_unwrapper_.Unwrap(frame_id);

// Ignore repeated or reordered frames.
// TODO(TODO(bugs.webrtc.org/10336): Handle frame reordering.
Expand Down Expand Up @@ -124,7 +124,7 @@ void LossNotificationController::OnAssembledFrame(
return;
}

const int64_t unwrapped_frame_id = frame_id_unwrapper_.Unwrap(frame_id);
const uint64_t unwrapped_frame_id = frame_id_unwrapper_.Unwrap(frame_id);
if (!AllDependenciesDecodable(unwrapped_frame_id, frame_dependency_diffs)) {
return;
}
Expand All @@ -142,7 +142,7 @@ void LossNotificationController::DiscardOldInformation() {
}

bool LossNotificationController::AllDependenciesDecodable(
int64_t unwrapped_frame_id,
uint64_t unwrapped_frame_id,
rtc::ArrayView<const uint16_t> frame_dependency_diffs) const {
RTC_DCHECK_CALLED_SEQUENTIALLY(&sequenced_task_checker_);

Expand All @@ -154,7 +154,8 @@ bool LossNotificationController::AllDependenciesDecodable(
// One possibility that is ignored, is that the packet may be corrupt.

for (uint16_t frame_dependency_diff : frame_dependency_diffs) {
const int64_t unwrapped_ref_frame_id =
RTC_DCHECK_GT(unwrapped_frame_id, frame_dependency_diff);
const uint64_t unwrapped_ref_frame_id =
unwrapped_frame_id - frame_dependency_diff;

const auto ref_frame_it =
Expand Down
6 changes: 3 additions & 3 deletions modules/video_coding/loss_notification_controller.h
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ class LossNotificationController {
void DiscardOldInformation();

bool AllDependenciesDecodable(
int64_t unwrapped_frame_id,
uint64_t unwrapped_frame_id,
rtc::ArrayView<const uint16_t> frame_dependency_diffs) const;

// When the loss of a packet or the non-decodability of a frame is detected,
Expand Down Expand Up @@ -70,7 +70,7 @@ class LossNotificationController {
RTC_GUARDED_BY(sequenced_task_checker_);

// Tracked to avoid processing repeated frames (buggy/malicious remote).
absl::optional<int64_t> last_received_unwrapped_frame_id_
absl::optional<uint64_t> last_received_unwrapped_frame_id_
RTC_GUARDED_BY(sequenced_task_checker_);

// Tracked to avoid processing repeated packets.
Expand All @@ -97,7 +97,7 @@ class LossNotificationController {
// Track which frames are decodable. Later frames are also decodable if
// all of their dependencies can be found in this container.
// (Naturally, later frames must also be assemblable to be decodable.)
std::set<int64_t> decodable_unwrapped_frame_ids_
std::set<uint64_t> decodable_unwrapped_frame_ids_
RTC_GUARDED_BY(sequenced_task_checker_);

rtc::SequencedTaskChecker sequenced_task_checker_;
Expand Down
Loading

0 comments on commit b5207b4

Please sign in to comment.