Cleanup TODOs from @miu

This patch removes all remaining TODOs from the now-departed @miu,
either reassigning them to jophba, or for the majority, just
implementing the fix directly.

Change-Id: I73c21f577bf115cf37ec880fac54eef1555de7c5
Reviewed-on: https://chromium-review.googlesource.com/c/openscreen/+/2831302
Commit-Queue: Jordan Bayles <jophba@chromium.org>
Reviewed-by: Brandon Tolsch <btolsch@chromium.org>

25 files changed, 140 insertions, 245 deletions

diff --git a/cast/common/channel/message_util.cc b/cast/common/channel/message_util.cc
index 92ea5007..f7f790bf 100644
--- a/cast/common/channel/message_util.cc
+++ b/cast/common/channel/message_util.cc
@@ -162,5 +162,12 @@ std::string MakeUniqueSessionId(const char* prefix) {
   return oss.str();
 }
 
+bool HasType(const Json::Value& object, CastMessageType type) {
+  OSP_DCHECK(object.isObject());
+  const Json::Value& value =
+      object.get(kMessageKeyType, Json::Value::nullSingleton());
+  return value.isString() && value.asString() == CastMessageTypeToString(type);
+}
+
 }  // namespace cast
 }  // namespace openscreen
diff --git a/cast/common/channel/message_util.h b/cast/common/channel/message_util.h
index 8e8fe823..418e7ad6 100644
--- a/cast/common/channel/message_util.h
+++ b/cast/common/channel/message_util.h
@@ -10,6 +10,10 @@
 #include "absl/strings/string_view.h"
 #include "cast/common/channel/proto/cast_channel.pb.h"
 
