Revert of Add a flags field to video timing extension. (patchset #15 id:280001 of https://codereview.webrtc.org/3000753002/ )
Reason for revert:
Speculative revet for breaking remoting_unittests in fyi bots.
https://build.chromium.org/p/chromium.webrtc.fyi/waterfall?builder=Win7%20Tester
Original issue's description:
> Add a flags field to video timing extension.
>
> The rtp header extension for video timing shuold have an additional
> field for signaling metadata, such as what triggered the extension for
> this particular frame. This will allow separating frames select because
> of outlier sizes from regular frames, for more accurate stats.
>
> This implementation is backwards compatible in that it can read video
> timing extensions without the new flag field, but it always sends with
> it included.
>
> BUG=webrtc:7594
>
> Review-Url: https://codereview.webrtc.org/3000753002
> Cr-Commit-Position: refs/heads/master@{#19353}
> Committed: https://chromium.googlesource.com/external/webrtc/+/cf5d485e147f7d7b3081692f101e496ce9e1d257
TBR=danilchap@webrtc.org,kthelgason@webrtc.org,stefan@webrtc.org,sprang@webrtc.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=webrtc:7594
Review-Url: https://codereview.webrtc.org/2995953002
Cr-Commit-Position: refs/heads/master@{#19360}
diff --git a/webrtc/modules/video_coding/codecs/h264/h264_encoder_impl.cc b/webrtc/modules/video_coding/codecs/h264/h264_encoder_impl.cc
index f5a1910..e84dffd 100644
--- a/webrtc/modules/video_coding/codecs/h264/h264_encoder_impl.cc
+++ b/webrtc/modules/video_coding/codecs/h264/h264_encoder_impl.cc
@@ -370,7 +370,7 @@
encoded_image_.content_type_ = (mode_ == kScreensharing)
? VideoContentType::SCREENSHARE
: VideoContentType::UNSPECIFIED;
- encoded_image_.timing_.flags = TimingFrameFlags::kInvalid;
+ encoded_image_.timing_.is_timing_frame = false;
encoded_image_._frameType = ConvertToVideoFrameType(info.eFrameType);
// Split encoded image up into fragments. This also updates |encoded_image_|.
diff --git a/webrtc/modules/video_coding/codecs/vp8/vp8_impl.cc b/webrtc/modules/video_coding/codecs/vp8/vp8_impl.cc
index a225036..3f9f879 100644
--- a/webrtc/modules/video_coding/codecs/vp8/vp8_impl.cc
+++ b/webrtc/modules/video_coding/codecs/vp8/vp8_impl.cc
@@ -900,7 +900,7 @@
encoded_images_[encoder_idx].content_type_ =
(codec_.mode == kScreensharing) ? VideoContentType::SCREENSHARE
: VideoContentType::UNSPECIFIED;
- encoded_images_[encoder_idx].timing_.flags = TimingFrameFlags::kInvalid;
+ encoded_images_[encoder_idx].timing_.is_timing_frame = false;
int qp = -1;
vpx_codec_control(&encoders_[encoder_idx], VP8E_GET_LAST_QUANTIZER_64, &qp);
diff --git a/webrtc/modules/video_coding/codecs/vp9/vp9_impl.cc b/webrtc/modules/video_coding/codecs/vp9/vp9_impl.cc
index fa3e623..e1776fc 100644
--- a/webrtc/modules/video_coding/codecs/vp9/vp9_impl.cc
+++ b/webrtc/modules/video_coding/codecs/vp9/vp9_impl.cc
@@ -710,7 +710,7 @@
: VideoContentType::UNSPECIFIED;
encoded_image_._encodedHeight = raw_->d_h;
encoded_image_._encodedWidth = raw_->d_w;
- encoded_image_.timing_.flags = TimingFrameFlags::kInvalid;
+ encoded_image_.timing_.is_timing_frame = false;
int qp = -1;
vpx_codec_control(encoder_, VP8E_GET_LAST_QUANTIZER, &qp);
encoded_image_.qp_ = qp;
diff --git a/webrtc/modules/video_coding/encoded_frame.cc b/webrtc/modules/video_coding/encoded_frame.cc
index bd407c7..ec7f560 100644
--- a/webrtc/modules/video_coding/encoded_frame.cc
+++ b/webrtc/modules/video_coding/encoded_frame.cc
@@ -88,7 +88,7 @@
_codec = kVideoCodecUnknown;
rotation_ = kVideoRotation_0;
content_type_ = VideoContentType::UNSPECIFIED;
- timing_.flags = TimingFrameFlags::kInvalid;
+ timing_.is_timing_frame = false;
_rotation_set = false;
}
diff --git a/webrtc/modules/video_coding/frame_buffer.cc b/webrtc/modules/video_coding/frame_buffer.cc
index 41c7fc8..f67210f 100644
--- a/webrtc/modules/video_coding/frame_buffer.cc
+++ b/webrtc/modules/video_coding/frame_buffer.cc
@@ -164,7 +164,8 @@
rotation_ = packet.video_header.rotation;
_rotation_set = true;
content_type_ = packet.video_header.content_type;
- if (packet.video_header.video_timing.flags != TimingFrameFlags::kInvalid) {
+ if (packet.video_header.video_timing.is_timing_frame) {
+ timing_.is_timing_frame = true;
timing_.encode_start_ms =
ntp_time_ms_ + packet.video_header.video_timing.encode_start_delta_ms;
timing_.encode_finish_ms =
@@ -181,8 +182,9 @@
timing_.network2_timestamp_ms =
ntp_time_ms_ +
packet.video_header.video_timing.network2_timstamp_delta_ms;
+ } else {
+ timing_.is_timing_frame = false;
}
- timing_.flags = packet.video_header.video_timing.flags;
}
if (packet.is_first_packet_in_frame) {
diff --git a/webrtc/modules/video_coding/frame_object.cc b/webrtc/modules/video_coding/frame_object.cc
index 86bcffb..1d858fc 100644
--- a/webrtc/modules/video_coding/frame_object.cc
+++ b/webrtc/modules/video_coding/frame_object.cc
@@ -32,7 +32,6 @@
: packet_buffer_(packet_buffer),
first_seq_num_(first_seq_num),
last_seq_num_(last_seq_num),
- timestamp_(0),
received_time_(received_time),
times_nacked_(times_nacked) {
VCMPacket* first_packet = packet_buffer_->GetPacket(first_seq_num);
@@ -114,10 +113,10 @@
rotation_ = last_packet->video_header.rotation;
_rotation_set = true;
content_type_ = last_packet->video_header.content_type;
- if (last_packet->video_header.video_timing.flags !=
- TimingFrameFlags::kInvalid) {
+ if (last_packet->video_header.video_timing.is_timing_frame) {
// ntp_time_ms_ may be -1 if not estimated yet. This is not a problem,
// as this will be dealt with at the time of reporting.
+ timing_.is_timing_frame = true;
timing_.encode_start_ms =
ntp_time_ms_ +
last_packet->video_header.video_timing.encode_start_delta_ms;
@@ -139,8 +138,9 @@
timing_.receive_start_ms = first_packet->receive_time_ms;
timing_.receive_finish_ms = last_packet->receive_time_ms;
+ } else {
+ timing_.is_timing_frame = false;
}
- timing_.flags = last_packet->video_header.video_timing.flags;
}
RtpFrameObject::~RtpFrameObject() {
diff --git a/webrtc/modules/video_coding/generic_decoder.cc b/webrtc/modules/video_coding/generic_decoder.cc
index 3b2a620..cc14794 100644
--- a/webrtc/modules/video_coding/generic_decoder.cc
+++ b/webrtc/modules/video_coding/generic_decoder.cc
@@ -92,7 +92,7 @@
frameInfo->renderTimeMs);
// Report timing information.
- if (frameInfo->timing.flags != TimingFrameFlags::kInvalid) {
+ if (frameInfo->timing.is_timing_frame) {
int64_t capture_time_ms = decodedImage.ntp_time_ms() - ntp_offset_;
// Convert remote timestamps to local time from ntp timestamps.
frameInfo->timing.encode_start_ms -= ntp_offset_;
@@ -137,7 +137,6 @@
timing_frame_info.decode_finish_ms = now_ms;
timing_frame_info.render_time_ms = frameInfo->renderTimeMs;
timing_frame_info.rtp_timestamp = decodedImage.timestamp();
- timing_frame_info.flags = frameInfo->timing.flags;
_timing->SetTimingFrameInfo(timing_frame_info);
}
diff --git a/webrtc/modules/video_coding/generic_encoder.cc b/webrtc/modules/video_coding/generic_encoder.cc
index 4c2a699..78eeef2 100644
--- a/webrtc/modules/video_coding/generic_encoder.cc
+++ b/webrtc/modules/video_coding/generic_encoder.cc
@@ -231,7 +231,7 @@
rtc::Optional<size_t> outlier_frame_size;
rtc::Optional<int64_t> encode_start_ms;
- uint8_t timing_flags = TimingFrameFlags::kInvalid;
+ bool is_timing_frame = false;
{
rtc::CritScope crit(&timing_params_lock_);
@@ -274,16 +274,14 @@
if (last_timing_frame_time_ms_ == -1 ||
timing_frame_delay_ms >= timing_frames_thresholds_.delay_ms ||
timing_frame_delay_ms == 0) {
- timing_flags = TimingFrameFlags::kTriggeredByTimer;
+ is_timing_frame = true;
last_timing_frame_time_ms_ = encoded_image.capture_time_ms_;
}
// Outliers trigger timing frames, but do not affect scheduled timing
// frames.
if (outlier_frame_size && encoded_image._length >= *outlier_frame_size) {
- if (timing_flags == TimingFrameFlags::kInvalid)
- timing_flags = 0;
- timing_flags |= TimingFrameFlags::kTriggeredBySize;
+ is_timing_frame = true;
}
}
@@ -292,10 +290,9 @@
// drift relative to rtc::TimeMillis(). We can't use it for Timing frames,
// because to being sent in the network capture time required to be less than
// all the other timestamps.
- if (timing_flags != TimingFrameFlags::kInvalid && encode_start_ms) {
+ if (is_timing_frame && encode_start_ms) {
encoded_image.SetEncodeTime(*encode_start_ms, rtc::TimeMillis());
}
- encoded_image.timing_.flags = timing_flags;
Result result = post_encode_callback_->OnEncodedImage(
encoded_image, codec_specific, fragmentation_header);
diff --git a/webrtc/modules/video_coding/generic_encoder_unittest.cc b/webrtc/modules/video_coding/generic_encoder_unittest.cc
index f3ca088..eec5b98 100644
--- a/webrtc/modules/video_coding/generic_encoder_unittest.cc
+++ b/webrtc/modules/video_coding/generic_encoder_unittest.cc
@@ -31,8 +31,7 @@
Result OnEncodedImage(const EncodedImage& encoded_image,
const CodecSpecificInfo* codec_specific_info,
const RTPFragmentationHeader* fragmentation) override {
- last_frame_was_timing_ =
- encoded_image.timing_.flags != TimingFrameFlags::kInvalid;
+ last_frame_was_timing_ = encoded_image.timing_.is_timing_frame;
return Result(Result::OK);
};