Extracted hardcoded constants from cast_rtp_stream.

chrome/renderer/media/cast_rtp_stream.cc

 // Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/renderer/media/cast_rtp_stream.h"
 
 #include <algorithm>
 
 #include "base/bind.h"
 #include "base/callback_helpers.h"
 #include "base/command_line.h"
 #include "base/logging.h"
 #include "base/memory/weak_ptr.h"
 #include "base/strings/stringprintf.h"
 #include "base/sys_info.h"
 #include "base/trace_event/trace_event.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/renderer/media/cast_session.h"
 #include "chrome/renderer/media/cast_udp_transport.h"
 #include "content/public/renderer/media_stream_audio_sink.h"
 #include "content/public/renderer/media_stream_video_sink.h"
 #include "content/public/renderer/render_thread.h"
 #include "content/public/renderer/video_encode_accelerator.h"
 #include "media/audio/audio_parameters.h"
 #include "media/base/audio_bus.h"
 #include "media/base/audio_converter.h"
 #include "media/base/audio_fifo.h"
 #include "media/base/bind_to_current_loop.h"
+#include "media/base/limits.h"
 #include "media/base/video_frame.h"
 #include "media/cast/cast_config.h"
 #include "media/cast/cast_defines.h"
 #include "media/cast/cast_sender.h"
 #include "media/cast/net/cast_transport_config.h"
 #include "third_party/WebKit/public/platform/WebMediaStreamSource.h"
 #include "ui/gfx/geometry/size.h"
 
 using media::cast::AudioSenderConfig;
 using media::cast::VideoSenderConfig;
@@ skipping 34 matching lines @@
   payload.min_bitrate = 50;
   payload.channels = 1;
   payload.max_frame_rate = media::cast::kDefaultMaxFrameRate;
   payload.codec_name = kCodecNameVp8;
   return payload;
 }
 
 CastRtpPayloadParams DefaultH264Payload() {
   CastRtpPayloadParams payload;
   // TODO(hshi): set different ssrc/rtpPayloadType values for H264 and VP8
   // once b/13696137 is fixed.

[mcasas, 2015/09/15 16:59:39]

Note: We shouldn't have buganizer bugs in Chromium 
codebase. And, this b/ is fixed :)

[miu, 2015/09/15 22:05:22]

Note: esnada@ is an external-to-Google contributor.  ;-)

Looking at this comment and the bug tracker, I'd say it'd be fine to just delete
it.

[msu.koo, 2015/09/17 07:00:50]

This comment lines were removed.

   payload.payload_type = 96;
   payload.max_latency_ms = media::cast::kDefaultRtpMaxDelayMs;
   payload.ssrc = 11;
   payload.feedback_ssrc = 12;
   payload.clock_rate = media::cast::kVideoFrequency;
   payload.max_bitrate = 2000;
   payload.min_bitrate = 50;

[mcasas, 2015/09/15 16:59:39]

