Skip to content

Commit

Permalink
Return status instead of CHECKing in event log parser.
Browse files Browse the repository at this point in the history 
This CL adds ParseStatus/ParseStatusOr classes and returns those instead
of CHECKing that the log is well formed. Some refactoring was required.

We also add a allow_incomplete_logs parameter to the parser which by
default is false. Setting it to true will make the parser log a warning
but return success for errors that typically indicate that the log has
been truncated. "Deeper" errors indicating log corruption still return
an error.

Bug: webrtc:11064
Change-Id: Id5bd6e321de07e250662ae3aaa5ef15f48db6d55
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/158746
Reviewed-by: Ivo Creusen <[email protected]>
Reviewed-by: Sebastian Jansson <[email protected]>
Commit-Queue: Björn Terelius <[email protected]>
Cr-Commit-Position: refs/heads/master@{#29679}
  • Loading branch information
Björn Terelius authored and Commit Bot committed Nov 4, 2019
1 parent cc9bf63 commit a06048a
Show file tree
Hide file tree
Showing 10 changed files with 1,008 additions and 711 deletions.
74 changes: 37 additions & 37 deletions logging/rtc_event_log/encoder/rtc_event_log_encoder_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ void RtcEventLogEncoderTest::TestRtcEventAudioNetworkAdaptation(
}

std::string encoded = encoder_->EncodeBatch(history_.begin(), history_.end());
ASSERT_TRUE(parsed_log_.ParseString(encoded));
ASSERT_TRUE(parsed_log_.ParseString(encoded).ok());
const auto& ana_configs = parsed_log_.audio_network_adaptation_events();

ASSERT_EQ(ana_configs.size(), events.size());
Expand Down Expand Up @@ -182,7 +182,7 @@ void RtcEventLogEncoderTest::TestRtpPackets() {

// Encode and parse.
std::string encoded = encoder_->EncodeBatch(history_.begin(), history_.end());
ASSERT_TRUE(parsed_log_.ParseString(encoded));
ASSERT_TRUE(parsed_log_.ParseString(encoded).ok());

// For each SSRC, make sure the RTP packets associated with it to have been
// correctly encoded and parsed.
Expand All @@ -209,7 +209,7 @@ TEST_P(RtcEventLogEncoderTest, RtcEventAlrState) {
}

std::string encoded = encoder_->EncodeBatch(history_.begin(), history_.end());
ASSERT_TRUE(parsed_log_.ParseString(encoded));
ASSERT_TRUE(parsed_log_.ParseString(encoded).ok());
const auto& alr_state_events = parsed_log_.alr_state_events();

ASSERT_EQ(alr_state_events.size(), event_count_);
Expand All @@ -230,7 +230,7 @@ TEST_P(RtcEventLogEncoderTest, RtcEventRouteChange) {
}

std::string encoded = encoder_->EncodeBatch(history_.begin(), history_.end());
ASSERT_TRUE(parsed_log_.ParseString(encoded));
ASSERT_TRUE(parsed_log_.ParseString(encoded).ok());
const auto& route_change_events = parsed_log_.route_change_events();

ASSERT_EQ(route_change_events.size(), event_count_);
Expand All @@ -252,7 +252,7 @@ TEST_P(RtcEventLogEncoderTest, RtcEventRemoteEstimate) {
}

std::string encoded = encoder_->EncodeBatch(history_.begin(), history_.end());
ASSERT_TRUE(parsed_log_.ParseString(encoded));
ASSERT_TRUE(parsed_log_.ParseString(encoded).ok());
const auto& parsed_events = parsed_log_.remote_estimate_events();

ASSERT_EQ(parsed_events.size(), event_count_);
Expand Down Expand Up @@ -406,7 +406,7 @@ TEST_P(RtcEventLogEncoderTest, RtcEventAudioPlayout) {
}

std::string encoded = encoder_->EncodeBatch(history_.begin(), history_.end());
ASSERT_TRUE(parsed_log_.ParseString(encoded));
ASSERT_TRUE(parsed_log_.ParseString(encoded).ok());

const auto& parsed_playout_events_by_ssrc =
parsed_log_.audio_playout_events();
Expand Down Expand Up @@ -442,7 +442,7 @@ TEST_P(RtcEventLogEncoderTest, RtcEventAudioReceiveStreamConfig) {
history_.push_back(event->Copy());

std::string encoded = encoder_->EncodeBatch(history_.begin(), history_.end());
ASSERT_TRUE(parsed_log_.ParseString(encoded));
ASSERT_TRUE(parsed_log_.ParseString(encoded).ok());
const auto& audio_recv_configs = parsed_log_.audio_recv_configs();

ASSERT_EQ(audio_recv_configs.size(), 1u);
Expand All @@ -458,7 +458,7 @@ TEST_P(RtcEventLogEncoderTest, RtcEventAudioSendStreamConfig) {
history_.push_back(event->Copy());

std::string encoded = encoder_->EncodeBatch(history_.begin(), history_.end());
ASSERT_TRUE(parsed_log_.ParseString(encoded));
ASSERT_TRUE(parsed_log_.ParseString(encoded).ok());
const auto& audio_send_configs = parsed_log_.audio_send_configs();

ASSERT_EQ(audio_send_configs.size(), 1u);
Expand All @@ -476,7 +476,7 @@ TEST_P(RtcEventLogEncoderTest, RtcEventBweUpdateDelayBased) {
}

std::string encoded = encoder_->EncodeBatch(history_.begin(), history_.end());
ASSERT_TRUE(parsed_log_.ParseString(encoded));
ASSERT_TRUE(parsed_log_.ParseString(encoded).ok());

const auto& bwe_delay_updates = parsed_log_.bwe_delay_updates();
ASSERT_EQ(bwe_delay_updates.size(), event_count_);
Expand All @@ -496,7 +496,7 @@ TEST_P(RtcEventLogEncoderTest, RtcEventBweUpdateLossBased) {
}

std::string encoded = encoder_->EncodeBatch(history_.begin(), history_.end());
ASSERT_TRUE(parsed_log_.ParseString(encoded));
ASSERT_TRUE(parsed_log_.ParseString(encoded).ok());

const auto& bwe_loss_updates = parsed_log_.bwe_loss_updates();
ASSERT_EQ(bwe_loss_updates.size(), event_count_);
Expand All @@ -520,7 +520,7 @@ TEST_P(RtcEventLogEncoderTest, RtcEventGenericPacketReceived) {
}

std::string encoded = encoder_->EncodeBatch(history_.begin(), history_.end());
ASSERT_TRUE(parsed_log_.ParseString(encoded));
ASSERT_TRUE(parsed_log_.ParseString(encoded).ok());

const auto& packets_received = parsed_log_.generic_packets_received();
ASSERT_EQ(packets_received.size(), event_count_);
Expand All @@ -544,7 +544,7 @@ TEST_P(RtcEventLogEncoderTest, RtcEventGenericPacketSent) {
}

std::string encoded = encoder_->EncodeBatch(history_.begin(), history_.end());
ASSERT_TRUE(parsed_log_.ParseString(encoded));
ASSERT_TRUE(parsed_log_.ParseString(encoded).ok());

const auto& packets_sent = parsed_log_.generic_packets_sent();
ASSERT_EQ(packets_sent.size(), event_count_);
Expand All @@ -567,7 +567,7 @@ TEST_P(RtcEventLogEncoderTest, RtcEventGenericAcksReceived) {
}

std::string encoded = encoder_->EncodeBatch(history_.begin(), history_.end());
ASSERT_TRUE(parsed_log_.ParseString(encoded));
ASSERT_TRUE(parsed_log_.ParseString(encoded).ok());

const auto& decoded_events = parsed_log_.generic_acks_received();
ASSERT_EQ(decoded_events.size(), event_count_);
Expand All @@ -588,7 +588,7 @@ TEST_P(RtcEventLogEncoderTest, RtcEventDtlsTransportState) {

const std::string encoded =
encoder_->EncodeBatch(history_.begin(), history_.end());
ASSERT_TRUE(parsed_log_.ParseString(encoded));
ASSERT_TRUE(parsed_log_.ParseString(encoded).ok());

const auto& dtls_transport_states = parsed_log_.dtls_transport_states();
if (!new_encoding_) {
Expand All @@ -614,7 +614,7 @@ TEST_P(RtcEventLogEncoderTest, RtcEventDtlsWritableState) {

const std::string encoded =
encoder_->EncodeBatch(history_.begin(), history_.end());
ASSERT_TRUE(parsed_log_.ParseString(encoded));
ASSERT_TRUE(parsed_log_.ParseString(encoded).ok());

const auto& dtls_writable_states = parsed_log_.dtls_writable_states();
if (!new_encoding_) {
Expand All @@ -637,7 +637,7 @@ TEST_P(RtcEventLogEncoderTest, RtcEventIceCandidatePairConfig) {
history_.push_back(event->Copy());

std::string encoded = encoder_->EncodeBatch(history_.begin(), history_.end());
ASSERT_TRUE(parsed_log_.ParseString(encoded));
ASSERT_TRUE(parsed_log_.ParseString(encoded).ok());
const auto& ice_candidate_pair_configs =
parsed_log_.ice_candidate_pair_configs();

Expand All @@ -652,7 +652,7 @@ TEST_P(RtcEventLogEncoderTest, RtcEventIceCandidatePair) {
history_.push_back(event->Copy());

std::string encoded = encoder_->EncodeBatch(history_.begin(), history_.end());
ASSERT_TRUE(parsed_log_.ParseString(encoded));
ASSERT_TRUE(parsed_log_.ParseString(encoded).ok());
const auto& ice_candidate_pair_events =
parsed_log_.ice_candidate_pair_events();

Expand All @@ -665,8 +665,8 @@ TEST_P(RtcEventLogEncoderTest, RtcEventLoggingStarted) {
const int64_t timestamp_us = rtc::TimeMicros();
const int64_t utc_time_us = rtc::TimeUTCMicros();

ASSERT_TRUE(parsed_log_.ParseString(
encoder_->EncodeLogStart(timestamp_us, utc_time_us)));
std::string encoded = encoder_->EncodeLogStart(timestamp_us, utc_time_us);
ASSERT_TRUE(parsed_log_.ParseString(encoded).ok());
const auto& start_log_events = parsed_log_.start_log_events();

ASSERT_EQ(start_log_events.size(), 1u);
Expand All @@ -676,8 +676,8 @@ TEST_P(RtcEventLogEncoderTest, RtcEventLoggingStarted) {

TEST_P(RtcEventLogEncoderTest, RtcEventLoggingStopped) {
const int64_t timestamp_us = rtc::TimeMicros();

ASSERT_TRUE(parsed_log_.ParseString(encoder_->EncodeLogEnd(timestamp_us)));
std::string encoded = encoder_->EncodeLogEnd(timestamp_us);
ASSERT_TRUE(parsed_log_.ParseString(encoded).ok());
const auto& stop_log_events = parsed_log_.stop_log_events();

ASSERT_EQ(stop_log_events.size(), 1u);
Expand All @@ -691,7 +691,7 @@ TEST_P(RtcEventLogEncoderTest, RtcEventProbeClusterCreated) {
history_.push_back(event->Copy());

std::string encoded = encoder_->EncodeBatch(history_.begin(), history_.end());
ASSERT_TRUE(parsed_log_.ParseString(encoded));
ASSERT_TRUE(parsed_log_.ParseString(encoded).ok());
const auto& bwe_probe_cluster_created_events =
parsed_log_.bwe_probe_cluster_created_events();

Expand All @@ -707,7 +707,7 @@ TEST_P(RtcEventLogEncoderTest, RtcEventProbeResultFailure) {
history_.push_back(event->Copy());

std::string encoded = encoder_->EncodeBatch(history_.begin(), history_.end());
ASSERT_TRUE(parsed_log_.ParseString(encoded));
ASSERT_TRUE(parsed_log_.ParseString(encoded).ok());
const auto& bwe_probe_failure_events = parsed_log_.bwe_probe_failure_events();

ASSERT_EQ(bwe_probe_failure_events.size(), 1u);
Expand All @@ -722,7 +722,7 @@ TEST_P(RtcEventLogEncoderTest, RtcEventProbeResultSuccess) {
history_.push_back(event->Copy());

std::string encoded = encoder_->EncodeBatch(history_.begin(), history_.end());
ASSERT_TRUE(parsed_log_.ParseString(encoded));
ASSERT_TRUE(parsed_log_.ParseString(encoded).ok());
const auto& bwe_probe_success_events = parsed_log_.bwe_probe_success_events();

ASSERT_EQ(bwe_probe_success_events.size(), 1u);
Expand All @@ -746,7 +746,7 @@ TEST_P(RtcEventLogEncoderTest, RtcEventRtcpPacketIncoming) {
}

std::string encoded = encoder_->EncodeBatch(history_.begin(), history_.end());
ASSERT_TRUE(parsed_log_.ParseString(encoded));
ASSERT_TRUE(parsed_log_.ParseString(encoded).ok());

const auto& incoming_rtcp_packets = parsed_log_.incoming_rtcp_packets();
ASSERT_EQ(incoming_rtcp_packets.size(), event_count_);
Expand All @@ -767,7 +767,7 @@ TEST_P(RtcEventLogEncoderTest, RtcEventRtcpPacketOutgoing) {
}

std::string encoded = encoder_->EncodeBatch(history_.begin(), history_.end());
ASSERT_TRUE(parsed_log_.ParseString(encoded));
ASSERT_TRUE(parsed_log_.ParseString(encoded).ok());

const auto& outgoing_rtcp_packets = parsed_log_.outgoing_rtcp_packets();
ASSERT_EQ(outgoing_rtcp_packets.size(), event_count_);
Expand Down Expand Up @@ -805,7 +805,7 @@ TEST_P(RtcEventLogEncoderTest, RtcEventRtcpReceiverReport) {

std::string encoded =
encoder_->EncodeBatch(history_.begin(), history_.end());
ASSERT_TRUE(parsed_log_.ParseString(encoded));
ASSERT_TRUE(parsed_log_.ParseString(encoded).ok());

const auto& receiver_reports = parsed_log_.receiver_reports(direction);
ASSERT_EQ(receiver_reports.size(), event_count_);
Expand Down Expand Up @@ -844,7 +844,7 @@ TEST_P(RtcEventLogEncoderTest, RtcEventRtcpSenderReport) {

std::string encoded =
encoder_->EncodeBatch(history_.begin(), history_.end());
ASSERT_TRUE(parsed_log_.ParseString(encoded));
ASSERT_TRUE(parsed_log_.ParseString(encoded).ok());

const auto& sender_reports = parsed_log_.sender_reports(direction);
ASSERT_EQ(sender_reports.size(), event_count_);
Expand Down Expand Up @@ -883,7 +883,7 @@ TEST_P(RtcEventLogEncoderTest, RtcEventRtcpExtendedReports) {

std::string encoded =
encoder_->EncodeBatch(history_.begin(), history_.end());
ASSERT_TRUE(parsed_log_.ParseString(encoded));
ASSERT_TRUE(parsed_log_.ParseString(encoded).ok());

const auto& extended_reports = parsed_log_.extended_reports(direction);
ASSERT_EQ(extended_reports.size(), event_count_);
Expand Down Expand Up @@ -922,7 +922,7 @@ TEST_P(RtcEventLogEncoderTest, RtcEventRtcpFir) {

std::string encoded =
encoder_->EncodeBatch(history_.begin(), history_.end());
ASSERT_TRUE(parsed_log_.ParseString(encoded));
ASSERT_TRUE(parsed_log_.ParseString(encoded).ok());

const auto& firs = parsed_log_.firs(direction);
ASSERT_EQ(firs.size(), event_count_);
Expand Down Expand Up @@ -960,7 +960,7 @@ TEST_P(RtcEventLogEncoderTest, RtcEventRtcpPli) {

std::string encoded =
encoder_->EncodeBatch(history_.begin(), history_.end());
ASSERT_TRUE(parsed_log_.ParseString(encoded));
ASSERT_TRUE(parsed_log_.ParseString(encoded).ok());

const auto& plis = parsed_log_.plis(direction);
ASSERT_EQ(plis.size(), event_count_);
Expand Down Expand Up @@ -998,7 +998,7 @@ TEST_P(RtcEventLogEncoderTest, RtcEventRtcpNack) {

std::string encoded =
encoder_->EncodeBatch(history_.begin(), history_.end());
ASSERT_TRUE(parsed_log_.ParseString(encoded));
ASSERT_TRUE(parsed_log_.ParseString(encoded).ok());

const auto& nacks = parsed_log_.nacks(direction);
ASSERT_EQ(nacks.size(), event_count_);
Expand Down Expand Up @@ -1036,7 +1036,7 @@ TEST_P(RtcEventLogEncoderTest, RtcEventRtcpRemb) {

std::string encoded =
encoder_->EncodeBatch(history_.begin(), history_.end());
ASSERT_TRUE(parsed_log_.ParseString(encoded));
ASSERT_TRUE(parsed_log_.ParseString(encoded).ok());

const auto& rembs = parsed_log_.rembs(direction);
ASSERT_EQ(rembs.size(), event_count_);
Expand Down Expand Up @@ -1075,7 +1075,7 @@ TEST_P(RtcEventLogEncoderTest, RtcEventRtcpTransportFeedback) {

std::string encoded =
encoder_->EncodeBatch(history_.begin(), history_.end());
ASSERT_TRUE(parsed_log_.ParseString(encoded));
ASSERT_TRUE(parsed_log_.ParseString(encoded).ok());

const auto& transport_feedbacks =
parsed_log_.transport_feedbacks(direction);
Expand Down Expand Up @@ -1116,7 +1116,7 @@ TEST_P(RtcEventLogEncoderTest, RtcEventRtcpLossNotification) {

std::string encoded =
encoder_->EncodeBatch(history_.begin(), history_.end());
ASSERT_TRUE(parsed_log_.ParseString(encoded));
ASSERT_TRUE(parsed_log_.ParseString(encoded).ok());

const auto& loss_notifications = parsed_log_.loss_notifications(direction);
ASSERT_EQ(loss_notifications.size(), event_count_);
Expand Down Expand Up @@ -1145,7 +1145,7 @@ TEST_P(RtcEventLogEncoderTest, RtcEventVideoReceiveStreamConfig) {
history_.push_back(event->Copy());

std::string encoded = encoder_->EncodeBatch(history_.begin(), history_.end());
ASSERT_TRUE(parsed_log_.ParseString(encoded));
ASSERT_TRUE(parsed_log_.ParseString(encoded).ok());
const auto& video_recv_configs = parsed_log_.video_recv_configs();

ASSERT_EQ(video_recv_configs.size(), 1u);
Expand All @@ -1161,7 +1161,7 @@ TEST_P(RtcEventLogEncoderTest, RtcEventVideoSendStreamConfig) {
history_.push_back(event->Copy());

std::string encoded = encoder_->EncodeBatch(history_.begin(), history_.end());
ASSERT_TRUE(parsed_log_.ParseString(encoded));
ASSERT_TRUE(parsed_log_.ParseString(encoded).ok());
const auto& video_send_configs = parsed_log_.video_send_configs();

ASSERT_EQ(video_send_configs.size(), 1u);
Expand Down
13 changes: 13 additions & 0 deletions logging/rtc_event_log/logged_events.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,4 +39,17 @@ LoggedPacketInfo::LoggedPacketInfo(const LoggedRtpPacket& rtp,
LoggedPacketInfo::LoggedPacketInfo(const LoggedPacketInfo&) = default;

LoggedPacketInfo::~LoggedPacketInfo() {}

LoggedRtcpPacket::LoggedRtcpPacket(int64_t timestamp_us,
const uint8_t* packet,
size_t total_length)
: timestamp_us(timestamp_us), raw_data(packet, packet + total_length) {}
LoggedRtcpPacket::LoggedRtcpPacket(int64_t timestamp_us,
const std::string& packet)
: timestamp_us(timestamp_us), raw_data(packet.size()) {
memcpy(raw_data.data(), packet.data(), packet.size());
}
LoggedRtcpPacket::LoggedRtcpPacket(const LoggedRtcpPacket& rhs) = default;
LoggedRtcpPacket::~LoggedRtcpPacket() = default;

} // namespace webrtc
14 changes: 7 additions & 7 deletions logging/rtc_event_log/logged_events.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -251,7 +251,7 @@ struct LoggedRemoteEstimateEvent {
};

struct LoggedRtpPacket {
LoggedRtpPacket(uint64_t timestamp_us,
LoggedRtpPacket(int64_t timestamp_us,
RTPHeader header,
size_t header_length,
size_t total_length)
Expand All @@ -271,7 +271,7 @@ struct LoggedRtpPacket {
};

struct LoggedRtpPacketIncoming {
LoggedRtpPacketIncoming(uint64_t timestamp_us,
LoggedRtpPacketIncoming(int64_t timestamp_us,
RTPHeader header,
size_t header_length,
size_t total_length)
Expand All @@ -283,7 +283,7 @@ struct LoggedRtpPacketIncoming {
};

struct LoggedRtpPacketOutgoing {
LoggedRtpPacketOutgoing(uint64_t timestamp_us,
LoggedRtpPacketOutgoing(int64_t timestamp_us,
RTPHeader header,
size_t header_length,
size_t total_length)
Expand All @@ -295,10 +295,10 @@ struct LoggedRtpPacketOutgoing {
};

struct LoggedRtcpPacket {
LoggedRtcpPacket(uint64_t timestamp_us,
LoggedRtcpPacket(int64_t timestamp_us,
const uint8_t* packet,
size_t total_length);
LoggedRtcpPacket(uint64_t timestamp_us, const std::string& packet);
LoggedRtcpPacket(int64_t timestamp_us, const std::string& packet);
LoggedRtcpPacket(const LoggedRtcpPacket&);
~LoggedRtcpPacket();

Expand All @@ -310,7 +310,7 @@ struct LoggedRtcpPacket {
};

struct LoggedRtcpPacketIncoming {
LoggedRtcpPacketIncoming(uint64_t timestamp_us,
LoggedRtcpPacketIncoming(int64_t timestamp_us,
const uint8_t* packet,
size_t total_length)
: rtcp(timestamp_us, packet, total_length) {}
Expand All @@ -324,7 +324,7 @@ struct LoggedRtcpPacketIncoming {
};

struct LoggedRtcpPacketOutgoing {
LoggedRtcpPacketOutgoing(uint64_t timestamp_us,
LoggedRtcpPacketOutgoing(int64_t timestamp_us,
const uint8_t* packet,
size_t total_length)
: rtcp(timestamp_us, packet, total_length) {}
Expand Down
Loading

0 comments on commit a06048a

Please sign in to comment.