+namespace Json {
+class Value;
+}
+
 namespace openscreen {
 namespace cast {
 
@@ -242,6 +246,8 @@ inline bool IsTransportNamespace(absl::string_view namespace_) {
 // |prefix| of "sender" will result in a string like "sender-12345".
 std::string MakeUniqueSessionId(const char* prefix);
 
+// Returns true if the type field in |object| is set to the given |type|.
+bool HasType(const Json::Value& object, CastMessageType type);
 }  // namespace cast
 }  // namespace openscreen
 
diff --git a/cast/common/discovery/e2e_test/tests.cc b/cast/common/discovery/e2e_test/tests.cc
index 7c294418..82234845 100644
--- a/cast/common/discovery/e2e_test/tests.cc
+++ b/cast/common/discovery/e2e_test/tests.cc
@@ -125,16 +125,7 @@ discovery::Config GetConfigSettings() {
   // Get the loopback interface to run on.
   InterfaceInfo loopback = GetLoopbackInterfaceForTesting().value();
   OSP_LOG_INFO << "Selected network interface for testing: " << loopback;
-  discovery::Config::NetworkInfo::AddressFamilies address_families =
-      discovery::Config::NetworkInfo::kNoAddressFamily;
-  if (loopback.GetIpAddressV4()) {
-    address_families |= discovery::Config::NetworkInfo::kUseIpV4;
-  }
-  if (loopback.GetIpAddressV6()) {
-    address_families |= discovery::Config::NetworkInfo::kUseIpV6;
-  }
-
-  return discovery::Config{{{std::move(loopback), address_families}}};
+  return discovery::Config{{std::move(loopback)}};
 }
 
 class DiscoveryE2ETest : public testing::Test {
diff --git a/cast/receiver/application_agent.cc b/cast/receiver/application_agent.cc
index df2b49e6..d5665391 100644
--- a/cast/receiver/application_agent.cc
+++ b/cast/receiver/application_agent.cc
@@ -11,6 +11,7 @@
 #include "cast/common/public/cast_socket.h"
 #include "platform/base/tls_credentials.h"
 #include "platform/base/tls_listen_options.h"
+#include "util/json/json_helpers.h"
 #include "util/json/json_serialization.h"
 #include "util/osp_logging.h"
 
@@ -18,24 +19,6 @@ namespace openscreen {
 namespace cast {
 namespace {
 
-// Parses the given string as a JSON object. If the parse fails, an empty object
-// is returned.
-Json::Value ParseAsObject(absl::string_view value) {
-  ErrorOr<Json::Value> parsed = json::Parse(value);
-  if (parsed.is_value() && parsed.value().isObject()) {
-    return std::move(parsed.value());
-  }
-  return Json::Value(Json::objectValue);
-}
-
-// Returns true if the type field in |object| is set to the given |type|.
-bool HasType(const Json::Value& object, CastMessageType type) {
-  OSP_DCHECK(object.isObject());
-  const Json::Value& value =
-      object.get(kMessageKeyType, Json::Value::nullSingleton());
-  return value.isString() && value.asString() == CastMessageTypeToString(type);
-}
-
 // Returns the first app ID for the given |app|, or the empty string if there is
 // none.
 std::string GetFirstAppId(ApplicationAgent::Application* app) {
@@ -142,25 +125,29 @@ void ApplicationAgent::OnMessage(VirtualConnectionRouter* router,
     return;
   }
 
-  const Json::Value request = ParseAsObject(message.payload_utf8());
+  const ErrorOr<Json::Value> request = json::Parse(message.payload_utf8());
+  if (request.is_error() || request.value().type() != Json::objectValue) {
+    return;
+  }
+
   Json::Value response;
   if (ns == kHeartbeatNamespace) {
-    if (HasType(request, CastMessageType::kPing)) {
+    if (HasType(request.value(), CastMessageType::kPing)) {
       response = HandlePing();
     }
   } else if (ns == kReceiverNamespace) {
-    if (request[kMessageKeyRequestId].isNull()) {
-      response = HandleInvalidCommand(request);
-    } else if (HasType(request, CastMessageType::kGetAppAvailability)) {
-      response = HandleGetAppAvailability(request);
-    } else if (HasType(request, CastMessageType::kGetStatus)) {
-      response = HandleGetStatus(request);
-    } else if (HasType(request, CastMessageType::kLaunch)) {
-      response = HandleLaunch(request, socket);
-    } else if (HasType(request, CastMessageType::kStop)) {
-      response = HandleStop(request);
+    if (request.value()[kMessageKeyRequestId].isNull()) {
+      response = HandleInvalidCommand(request.value());
+    } else if (HasType(request.value(), CastMessageType::kGetAppAvailability)) {
+      response = HandleGetAppAvailability(request.value());
+    } else if (HasType(request.value(), CastMessageType::kGetStatus)) {
+      response = HandleGetStatus(request.value());
+    } else if (HasType(request.value(), CastMessageType::kLaunch)) {
+      response = HandleLaunch(request.value(), socket);
+    } else if (HasType(request.value(), CastMessageType::kStop)) {
+      response = HandleStop(request.value());
     } else {
-      response = HandleInvalidCommand(request);
+      response = HandleInvalidCommand(request.value());
     }
   } else {
     // Ignore messages for all other namespaces.
diff --git a/cast/standalone_receiver/cast_service.cc b/cast/standalone_receiver/cast_service.cc
index 75790197..efab229b 100644
--- a/cast/standalone_receiver/cast_service.cc
+++ b/cast/standalone_receiver/cast_service.cc
@@ -32,18 +32,7 @@ IPEndpoint DetermineEndpoint(const InterfaceInfo& interface) {
 }
 
 discovery::Config MakeDiscoveryConfig(const InterfaceInfo& interface) {
-  discovery::Config config;
-
-  discovery::Config::NetworkInfo::AddressFamilies supported_address_families =
-      discovery::Config::NetworkInfo::kNoAddressFamily;
-  if (interface.GetIpAddressV4()) {
-    supported_address_families |= discovery::Config::NetworkInfo::kUseIpV4;
-  } else if (interface.GetIpAddressV6()) {
-    supported_address_families |= discovery::Config::NetworkInfo::kUseIpV6;
-  }
-  config.network_info.push_back({interface, supported_address_families});
-
-  return config;
+  return discovery::Config{.network_info = {interface}};
 }
 
 }  // namespace
diff --git a/cast/standalone_receiver/mirroring_application.cc b/cast/standalone_receiver/mirroring_application.cc
index a04c401a..b654e010 100644
--- a/cast/standalone_receiver/mirroring_application.cc
+++ b/cast/standalone_receiver/mirroring_application.cc
@@ -5,6 +5,7 @@
 #include "cast/standalone_receiver/mirroring_application.h"
 
 #include "cast/common/public/message_port.h"
+#include "cast/streaming/constants.h"
 #include "cast/streaming/environment.h"
 #include "cast/streaming/message_fields.h"
 #include "cast/streaming/receiver_session.h"
@@ -14,9 +15,6 @@
 namespace openscreen {
 namespace cast {
 
-const char kMirroringAppId[] = "0F5096E8";
-const char kMirroringAudioOnlyAppId[] = "85CDB22F";
-
 const char kMirroringDisplayName[] = "Chrome Mirroring";
 const char kRemotingRpcNamespace[] = "urn:x-cast:com.google.cast.remoting";
 
diff --git a/cast/standalone_sender/looping_file_cast_agent.cc b/cast/standalone_sender/looping_file_cast_agent.cc
index 4608465a..5477c155 100644
--- a/cast/standalone_sender/looping_file_cast_agent.cc
+++ b/cast/standalone_sender/looping_file_cast_agent.cc
@@ -15,6 +15,7 @@
 #include "cast/streaming/offer_messages.h"
 #include "json/value.h"
 #include "platform/api/tls_connection_factory.h"
+#include "util/json/json_helpers.h"
 #include "util/stringprintf.h"
 #include "util/trace_logging.h"
 
@@ -24,45 +25,6 @@ namespace {
 
 using DeviceMediaPolicy = SenderSocketFactory::DeviceMediaPolicy;
 
-// TODO(miu): These string constants appear in a few places and should be
-// de-duped to a common location.
-constexpr char kMirroringAppId[] = "0F5096E8";
-constexpr char kMirroringAudioOnlyAppId[] = "85CDB22F";
-
-// Parses the given string as a JSON object. If the parse fails, an empty object
-// is returned.
-//
-// TODO(miu): De-dupe this code (same as in cast/receiver/application_agent.cc)!
-Json::Value ParseAsObject(absl::string_view value) {
-  ErrorOr<Json::Value> parsed = json::Parse(value);
-  if (parsed.is_value() && parsed.value().isObject()) {
-    return std::move(parsed.value());
-  }
-  return Json::Value(Json::objectValue);
-}
-
-// Returns true if the 'type' field in |object| has the given |type|.
-//
-// TODO(miu): De-dupe this code (same as in cast/receiver/application_agent.cc)!
-bool HasType(const Json::Value& object, CastMessageType type) {
-  OSP_DCHECK(object.isObject());
-  const Json::Value& value =
-      object.get(kMessageKeyType, Json::Value::nullSingleton());
-  return value.isString() && value.asString() == CastMessageTypeToString(type);
-}
-
-// Returns the string found in object[field] if possible; otherwise, returns
-// |fallback|. The fallback string is returned if |object| is not an object or
-// the |field| key does not reference a string within the object.
-std::string ExtractStringFieldValue(const Json::Value& object,
-                                    const char* field,
-                                    std::string fallback = {}) {
-  if (object.isObject() && object[field].isString()) {
-    return object[field].asString();
-  }
-  return fallback;
-}
-
 }  // namespace
 
 LoopingFileCastAgent::LoopingFileCastAgent(TaskRunner* task_runner,
@@ -161,18 +123,31 @@ void LoopingFileCastAgent::OnMessage(VirtualConnectionRouter* router,
 
   if (message.namespace_() == kReceiverNamespace &&
       message_port_.GetSocketId() == ToCastSocketId(socket)) {
-    const Json::Value payload = ParseAsObject(message.payload_utf8());
-    if (HasType(payload, CastMessageType::kReceiverStatus)) {
-      HandleReceiverStatus(payload);
-    } else if (HasType(payload, CastMessageType::kLaunchError)) {
+    const ErrorOr<Json::Value> payload = json::Parse(message.payload_utf8());
+    if (payload.is_error()) {
+      OSP_LOG_ERROR << "Failed to parse message: " << payload.error();
+    }
+
+    if (HasType(payload.value(), CastMessageType::kReceiverStatus)) {
+      HandleReceiverStatus(payload.value());
+    } else if (HasType(payload.value(), CastMessageType::kLaunchError)) {
+      std::string reason;
+      if (!json::ParseAndValidateString(payload.value()[kMessageKeyReason],
+                                        &reason)) {
+        reason = "UNKNOWN";
+      }
       OSP_LOG_ERROR
           << "Failed to launch the Cast Mirroring App on the Receiver! Reason: "
-          << ExtractStringFieldValue(payload, kMessageKeyReason, "UNKNOWN");
+          << reason;
       Shutdown();
-    } else if (HasType(payload, CastMessageType::kInvalidRequest)) {
+    } else if (HasType(payload.value(), CastMessageType::kInvalidRequest)) {
+      std::string reason;
+      if (!json::ParseAndValidateString(payload.value()[kMessageKeyReason],
+                                        &reason)) {
+        reason = "UNKNOWN";
+      }
       OSP_LOG_ERROR << "Cast Receiver thinks our request is invalid: "
-                    << ExtractStringFieldValue(payload, kMessageKeyReason,
-                                               "UNKNOWN");
+                    << reason;
     }
   }
 }
@@ -191,9 +166,10 @@ void LoopingFileCastAgent::HandleReceiverStatus(const Json::Value& status) {
           ? status[kMessageKeyStatus][kMessageKeyApplications][0]
           : Json::Value();
 
-  const std::string& running_app_id =
-      ExtractStringFieldValue(details, kMessageKeyAppId);
-  if (running_app_id != GetMirroringAppId()) {
+  std::string running_app_id;
+  if (!json::ParseAndValidateString(details[kMessageKeyAppId],
+                                    &running_app_id) ||
+      running_app_id != GetMirroringAppId()) {
     // The mirroring app is not running. If it was just stopped, Shutdown() will
     // tear everything down. If it has been stopped already, Shutdown() is a
     // no-op.
@@ -201,9 +177,10 @@ void LoopingFileCastAgent::HandleReceiverStatus(const Json::Value& status) {
     return;
   }
 
-  const std::string& session_id =
-      ExtractStringFieldValue(details, kMessageKeySessionId);
-  if (session_id.empty()) {
+  std::string session_id;
+  if (!json::ParseAndValidateString(details[kMessageKeySessionId],
+                                    &session_id) ||
+      session_id.empty()) {
     OSP_LOG_ERROR
         << "Cannot continue: Cast Receiver did not provide a session ID for "
            "the Mirroring App running on it.";
@@ -229,9 +206,10 @@ void LoopingFileCastAgent::HandleReceiverStatus(const Json::Value& status) {
     return;
   }
 
-  const std::string& message_destination_id =
-      ExtractStringFieldValue(details, kMessageKeyTransportId);
-  if (message_destination_id.empty()) {
+  std::string message_destination_id;
+  if (!json::ParseAndValidateString(details[kMessageKeyTransportId],
+                                    &message_destination_id) ||
+      message_destination_id.empty()) {
     OSP_LOG_ERROR
         << "Cannot continue: Cast Receiver did not provide a transport ID for "
            "routing messages to the Mirroring App running on it.";
@@ -291,7 +269,7 @@ void LoopingFileCastAgent::CreateAndStartSession() {
   video_config.max_bit_rate =
       connection_settings_->max_bitrate - audio_config.bit_rate;
   // Use default display resolution of 1080P.
-  video_config.resolutions.emplace_back(Resolution{});
+  video_config.resolutions.emplace_back(Resolution{1920, 1080});
 
   OSP_VLOG << "Starting session negotiation.";
   const Error negotiation_error =
diff --git a/cast/standalone_sender/looping_file_sender.cc b/cast/standalone_sender/looping_file_sender.cc
index 9fae8439..c66722f5 100644
--- a/cast/standalone_sender/looping_file_sender.cc
+++ b/cast/standalone_sender/looping_file_sender.cc
@@ -125,7 +125,7 @@ void LoopingFileSender::OnVideoFrame(const AVFrame& av_frame,
   for (int i = 0; i < 3; ++i) {
     frame.yuv_strides[i] = av_frame.linesize[i];
   }
-  // TODO(miu): Add performance metrics visual overlay (based on Stats
+  // TODO(jophba): Add performance metrics visual overlay (based on Stats
   // callback).
   video_encoder_.EncodeAndSend(frame, capture_time, {});
 }
diff --git a/cast/standalone_sender/receiver_chooser.cc b/cast/standalone_sender/receiver_chooser.cc
index 828ea8ef..8f61326c 100644
--- a/cast/standalone_sender/receiver_chooser.cc
+++ b/cast/standalone_sender/receiver_chooser.cc
@@ -27,24 +27,10 @@ ReceiverChooser::ReceiverChooser(const InterfaceInfo& interface,
                                  ResultCallback result_callback)
     : result_callback_(std::move(result_callback)),
       menu_alarm_(&Clock::now, task_runner) {
-  using discovery::Config;
-  Config config;
-  // TODO(miu): Remove AddressFamilies from the Config in a follow-up patch. No
-  // client uses this to do anything other than "enabled for all address
-  // families," and so it doesn't need to be configurable.
-  Config::NetworkInfo::AddressFamilies families =
-      Config::NetworkInfo::kNoAddressFamily;
-  if (interface.GetIpAddressV4()) {
-    families |= Config::NetworkInfo::kUseIpV4;
-  }
-  if (interface.GetIpAddressV6()) {
-    families |= Config::NetworkInfo::kUseIpV6;
-  }
-  config.network_info.push_back({interface, families});
-  config.enable_publication = false;
-  config.enable_querying = true;
-  service_ =
-      discovery::CreateDnsSdService(task_runner, this, std::move(config));
+  discovery::Config config{.network_info = {interface},
+                           .enable_publication = false,
+                           .enable_querying = true};
+  discovery::CreateDnsSdService(task_runner, this, std::move(config));
 
   watcher_ = std::make_unique<discovery::DnsSdServiceWatcher<ServiceInfo>>(
       service_.get(), kCastV2ServiceId, DnsSdInstanceEndpointToServiceInfo,
diff --git a/cast/standalone_sender/streaming_vp8_encoder.cc b/cast/standalone_sender/streaming_vp8_encoder.cc
index 8b8e18dc..7cc10e35 100644
--- a/cast/standalone_sender/streaming_vp8_encoder.cc
+++ b/cast/standalone_sender/streaming_vp8_encoder.cc
@@ -138,7 +138,7 @@ void StreamingVp8Encoder::EncodeAndSend(
     std::function<void(Stats)> stats_callback) {
   WorkUnit work_unit;
 
-  // TODO(miu): The |VideoFrame| struct should provide the media timestamp,
+  // TODO(jophba): The |VideoFrame| struct should provide the media timestamp,
   // instead of this code inferring it from the reference timestamps, since: 1)
   // the video capturer's clock may tick at a different rate than the system
   // clock; and 2) to reduce jitter.
diff --git a/cast/streaming/constants.h b/cast/streaming/constants.h
index 2edbde0c..bfcc2365 100644
--- a/cast/streaming/constants.h
+++ b/cast/streaming/constants.h
@@ -17,6 +17,12 @@
 namespace openscreen {
 namespace cast {
 
+// Mirroring App identifier.
+constexpr char kMirroringAppId[] = "0F5096E8";
+
+// Mirroring App identifier for audio-only mirroring.
+constexpr char kMirroringAudioOnlyAppId[] = "85CDB22F";
+
 // Default target playout delay. The playout delay is the window of time between
 // capture from the source until presentation at the receiver.
 constexpr std::chrono::milliseconds kDefaultTargetPlayoutDelay(400);
diff --git a/cast/streaming/receiver.cc b/cast/streaming/receiver.cc
index 0d3358b4..271904f5 100644
--- a/cast/streaming/receiver.cc
+++ b/cast/streaming/receiver.cc
@@ -14,6 +14,7 @@
 #include "util/chrono_helpers.h"
 #include "util/osp_logging.h"
 #include "util/std_util.h"
+#include "util/trace_logging.h"
 
 namespace openscreen {
 namespace cast {
@@ -22,10 +23,7 @@ namespace cast {
 // to help distinguish one out of multiple instances in a Cast Streaming
 // session.
 //
-// TODO(miu): Replace RECEIVER_VLOG's with trace event logging once the tracing
-// infrastructure is ready.
 #define RECEIVER_LOG(level) OSP_LOG_##level << "[SSRC:" << ssrc() << "] "
-#define RECEIVER_VLOG OSP_VLOG << "[SSRC:" << ssrc() << "] "
 
 Receiver::Receiver(Environment* environment,
                    ReceiverPacketRouter* packet_router,
@@ -85,6 +83,7 @@ void Receiver::RequestKeyFrame() {
 }
 
 int Receiver::AdvanceToNextFrame() {
+  TRACE_DEFAULT_SCOPED(TraceCategory::kReceiver);
   const FrameId immediate_next_frame = last_frame_consumed_ + 1;
 
   // Scan the queue for the next frame that should be consumed. Typically, this
@@ -96,13 +95,13 @@ int Receiver::AdvanceToNextFrame() {
       const EncryptedFrame& encrypted_frame =
           entry.collector.PeekAtAssembledFrame();
       if (f == immediate_next_frame) {  // Typical case.
-        RECEIVER_VLOG << "AdvanceToNextFrame: Next in sequence (" << f << ')';
+        OSP_DVLOG << "AdvanceToNextFrame: Next in sequence (" << f << ')';
         return FrameCrypto::GetPlaintextSize(encrypted_frame);
       }
       if (encrypted_frame.dependency != EncodedFrame::DEPENDS_ON_ANOTHER) {
         // Found a frame after skipping past some frames. Drop the ones being
         // skipped, advancing |last_frame_consumed_| before returning.
-        RECEIVER_VLOG << "AdvanceToNextFrame: Skipping-ahead → " << f;
+        OSP_DVLOG << "AdvanceToNextFrame: Skipping-ahead → " << f;
         DropAllFramesBefore(f);
         return FrameCrypto::GetPlaintextSize(encrypted_frame);
       }
@@ -130,12 +129,13 @@ int Receiver::AdvanceToNextFrame() {
     }
   }
 
-  RECEIVER_VLOG << "AdvanceToNextFrame: No frames ready. Last consumed was "
-                << last_frame_consumed_ << '.';
+  OSP_DVLOG << "AdvanceToNextFrame: No frames ready. Last consumed was "
+            << last_frame_consumed_ << '.';
   return kNoFramesReady;
 }
 
 EncodedFrame Receiver::ConsumeNextFrame(absl::Span<uint8_t> buffer) {
+  TRACE_DEFAULT_SCOPED(TraceCategory::kReceiver);
   // Assumption: The required call to AdvanceToNextFrame() ensures that
   // |last_frame_consumed_| is set to one before the frame to be consumed here.
   const FrameId frame_id = last_frame_consumed_ + 1;
@@ -151,14 +151,14 @@ EncodedFrame Receiver::ConsumeNextFrame(absl::Span<uint8_t> buffer) {
   frame.reference_time =
       *entry.estimated_capture_time + ResolveTargetPlayoutDelay(frame_id);
 
-  RECEIVER_VLOG << "ConsumeNextFrame → " << frame.frame_id << ": "
-                << frame.data.size() << " payload bytes, RTP Timestamp "
-                << frame.rtp_timestamp
-                       .ToTimeSinceOrigin<microseconds>(rtp_timebase_)
-                       .count()
-                << " µs, to play-out "
-                << to_microseconds(frame.reference_time - now_()).count()
-                << " µs from now.";
+  OSP_DVLOG << "ConsumeNextFrame → " << frame.frame_id << ": "
+            << frame.data.size() << " payload bytes, RTP Timestamp "
+            << frame.rtp_timestamp
+                   .ToTimeSinceOrigin<microseconds>(rtp_timebase_)
+                   .count()
+            << " µs, to play-out "
+            << to_microseconds(frame.reference_time - now_()).count()
+            << " µs from now.";
 
   entry.Reset();
   last_frame_consumed_ = frame_id;
@@ -195,8 +195,8 @@ void Receiver::OnReceivedRtpPacket(Clock::time_point arrival_time,
     const FrameId max_allowed_frame_id =
         last_frame_consumed_ + kMaxUnackedFrames;
     if (part->frame_id > max_allowed_frame_id) {
-      RECEIVER_VLOG << "Dropping RTP packet for " << part->frame_id
-                    << ": Too many frames are already in-flight.";
+      OSP_DVLOG << "Dropping RTP packet for " << part->frame_id
+                << ": Too many frames are already in-flight.";
       return;
     }
     do {
@@ -204,8 +204,7 @@ void Receiver::OnReceivedRtpPacket(Clock::time_point arrival_time,
       GetQueueEntry(latest_frame_expected_)
           .collector.set_frame_id(latest_frame_expected_);
     } while (latest_frame_expected_ < part->frame_id);
-    RECEIVER_VLOG << "Advanced latest frame expected to "
-                  << latest_frame_expected_;
+    OSP_DVLOG << "Advanced latest frame expected to " << latest_frame_expected_;
   }
 
   // Start-up edge case: Blatantly drop the first packet of all frames until the
@@ -253,9 +252,9 @@ void Receiver::OnReceivedRtpPacket(Clock::time_point arrival_time,
 
     // If a target playout delay change was included in this packet, record it.
     if (part->new_playout_delay > milliseconds::zero()) {
-      RECEIVER_VLOG << "Target playout delay changes to "
-                    << part->new_playout_delay.count() << " ms, as of "
-                    << part->frame_id;
+      OSP_DVLOG << "Target playout delay changes to "
+                << part->new_playout_delay.count() << " ms, as of "
+                << part->frame_id;
       RecordNewTargetPlayoutDelay(part->frame_id, part->new_playout_delay);
     }
 
@@ -289,6 +288,7 @@ void Receiver::OnReceivedRtpPacket(Clock::time_point arrival_time,
 
 void Receiver::OnReceivedRtcpPacket(Clock::time_point arrival_time,
                                     std::vector<uint8_t> packet) {
+  TRACE_DEFAULT_SCOPED(TraceCategory::kReceiver);
   absl::optional<SenderReportParser::SenderReportWithId> parsed_report =
       rtcp_parser_.Parse(packet);
   if (!parsed_report) {
@@ -311,10 +311,9 @@ void Receiver::OnReceivedRtcpPacket(Clock::time_point arrival_time,
   const Clock::duration measured_offset =
       arrival_time - last_sender_report_->reference_time;
   smoothed_clock_offset_.Update(arrival_time, measured_offset);
-  RECEIVER_VLOG
-      << "Received Sender Report: Local clock is ahead of Sender's by "
-      << to_microseconds(smoothed_clock_offset_.Current()).count()
-      << " µs (minus one-way network transit time).";
+  OSP_DVLOG << "Received Sender Report: Local clock is ahead of Sender's by "
+            << to_microseconds(smoothed_clock_offset_.Current()).count()
+            << " µs (minus one-way network transit time).";
 
   RtcpReportBlock report;
   report.ssrc = rtcp_session_.sender_ssrc();
@@ -347,7 +346,7 @@ void Receiver::SendRtcp() {
   packet_router_->SendRtcpPacket(rtcp_builder_.BuildPacket(
       last_rtcp_send_time_,
       absl::Span<uint8_t>(rtcp_buffer_.get(), rtcp_buffer_capacity_)));
-  RECEIVER_VLOG << "Sent RTCP packet.";
+  OSP_DVLOG << "Sent RTCP packet.";
 
   // Schedule the automatic sending of another RTCP packet, if this method is
   // not called within some bounded amount of time. While incomplete frames
@@ -413,6 +412,7 @@ milliseconds Receiver::ResolveTargetPlayoutDelay(FrameId frame_id) const {
 }
 
 void Receiver::AdvanceCheckpoint(FrameId new_checkpoint) {
+  TRACE_DEFAULT_SCOPED(TraceCategory::kReceiver);
   OSP_DCHECK_GT(new_checkpoint, checkpoint_frame());
   OSP_DCHECK_LE(new_checkpoint, latest_frame_expected_);
 
@@ -424,7 +424,7 @@ void Receiver::AdvanceCheckpoint(FrameId new_checkpoint) {
     new_checkpoint = next;
   }
 
-  RECEIVER_VLOG << "Advancing checkpoint to " << new_checkpoint;
+  OSP_DVLOG << "Advancing checkpoint to " << new_checkpoint;
   set_checkpoint_frame(new_checkpoint);
   rtcp_builder_.SetPlayoutDelay(ResolveTargetPlayoutDelay(new_checkpoint));
   SendRtcp();
diff --git a/cast/streaming/receiver.h b/cast/streaming/receiver.h
index 057c56d8..66edb7d5 100644
--- a/cast/streaming/receiver.h
+++ b/cast/streaming/receiver.h
@@ -346,8 +346,8 @@ class Receiver {
   // The interval between sending ACK/NACK feedback RTCP messages while
   // incomplete frames exist in the queue.
   //
-  // TODO(miu): This should be a function of the current target playout delay,
-  // similar to the Sender's kickstart interval logic.
+  // TODO(jophba): This should be a function of the current target playout
+  // delay, similar to the Sender's kickstart interval logic.
   static constexpr std::chrono::milliseconds kNackFeedbackInterval{30};
 };
 
diff --git a/cast/streaming/rtcp_common.h b/cast/streaming/rtcp_common.h
index 25e2c2ed..5fbf5f72 100644
--- a/cast/streaming/rtcp_common.h
+++ b/cast/streaming/rtcp_common.h
@@ -161,10 +161,6 @@ struct PacketNack {
   FrameId frame_id;
   FramePacketId packet_id;
 
-  // Comparison operators. Define more when you need them!
-  // TODO(miu): In C++20, just
-  // replace all of this with one operator<=>() definition to get them all for
-  // free.
   constexpr bool operator==(const PacketNack& other) const {
     return frame_id == other.frame_id && packet_id == other.packet_id;
   }
diff --git a/cast/streaming/sender.cc b/cast/streaming/sender.cc
index ba42bcb4..fd3e3b75 100644
--- a/cast/streaming/sender.cc
+++ b/cast/streaming/sender.cc
@@ -12,6 +12,7 @@
 #include "util/chrono_helpers.h"
 #include "util/osp_logging.h"
 #include "util/std_util.h"
+#include "util/trace_logging.h"
 
 namespace openscreen {
 namespace cast {
@@ -311,10 +312,11 @@ void Sender::OnReceiverReport(const RtcpReportBlock& receiver_report) {
     round_trip_time_ =
         (kInertia * round_trip_time_ + measurement) / (kInertia + 1);
   }
-  // TODO(miu): Add tracing event here to note the updated RTT.
+  TRACE_SCOPED(TraceCategory::kSender, "UpdatedRTT");
 }
 
 void Sender::OnReceiverIndicatesPictureLoss() {
+  TRACE_DEFAULT_SCOPED(TraceCategory::kSender);
   // The Receiver will continue the PLI notifications until it has received a
   // key frame. Thus, if a key frame is already in-flight, don't make a state
   // change that would cause this Sender to force another expensive key frame.
@@ -342,6 +344,7 @@ void Sender::OnReceiverIndicatesPictureLoss() {
 
 void Sender::OnReceiverCheckpoint(FrameId frame_id,
                                   milliseconds playout_delay) {
+  TRACE_DEFAULT_SCOPED(TraceCategory::kSender);
   if (frame_id > last_enqueued_frame_id_) {
     OSP_LOG_ERROR
         << "Ignoring checkpoint for " << latest_expected_frame_id_
@@ -415,14 +418,13 @@ void Sender::OnReceiverIsMissingPackets(std::vector<PacketNack> nacks) {
     // happen in rare cases where RTCP packets arrive out-of-order (i.e., the
     // network shuffled them).
     if (!slot) {
-      // TODO(miu): Add tracing event here to record this.
+      TRACE_SCOPED(TraceCategory::kSender, "MissingNackSlot");
       for (++nack_it; nack_it != nacks.end() && nack_it->frame_id == frame_id;
            ++nack_it) {
       }
       continue;
     }
 
-    // NOLINTNEXTLINE
     latest_expected_frame_id_ = std::max(latest_expected_frame_id_, frame_id);
 
     const auto HandleIndividualNack = [&](FramePacketId packet_id) {
diff --git a/cast/streaming/session_messager.cc b/cast/streaming/session_messager.cc
index 126f7218..184da30c 100644
--- a/cast/streaming/session_messager.cc
+++ b/cast/streaming/session_messager.cc
@@ -180,6 +180,8 @@ void SenderSessionMessager::OnMessage(const std::string& source_id,
       return;
     }
 
+    OSP_DVLOG << "Received a valid reply from " << source_id
+              << ". Reply body: " << message;
     it->second(std::move(receiver_message.value({})));
 
     // Calling the function callback may result in the checksum of the pointed
@@ -266,6 +268,8 @@ void ReceiverSessionMessager::OnMessage(const std::string& source_id,
   if (it == callbacks_.end()) {
     OSP_DLOG_INFO << "Received message without a callback, dropping";
   } else {
+    OSP_DVLOG << "Received valid message from " << source_id
+              << ", executing callback. Message body: " << message;
     it->second(sender_message.value());
   }
 }
diff --git a/discovery/common/config.h b/discovery/common/config.h
index b1ef731a..940001ce 100644
--- a/discovery/common/config.h
+++ b/discovery/common/config.h
@@ -14,28 +14,13 @@ namespace discovery {
 
 // This struct provides parameters needed to initialize the discovery pipeline.
 struct Config {
-  struct NetworkInfo {
-    enum AddressFamilies : uint8_t {
-      kNoAddressFamily = 0,
-      kUseIpV4 = 0x01 << 0,
-      kUseIpV6 = 0x01 << 1
-    };
-
-    // Network Interface on which discovery should be run.
-    InterfaceInfo interface;
-
-    // IP Address Families supported by this network interface and on which the
-    // mDNS Service should listen for and/or publish records.
-    AddressFamilies supported_address_families;
-  };
-
   /*****************************************
    * Common Settings
    *****************************************/
 
   // Interfaces on which services should be published, and on which discovery
   // should listen for announced service instances.
-  std::vector<NetworkInfo> network_info;
+  std::vector<InterfaceInfo> network_info;
 
   // Maximum allowed size in bytes for the rdata in an incoming record. All
   // received records with rdata size exceeding this size will be dropped.
@@ -98,32 +83,6 @@ struct Config {
   bool ignore_nsec_responses = false;
 };
 
-inline Config::NetworkInfo::AddressFamilies operator&(
-    Config::NetworkInfo::AddressFamilies lhs,
-    Config::NetworkInfo::AddressFamilies rhs) {
-  return static_cast<Config::NetworkInfo::AddressFamilies>(
-      static_cast<uint8_t>(lhs) & static_cast<uint8_t>(rhs));
-}
-
-inline Config::NetworkInfo::AddressFamilies operator|(
-    Config::NetworkInfo::AddressFamilies lhs,
-    Config::NetworkInfo::AddressFamilies rhs) {
-  return static_cast<Config::NetworkInfo::AddressFamilies>(
-      static_cast<uint8_t>(lhs) | static_cast<uint8_t>(rhs));
-}
-
-inline Config::NetworkInfo::AddressFamilies operator|=(
-    Config::NetworkInfo::AddressFamilies& lhs,
-    Config::NetworkInfo::AddressFamilies rhs) {
-  return lhs = lhs | rhs;
-}
-
-inline Config::NetworkInfo::AddressFamilies operator&=(
-    Config::NetworkInfo::AddressFamilies& lhs,
-    Config::NetworkInfo::AddressFamilies rhs) {
-  return lhs = lhs & rhs;
-}
-
 }  // namespace discovery
 }  // namespace openscreen
 
diff --git a/discovery/dnssd/impl/service_instance.cc b/discovery/dnssd/impl/service_instance.cc
index 7d4b014f..d923eef9 100644
--- a/discovery/dnssd/impl/service_instance.cc
+++ b/discovery/dnssd/impl/service_instance.cc
@@ -16,29 +16,15 @@ namespace discovery {
 ServiceInstance::ServiceInstance(TaskRunner* task_runner,
                                  ReportingClient* reporting_client,
                                  const Config& config,
-                                 const Config::NetworkInfo& network_info)
+                                 const InterfaceInfo& network_info)
     : task_runner_(task_runner),
       mdns_service_(MdnsService::Create(task_runner,
                                         reporting_client,
                                         config,
                                         network_info)),
-      network_config_(network_info.interface.index,
-                      (network_info.supported_address_families &
-                       Config::NetworkInfo::kUseIpV4)
-                          ? network_info.interface.GetIpAddressV4()
-                          : IPAddress{},
-                      (network_info.supported_address_families &
-                       Config::NetworkInfo::kUseIpV6)
-                          ? network_info.interface.GetIpAddressV6()
-                          : IPAddress{}) {
-  const Config::NetworkInfo::AddressFamilies supported_address_families =
-      network_info.supported_address_families;
-
-  OSP_DCHECK(!(supported_address_families & Config::NetworkInfo::kUseIpV4) ||
-             network_config_.HasAddressV4());
-  OSP_DCHECK(!(supported_address_families & Config::NetworkInfo::kUseIpV6) ||
-             network_config_.HasAddressV6());
-
+      network_config_(network_info.index,
+                      network_info.GetIpAddressV4(),
+                      network_info.GetIpAddressV6()) {
   if (config.enable_querying) {
     querier_ = std::make_unique<QuerierImpl>(
         mdns_service_.get(), task_runner_, reporting_client, &network_config_);
diff --git a/discovery/dnssd/impl/service_instance.h b/discovery/dnssd/impl/service_instance.h
index e06ca564..798a17b0 100644
--- a/discovery/dnssd/impl/service_instance.h
+++ b/discovery/dnssd/impl/service_instance.h
@@ -26,9 +26,9 @@ class ServiceInstance final : public DnsSdService {
   ServiceInstance(TaskRunner* task_runner,
                   ReportingClient* reporting_client,
                   const Config& config,
-                  const Config::NetworkInfo& network_info);
+                  const InterfaceInfo& network_info);
   ServiceInstance(const ServiceInstance& other) = delete;
-  ServiceInstance(ServiceInstance&& other) = delete;
+  ServiceInstance(ServiceInstance&& other) noexcept = delete;
   ~ServiceInstance() override;
 
   ServiceInstance& operator=(const ServiceInstance& other) = delete;
diff --git a/discovery/mdns/mdns_service_impl.cc b/discovery/mdns/mdns_service_impl.cc
index 6d94c3c7..bbbf0815 100644
--- a/discovery/mdns/mdns_service_impl.cc
+++ b/discovery/mdns/mdns_service_impl.cc
@@ -20,7 +20,7 @@ std::unique_ptr<MdnsService> MdnsService::Create(
     TaskRunner* task_runner,
     ReportingClient* reporting_client,
     const Config& config,
-    const Config::NetworkInfo& network_info) {
+    const InterfaceInfo& network_info) {
   return std::make_unique<MdnsServiceImpl>(
       task_runner, Clock::now, reporting_client, config, network_info);
 }
@@ -29,22 +29,21 @@ MdnsServiceImpl::MdnsServiceImpl(TaskRunner* task_runner,
                                  ClockNowFunctionPtr now_function,
                                  ReportingClient* reporting_client,
                                  const Config& config,
-                                 const Config::NetworkInfo& network_info)
+                                 const InterfaceInfo& network_info)
     : task_runner_(task_runner),
       now_function_(now_function),
       reporting_client_(reporting_client),
       receiver_(config),
-      interface_(network_info.interface.index) {
+      interface_(network_info.index) {
   OSP_DCHECK(task_runner_);
   OSP_DCHECK(reporting_client_);
-  OSP_DCHECK(network_info.supported_address_families);
 
   // Create all UDP sockets needed for this object. They should not yet be bound
   // so that they do not send or receive data until the objects on which their
   // callback depends is initialized.
   // NOTE: we bind to the Any addresses here because traffic is filtered by
   // the multicast join calls.
-  if (network_info.supported_address_families & Config::NetworkInfo::kUseIpV4) {
+  if (network_info.GetIpAddressV4()) {
     ErrorOr<std::unique_ptr<UdpSocket>> socket = UdpSocket::Create(
         task_runner, this,
         IPEndpoint{IPAddress::kAnyV4(), kDefaultMulticastPort});
@@ -55,7 +54,7 @@ MdnsServiceImpl::MdnsServiceImpl(TaskRunner* task_runner,
     socket_v4_ = std::move(socket.value());
   }
 
-  if (network_info.supported_address_families & Config::NetworkInfo::kUseIpV6) {
+  if (network_info.GetIpAddressV6()) {
     ErrorOr<std::unique_ptr<UdpSocket>> socket = UdpSocket::Create(
         task_runner, this,
         IPEndpoint{IPAddress::kAnyV6(), kDefaultMulticastPort});
diff --git a/discovery/mdns/mdns_service_impl.h b/discovery/mdns/mdns_service_impl.h
index e1c15226..523f078f 100644
--- a/discovery/mdns/mdns_service_impl.h
+++ b/discovery/mdns/mdns_service_impl.h
@@ -40,7 +40,7 @@ class MdnsServiceImpl : public MdnsService, public UdpSocket::Client {
                   ClockNowFunctionPtr now_function,
                   ReportingClient* reporting_client,
                   const Config& config,
-                  const Config::NetworkInfo& network_info);
+                  const InterfaceInfo& network_info);
   ~MdnsServiceImpl() override;
 
   // MdnsService Overrides.
diff --git a/discovery/mdns/public/mdns_service.h b/discovery/mdns/public/mdns_service.h
index 03e58008..76a8f05d 100644
--- a/discovery/mdns/public/mdns_service.h
+++ b/discovery/mdns/public/mdns_service.h
@@ -34,11 +34,10 @@ class MdnsService {
   // Creates a new MdnsService instance, to be owned by the caller. On failure,
   // returns nullptr. |task_runner|, |reporting_client|, and |config| must exist
   // for the duration of the resulting instance's life.
-  static std::unique_ptr<MdnsService> Create(
-      TaskRunner* task_runner,
-      ReportingClient* reporting_client,
-      const Config& config,
-      const Config::NetworkInfo& network_info);
+  static std::unique_ptr<MdnsService> Create(TaskRunner* task_runner,
+                                             ReportingClient* reporting_client,
+                                             const Config& config,
+                                             const InterfaceInfo& network_info);
 
   // Starts an mDNS query with the given properties. Updated records are passed
   // to |callback|.  The caller must ensure |callback| remains alive while it is
diff --git a/platform/base/trace_logging_types.h b/platform/base/trace_logging_types.h
index 257feaff..73592557 100644
--- a/platform/base/trace_logging_types.h
+++ b/platform/base/trace_logging_types.h
@@ -62,6 +62,8 @@ struct TraceCategory {
     kStandaloneReceiver = 0x01 << 4,
     kDiscovery = 0x01 << 5,
     kStandaloneSender = 0x01 << 6,
+    kReceiver = 0x01 << 7,
+    kSender = 0x01 << 8
   };
 };
 
diff --git a/platform/impl/task_runner.cc b/platform/impl/task_runner.cc
index f306e792..35b9a5e2 100644
--- a/platform/impl/task_runner.cc
+++ b/platform/impl/task_runner.cc
@@ -129,7 +129,6 @@ void TaskRunnerImpl::RequestStopSoon() {
 }
 
 void TaskRunnerImpl::RunRunnableTasks() {
-  OSP_DVLOG << "Running " << running_tasks_.size() << " tasks...";
   for (TaskWithMetadata& running_task : running_tasks_) {
     // Move the task to the stack so that its bound state is freed immediately
     // after being run.
diff --git a/util/json/json_helpers.h b/util/json/json_helpers.h
index 1943973d..499d6352 100644
--- a/util/json/json_helpers.h
+++ b/util/json/json_helpers.h
@@ -16,6 +16,7 @@
 #include "json/value.h"
 #include "platform/base/error.h"
 #include "util/chrono_helpers.h"
+#include "util/json/json_serialization.h"
 #include "util/simple_fraction.h"
 
 // This file contains helper methods for parsing JSON, in an attempt to