commit 4b64411406af7b35e330708b2eddf82d468cf32b
author Karl Wiberg <kwiberg@webrtc.org> Fri Oct 11 09:37:42 2019 +0200
committer Commit Bot <commit-bot@chromium.org> Fri Oct 11 08:34:53 2019 +0000
tree ba6c0898bf4e41833886da95c0aa23644b7ba882
parent 3b819f3d8bbeb72eb0aa2532564fec2f404284c8

NetEqImpl::GetDecoderFormat: Return RTP clockrate, not codec sample rate

Well, in fact we need to return both. But return codec sample rate
separately and let the SdpAudioFormat contain the RTP clockrate,
otherwise we're essentially lying to our callers.

Bug: webrtc:11028
Change-Id: I40f36cb9db6b9824404ade6b0515a8312ff97009
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/156307
Commit-Queue: Karl Wiberg <kwiberg@webrtc.org>
Reviewed-by: Ivo Creusen <ivoc@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#29444}

modules/audio_coding/neteq/include/neteq.h

diff --git a/modules/audio_coding/neteq/include/neteq.h b/modules/audio_coding/neteq/include/neteq.h
index c6af751..b53b5ad 100644
--- a/modules/audio_coding/neteq/include/neteq.h
+++ b/modules/audio_coding/neteq/include/neteq.h
@@ -143,6 +143,13 @@
 
   enum ReturnCodes { kOK = 0, kFail = -1 };
 
+  // Return type for GetDecoderFormat.
+  struct DecoderFormat {
+    int sample_rate_hz;
+    int num_channels;
+    SdpAudioFormat sdp_format;
+  };
+
   // Creates a new NetEq object, with parameters set in |config|. The |config|
   // object will only have to be valid for the duration of the call to this
   // method.
@@ -265,7 +272,7 @@
 
   // Returns the decoder info for the given payload type. Returns empty if no
   // such payload type was registered.
-  virtual absl::optional<SdpAudioFormat> GetDecoderFormat(
+  virtual absl::optional<DecoderFormat> GetDecoderFormat(
       int payload_type) const = 0;
 
   // Flushes both the packet buffer and the sync buffer.

modules/audio_coding/neteq/neteq_impl.cc

diff --git a/modules/audio_coding/neteq/neteq_impl.cc b/modules/audio_coding/neteq/neteq_impl.cc
index 751fc45..37036e3 100644
--- a/modules/audio_coding/neteq/neteq_impl.cc
+++ b/modules/audio_coding/neteq/neteq_impl.cc
@@ -392,21 +392,23 @@
   return last_output_sample_rate_hz_;
 }
 
-absl::optional<SdpAudioFormat> NetEqImpl::GetDecoderFormat(
+absl::optional<NetEq::DecoderFormat> NetEqImpl::GetDecoderFormat(
     int payload_type) const {
   rtc::CritScope lock(&crit_sect_);
   const DecoderDatabase::DecoderInfo* const di =
       decoder_database_->GetDecoderInfo(payload_type);
-  if (!di) {
-    return absl::nullopt;  // Payload type not registered.
+  if (di) {
+    const AudioDecoder* const decoder = di->GetDecoder();
+    // TODO(kwiberg): Why the special case for RED?
+    return DecoderFormat{
+        /*sample_rate_hz=*/di->IsRed() ? 8000 : di->SampleRateHz(),
+        /*num_channels=*/
+        decoder ? rtc::dchecked_cast<int>(decoder->Channels()) : 1,
+        /*sdp_format=*/di->GetFormat()};
+  } else {
+    // Payload type not registered.
+    return absl::nullopt;
   }
-
-  SdpAudioFormat format = di->GetFormat();
-  // TODO(solenberg): This is legacy but messed up - mixing RTP rate and SR.
-  format.clockrate_hz = di->IsRed() ? 8000 : di->SampleRateHz();
-  const AudioDecoder* const decoder = di->GetDecoder();
-  format.num_channels = decoder ? decoder->Channels() : 1;
-  return format;
 }
 
 void NetEqImpl::FlushBuffers() {

modules/audio_coding/neteq/neteq_impl.h

diff --git a/modules/audio_coding/neteq/neteq_impl.h b/modules/audio_coding/neteq/neteq_impl.h
index 8ecb9b6..842869f 100644
--- a/modules/audio_coding/neteq/neteq_impl.h
+++ b/modules/audio_coding/neteq/neteq_impl.h
@@ -182,7 +182,7 @@
 
   int last_output_sample_rate_hz() const override;
 
-  absl::optional<SdpAudioFormat> GetDecoderFormat(
+  absl::optional<DecoderFormat> GetDecoderFormat(
       int payload_type) const override;
 
   // Flushes both the packet buffer and the sync buffer.