What about these |50| and |2000| ? (l.73-74, 90-91)

   payload.channels = 1;
   payload.max_frame_rate = media::cast::kDefaultMaxFrameRate;
   payload.codec_name = kCodecNameH264;
   return payload;
 }
 
 bool IsHardwareVP8EncodingSupported() {
   const base::CommandLine* cmd_line = base::CommandLine::ForCurrentProcess();
   if (cmd_line->HasSwitch(switches::kDisableCastStreamingHWEncoding)) {
     DVLOG(1) << "Disabled hardware VP8 support for Cast Streaming.";
@@ skipping 59 matching lines @@
 
   return supported_params;
 }
 
 bool ToAudioSenderConfig(const CastRtpParams& params,
                          AudioSenderConfig* config) {
   config->ssrc = params.payload.ssrc;
   config->receiver_ssrc = params.payload.feedback_ssrc;
   if (config->ssrc == config->receiver_ssrc)
     return false;
   config->min_playout_delay =

[mcasas, 2015/09/15 16:59:39]

see my comment on l.207

[msu.koo, 2015/09/17 07:00:50]

Done.

       base::TimeDelta::FromMilliseconds(
           params.payload.min_latency_ms ?
           params.payload.min_latency_ms :
           params.payload.max_latency_ms);
   config->max_playout_delay =
       base::TimeDelta::FromMilliseconds(params.payload.max_latency_ms);
   if (config->min_playout_delay <= base::TimeDelta())
     return false;
   if (config->min_playout_delay > config->max_playout_delay)
     return false;
   config->rtp_payload_type = params.payload.payload_type;
   config->use_external_encoder = false;
   config->frequency = params.payload.clock_rate;
-  if (config->frequency < 8000)
+  if (config->frequency < media::cast::kMinSampleRateForEncoding)

[mcasas, 2015/09/15 16:59:39]

If |kMinSampleRateForEncoding| is only used in this file
you should move it to here around l.43, so is not exported
-- less symbols for the linker etc.

[miu, 2015/09/15 22:05:22]

I disagree with this one.  IMO, the constant belongs in the cast_defines.h
header file since it pertains to all clients of the media/cast library.

Side note: We plan to re-do all the int constants as enums as part of a massive
clean-up bug we're about to start on, which would resolve the "extra linking"
problem.

     return false;
   config->channels = params.payload.channels;
   if (config->channels < 1)
     return false;
   config->bitrate = params.payload.max_bitrate * kBitrateMultiplier;
   if (params.payload.codec_name == kCodecNameOpus)
     config->codec = media::cast::CODEC_AUDIO_OPUS;
   else
     return false;
   config->aes_key = params.payload.aes_key;
   config->aes_iv_mask = params.payload.aes_iv_mask;
   return true;
 }
 
 bool ToVideoSenderConfig(const CastRtpParams& params,
                          VideoSenderConfig* config) {
   config->ssrc = params.payload.ssrc;
   config->receiver_ssrc = params.payload.feedback_ssrc;
   if (config->ssrc == config->receiver_ssrc)
     return false;
   config->min_playout_delay =
       base::TimeDelta::FromMilliseconds(

[mcasas, 2015/09/15 16:59:39]

l.206-210 should be formatted:

  config->min_playout_delay = base::TimeDelta::FromMilliseconds(
                                  params.payload.min_latency_ms ?
                                  params.payload.min_latency_ms :
                                  params.payload.max_latency_ms);

[msu.koo, 2015/09/17 07:00:50]

Done.

           params.payload.min_latency_ms ?
           params.payload.min_latency_ms :
           params.payload.max_latency_ms);
   config->max_playout_delay =
       base::TimeDelta::FromMilliseconds(params.payload.max_latency_ms);
   if (config->min_playout_delay <= base::TimeDelta())
     return false;
   if (config->min_playout_delay > config->max_playout_delay)
     return false;
   config->rtp_payload_type = params.payload.payload_type;
   config->min_bitrate = config->start_bitrate =
       params.payload.min_bitrate * kBitrateMultiplier;
   config->max_bitrate = params.payload.max_bitrate * kBitrateMultiplier;
   if (config->min_bitrate > config->max_bitrate)
     return false;
   config->start_bitrate = config->min_bitrate;
   config->max_frame_rate = static_cast<int>(
       std::max(1.0, params.payload.max_frame_rate) + 0.5);
-  if (config->max_frame_rate > 120)
+  if (config->max_frame_rate > media::limits::kMaxFramesPerSecond)
     return false;
   if (params.payload.codec_name == kCodecNameVp8) {
     config->use_external_encoder = IsHardwareVP8EncodingSupported();
     config->codec = media::cast::CODEC_VIDEO_VP8;
   } else if (params.payload.codec_name == kCodecNameH264) {
     config->use_external_encoder = IsHardwareH264EncodingSupported();
     config->codec = media::cast::CODEC_VIDEO_H264;
   } else {
     return false;
   }
   if (!config->use_external_encoder) {

[mcasas, 2015/09/15 16:59:39]

no {}

[msu.koo, 2015/09/17 07:00:50]

Done.

     config->number_of_encode_threads = NumberOfEncodeThreads();
   }
   config->aes_key = params.payload.aes_key;
   config->aes_iv_mask = params.payload.aes_iv_mask;
   return true;
 }
 
 }  // namespace
 
 // This class receives MediaStreamTrack events and video frames from a
@@ skipping 328 matching lines @@
   DCHECK(content::RenderThread::Get());
   DVLOG(1) << "CastRtpStream::DidEncounterError(" << message << ") = "
            << (IsAudio() ? "audio" : "video");
   // Save the WeakPtr first because the error callback might delete this object.
   base::WeakPtr<CastRtpStream> ptr = weak_factory_.GetWeakPtr();
   error_callback_.Run(message);
   base::ThreadTaskRunnerHandle::Get()->PostTask(
       FROM_HERE,
       base::Bind(&CastRtpStream::Stop, ptr));
 }