VideoCaptureImpl & relatives small cleanup.

content/renderer/media/media_stream_video_source.cc

 // Copyright 2014 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 "content/renderer/media/media_stream_video_source.h"
 
 #include <algorithm>
 #include <limits>
 #include <string>
 
@@ skipping 191 matching lines @@
     bool mandatory,
     media::VideoCaptureFormats* formats) {
   DVLOG(3) << "FilterFormatsByConstraint("
            << "{ constraint.m_name = " << constraint.m_name.utf8()
            << "  constraint.m_value = " << constraint.m_value.utf8()
            << "  mandatory =  " << mandatory << "})";
   media::VideoCaptureFormats::iterator format_it = formats->begin();
   while (format_it != formats->end()) {
     // Modify the format_it to fulfill the constraint if possible.
     // Delete it otherwise.
-    if (!UpdateFormatForConstraint(constraint, mandatory, &(*format_it))) {
+    if (!UpdateFormatForConstraint(constraint, mandatory, &(*format_it)))
       format_it = formats->erase(format_it);
-    } else {
+    else
       ++format_it;
-    }
   }
 }
 
 // Returns the media::VideoCaptureFormats that matches |constraints|.
 media::VideoCaptureFormats FilterFormats(
     const blink::WebMediaConstraints& constraints,
     const media::VideoCaptureFormats& supported_formats,
     blink::WebString* unsatisfied_constraint) {
-  if (constraints.isNull()) {
+  if (constraints.isNull())
     return supported_formats;
-  }
 
   double max_aspect_ratio;
   double min_aspect_ratio;
   GetDesiredMinAndMaxAspectRatio(constraints,
                                  &min_aspect_ratio,
                                  &max_aspect_ratio);
 
   if (min_aspect_ratio > max_aspect_ratio || max_aspect_ratio < 0.05f) {
     DLOG(WARNING) << "Wrong requested aspect ratio.";
     return media::VideoCaptureFormats();
@@ skipping 46 matching lines @@
 
   // Ok - all mandatory checked and we still have candidates.
   // Let's try filtering using the optional constraints. The optional
   // constraints must be filtered in the order they occur in |optional|.
   // But if a constraint produce zero candidates, the constraint is ignored and
   // the next constraint is tested.
   // http://dev.w3.org/2011/webrtc/editor/getusermedia.html#idl-def-Constraints
   for (size_t i = 0; i < optional.size(); ++i) {
     media::VideoCaptureFormats current_candidates = candidates;
     FilterFormatsByConstraint(optional[i], false, &current_candidates);
-    if (!current_candidates.empty()) {
+    if (!current_candidates.empty())
       candidates = current_candidates;
-    }
   }
 
   // We have done as good as we can to filter the supported resolutions.
   return candidates;
 }
 
 const media::VideoCaptureFormat& GetBestFormatBasedOnArea(
     const media::VideoCaptureFormats& formats,
     int area) {
   media::VideoCaptureFormats::const_iterator it = formats.begin();
   media::VideoCaptureFormats::const_iterator best_it = formats.begin();
   int best_diff = std::numeric_limits<int>::max();
   for (; it != formats.end(); ++it) {

[wolenetz, 2015/03/02 23:51:08]

nit: ISTM this could be a candidate for a range-based for loop.

[mcasas, 2015/03/03 15:40:52]

It seems so but if you see previous Patchsets it 
just didn't bend to it...

-    int diff = abs(area - it->frame_size.width() * it->frame_size.height());
+    const int diff = abs(area - it->frame_size.GetArea());

[wolenetz, 2015/03/02 23:51:09]

nit: please update the CL description to note there is more than just auto/const
auto/range-based-for stuff contained in the CL like this (and the removal of
unnecessary {}, above).

[mcasas, 2015/03/03 15:40:52]

Done.

     if (diff < best_diff) {
       best_diff = diff;
       best_it = it;
     }
   }
   return *best_it;
 }
 
 // Find the format that best matches the default video size.
 // This algorithm is chosen since a resolution must be picked even if no
@@ skipping 20 matching lines @@
 }  // anonymous namespace
 
 // static
 MediaStreamVideoSource* MediaStreamVideoSource::GetVideoSource(
     const blink::WebMediaStreamSource& source) {
   return static_cast<MediaStreamVideoSource*>(source.extraData());
 }
 
 // static
 bool MediaStreamVideoSource::IsConstraintSupported(const std::string& name) {
-  for (size_t i = 0; i < arraysize(kSupportedConstraints); ++i) {
-    if (kSupportedConstraints[i] == name)
+  for (const auto& constraint : kSupportedConstraints) {

[wolenetz, 2015/03/02 23:51:09]

each of kSupportedConstraints is a char*. so, const auto* (or, just const
char*)? Specifically, from https://chromium-cpp.appspot.com/, "For pointers, use
for (auto* ...) to make clear that the copy of the loop variable is intended,
and only a pointer is copied"

[mcasas, 2015/03/03 15:40:52]

Done.

+    if (constraint == name)
       return true;
   }
   return false;
 }
 
 MediaStreamVideoSource::MediaStreamVideoSource()
     : state_(NEW),
       track_adapter_(new VideoTrackAdapter(
           ChildProcess::current()->io_message_loop_proxy())),
       weak_factory_(this) {
@@ skipping 122 matching lines @@
   StartSourceImpl(
       current_format_,
       base::Bind(&VideoTrackAdapter::DeliverFrameOnIO, track_adapter_));
 }
 
 bool MediaStreamVideoSource::FindBestFormatWithConstraints(
     const media::VideoCaptureFormats& formats,
     media::VideoCaptureFormat* best_format) {
   DCHECK(CalledOnValidThread());
   // Find the first constraints that we can fulfill.
-  for (std::vector<RequestedConstraints>::iterator request_it =
-           requested_constraints_.begin();
-       request_it != requested_constraints_.end(); ++request_it) {
+  for (const auto& request_it : requested_constraints_) {

[wolenetz, 2015/03/02 23:51:09]

nit: s/_it//

[mcasas, 2015/03/03 15:40:52]

Done.

     const blink::WebMediaConstraints& requested_constraints =
-        request_it->constraints;
+        request_it.constraints;

[wolenetz, 2015/03/02 23:51:09]

nit: s/_it//

[mcasas, 2015/03/03 15:40:52]

Done.

 
     // If the source doesn't support capability enumeration it is still ok if
     // no mandatory constraints have been specified. That just means that
     // we will start with whatever format is native to the source.
     if (formats.empty() && !HasMandatoryConstraints(requested_constraints)) {
       *best_format = media::VideoCaptureFormat();
       return true;
     }
     blink::WebString unsatisfied_constraint;
     media::VideoCaptureFormats filtered_formats =
@@ skipping 31 matching lines @@
   FinalizeAddTrack();
 }
 
 void MediaStreamVideoSource::FinalizeAddTrack() {
   DCHECK(CalledOnValidThread());
   media::VideoCaptureFormats formats;
   formats.push_back(current_format_);
 
   std::vector<RequestedConstraints> callbacks;
   callbacks.swap(requested_constraints_);
-  for (std::vector<RequestedConstraints>::iterator it = callbacks.begin();
-       it != callbacks.end(); ++it) {
+  for (const auto& it : callbacks) {

[wolenetz, 2015/03/02 23:51:09]

nit: s/it/callback/ or s/it/requestedConstraint/ for readability? I don't know
this code that well, but "it" seems to be not very explanatory, especially when
"it" is an auto&.

[mcasas, 2015/03/03 15:40:52]

I think |request| is good enough.
I'll let the RTC owners decide otherwise.

     MediaStreamRequestResult result = MEDIA_DEVICE_OK;
     blink::WebString unsatisfied_constraint;
 
-    if (HasMandatoryConstraints(it->constraints) &&
-        FilterFormats(it->constraints, formats,
-                      &unsatisfied_constraint).empty())
+    if (HasMandatoryConstraints(it.constraints) &&
+        FilterFormats(it.constraints, formats,
+                      &unsatisfied_constraint).empty()) {
       result = MEDIA_DEVICE_CONSTRAINT_NOT_SATISFIED;
+    }
 
     if (state_ != STARTED && result == MEDIA_DEVICE_OK)
       result = MEDIA_DEVICE_TRACK_START_FAILURE;
 
     if (result == MEDIA_DEVICE_OK) {
       int max_width;
       int max_height;
-      GetDesiredMaxWidthAndHeight(it->constraints, &max_width, &max_height);
+      GetDesiredMaxWidthAndHeight(it.constraints, &max_width, &max_height);
       double max_aspect_ratio;
       double min_aspect_ratio;
-      GetDesiredMinAndMaxAspectRatio(it->constraints,
+      GetDesiredMinAndMaxAspectRatio(it.constraints,
                                      &min_aspect_ratio,
                                      &max_aspect_ratio);
       double max_frame_rate = 0.0f;
-      GetConstraintValueAsDouble(it->constraints,
+      GetConstraintValueAsDouble(it.constraints,
                                  kMaxFrameRate, &max_frame_rate);
 
-      track_adapter_->AddTrack(it->track, it->frame_callback,
+      track_adapter_->AddTrack(it.track, it.frame_callback,
                                max_width, max_height,
                                min_aspect_ratio, max_aspect_ratio,
                                max_frame_rate);
     }
 
     DVLOG(3) << "FinalizeAddTrack() result " << result;
 
-    if (!it->callback.is_null()) {
-      it->callback.Run(this, result, unsatisfied_constraint);
+    if (!it.callback.is_null()) {
+      it.callback.Run(this, result, unsatisfied_constraint);
     }
   }
 }
 
 void MediaStreamVideoSource::SetReadyState(
     blink::WebMediaStreamSource::ReadyState state) {
   DVLOG(3) << "MediaStreamVideoSource::SetReadyState state " << state;
   DCHECK(CalledOnValidThread());
   if (!owner().isNull())
     owner().setReadyState(state);
-  for (std::vector<MediaStreamVideoTrack*>::iterator it = tracks_.begin();
-       it != tracks_.end(); ++it) {
-    (*it)->OnReadyStateChanged(state);
-  }
+  for (const auto& it : tracks_)

[wolenetz, 2015/03/02 23:51:09]

nit: s/it/something-that-describes-expected-type-of-it-like-maybe-track/ ?

[mcasas, 2015/03/03 15:40:52]

Done.

+    it->OnReadyStateChanged(state);
 }
 
 void MediaStreamVideoSource::SetMutedState(bool muted_state) {
   DVLOG(3) << "MediaStreamVideoSource::SetMutedState state=" << muted_state;
   DCHECK(CalledOnValidThread());
   if (!owner().isNull()) {
     owner().setReadyState(muted_state
         ? blink::WebMediaStreamSource::ReadyStateMuted
         : blink::WebMediaStreamSource::ReadyStateLive);
   }
 }
 
 MediaStreamVideoSource::RequestedConstraints::RequestedConstraints(
     MediaStreamVideoTrack* track,
     const VideoCaptureDeliverFrameCB& frame_callback,
     const blink::WebMediaConstraints& constraints,
     const ConstraintsCallback& callback)
     : track(track),
       frame_callback(frame_callback),
       constraints(constraints),
       callback(callback) {
 }
 
 MediaStreamVideoSource::RequestedConstraints::~RequestedConstraints() {
 }
 
 }  // namespace content