Event logs - encode N channels as N-1
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 <eladalon@webrtc.org>
Reviewed-by: Björn Terelius <terelius@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#25601}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 8893759..ee470fd 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 @@
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 @@
// 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<size_t> 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<size_t> 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 78e2746..131a0a7 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 @@
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 @@
RTC_CHECK_EQ(enable_dtx_values.size(), number_of_deltas);
// num_channels
- const absl::optional<uint64_t> num_channels =
- proto.has_num_channels() ? absl::optional<uint64_t>(proto.num_channels())
- : absl::optional<uint64_t>();
- std::vector<absl::optional<uint64_t>> 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<uint64_t> shifted_base_num_channels;
+ if (proto.has_num_channels()) {
+ shifted_base_num_channels =
+ absl::optional<uint64_t>(proto.num_channels() - 1);
+ }
+ std::vector<absl::optional<uint64_t>> 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
