From 19084f89d2ced9926df06fafe122d9ea5595df84 Mon Sep 17 00:00:00 2001 From: Elad Alon Date: Fri, 9 Nov 2018 21:55:10 +0100 Subject: [PATCH] Event logs - encode N channels as N-1 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Since the number of channels is always greater than 0, smaller deltas can be accomplished by encoding a sequence of (1, 2, 1) as if the sequence were (0, 1, 0). This way, wrap around to the first value is a delta of 1, rahter than a delta of 3. For simplicity's sake, though at the cost of consistency, we still encode the base event's number of channels unshifted. We do so because there are no bits to be gained by doing it otherwise, and the value there is more likely to be manually inspected, than are the deltas, so a simpler scheme has merit. Bug: webrtc:8111 Change-Id: I2d4def67da85c42802fe13cd0494fdd9f2b38f7a Reviewed-on: https://webrtc-review.googlesource.com/c/110242 Commit-Queue: Elad Alon Reviewed-by: Björn Terelius Cr-Commit-Position: refs/heads/master@{#25601} --- .../rtc_event_log_encoder_new_format.cc | 25 +++++++++++++++++-- .../rtc_event_log/rtc_event_log_parser_new.cc | 24 +++++++++++++----- 2 files changed, 41 insertions(+), 8 deletions(-) diff --git a/logging/rtc_event_log/encoder/rtc_event_log_encoder_new_format.cc b/logging/rtc_event_log/encoder/rtc_event_log_encoder_new_format.cc index 8893759800..ee470fd5f5 100644 --- a/logging/rtc_event_log/encoder/rtc_event_log_encoder_new_format.cc +++ b/logging/rtc_event_log/encoder/rtc_event_log_encoder_new_format.cc @@ -845,6 +845,9 @@ void RtcEventLogEncoderNewFormat::EncodeAudioNetworkAdaptation( proto_batch->set_enable_fec(base_event->config().enable_fec.value()); if (base_event->config().enable_dtx.has_value()) proto_batch->set_enable_dtx(base_event->config().enable_dtx.value()); + // Note that |num_channels_deltas| encodes N as N-1, to keep deltas smaller, + // but there's no reason to do the same for the base event's value, since + // no bits will be spared. if (base_event->config().num_channels.has_value()) proto_batch->set_num_channels(base_event->config().num_channels.value()); @@ -940,9 +943,27 @@ void RtcEventLogEncoderNewFormat::EncodeAudioNetworkAdaptation( // num_channels for (size_t i = 0; i < values.size(); ++i) { const RtcEventAudioNetworkAdaptation* event = batch[i + 1]; - values[i] = event->config().num_channels; + const absl::optional num_channels = event->config().num_channels; + if (num_channels.has_value()) { + // Since the number of channels is always greater than 0, we can encode + // N channels as N-1, thereby making sure that we get smaller deltas. + // That is, a toggle of 1->2->1 can be encoded as deltas vector (1, 1), + // rather than as (1, 3) or (1, -1), either of which would require two + // bits per delta. + RTC_DCHECK_GT(num_channels.value(), 0u); + values[i] = num_channels.value() - 1; + } else { + values[i].reset(); + } } - encoded_deltas = EncodeDeltas(base_event->config().num_channels, values); + // In the base event, N channels encoded as N channels, but for delta + // compression purposes, also shifted down by 1. + absl::optional shifted_base_num_channels; + if (base_event->config().num_channels.has_value()) { + RTC_DCHECK_GT(base_event->config().num_channels.value(), 0u); + shifted_base_num_channels = base_event->config().num_channels.value() - 1; + } + encoded_deltas = EncodeDeltas(shifted_base_num_channels, values); if (!encoded_deltas.empty()) { proto_batch->set_num_channels_deltas(encoded_deltas); } diff --git a/logging/rtc_event_log/rtc_event_log_parser_new.cc b/logging/rtc_event_log/rtc_event_log_parser_new.cc index 78e2746503..131a0a7cb4 100644 --- a/logging/rtc_event_log/rtc_event_log_parser_new.cc +++ b/logging/rtc_event_log/rtc_event_log_parser_new.cc @@ -2323,7 +2323,7 @@ void ParsedRtcEventLogNew::StoreAudioNetworkAdaptationEvent( runtime_config.enable_dtx = proto.enable_dtx(); } if (proto.has_num_channels()) { - // TODO(eladalon): Encode 1/2 -> 0/1, to improve + // Note: Encoding N as N-1 only done for |num_channels_deltas|. runtime_config.num_channels = proto.num_channels(); } audio_network_adaptation_events_.emplace_back(1000 * proto.timestamp_ms(), @@ -2387,11 +2387,23 @@ void ParsedRtcEventLogNew::StoreAudioNetworkAdaptationEvent( RTC_CHECK_EQ(enable_dtx_values.size(), number_of_deltas); // num_channels - const absl::optional num_channels = - proto.has_num_channels() ? absl::optional(proto.num_channels()) - : absl::optional(); - std::vector> num_channels_values = - DecodeDeltas(proto.num_channels_deltas(), num_channels, number_of_deltas); + // Note: For delta encoding, all num_channel values, including the base, + // were shifted down by one, but in the base event, they were not. + // We likewise shift the base event down by one, to get the same base as + // encoding had, but then shift all of the values (except the base) back up + // to their original value. + absl::optional shifted_base_num_channels; + if (proto.has_num_channels()) { + shifted_base_num_channels = + absl::optional(proto.num_channels() - 1); + } + std::vector> num_channels_values = DecodeDeltas( + proto.num_channels_deltas(), shifted_base_num_channels, number_of_deltas); + for (size_t i = 0; i < num_channels_values.size(); ++i) { + if (num_channels_values[i].has_value()) { + num_channels_values[i] = num_channels_values[i].value() + 1; + } + } RTC_CHECK_EQ(num_channels_values.size(), number_of_deltas); // Delta decoding