Added the JNI interface to get and set RtpParameters and the maximum bitrate limits.

webrtc/api/java/jni/jni_helpers.h

 /*
  *  Copyright 2015 The WebRTC project authors. All Rights Reserved.
  *
  *  Use of this source code is governed by a BSD-style license
  *  that can be found in the LICENSE file in the root of the source
  *  tree. An additional intellectual property rights grant can be found
  *  in the file PATENTS.  All contributing project authors may
  *  be found in the AUTHORS file in the root of the source tree.
  */
 
 // This file contain convenience functions and classes for JNI.
 // Before using any of the methods, InitGlobalJniVariables must be called.
 
 #ifndef WEBRTC_API_JAVA_JNI_JNI_HELPERS_H_
 #define WEBRTC_API_JAVA_JNI_JNI_HELPERS_H_
 
 #include <jni.h>
 #include <string>
 
+#include "webrtc/base/constructormagic.h"
 #include "webrtc/base/checks.h"
 
 // Abort the process if |jni| has a Java exception pending.
 // This macros uses the comma operator to execute ExceptionDescribe
 // and ExceptionClear ignoring their return values and sending ""
 // to the error stream.
 #define CHECK_EXCEPTION(jni)        \
   RTC_CHECK(!jni->ExceptionCheck()) \
       << (jni->ExceptionDescribe(), jni->ExceptionClear(), "")
 
@@ skipping 85 matching lines @@
   ~ScopedGlobalRef() {
     DeleteGlobalRef(AttachCurrentThreadIfNeeded(), obj_);
   }
   T operator*() const {
     return obj_;
   }
  private:
   T obj_;
 };
 
+// Provides a convenient way to iterate over a Java Iterable using the
+// C++ range-for loop.
+// E.g. for (jobject value : Iterable(jni, j_iterable)) { ... }
+// Note: Since Java iterators cannot be duplicated, this class is not copyable
+// to prevent creating multiple C++ iterators that refer to the same Java
+// iterator.
+class Iterator {

[magjed_webrtc, 2016/03/22 15:15:30]

Maybe this should be a nested class in Iterable instead?

[skvlad, 2016/03/23 00:29:27]

Done.

+ public:
+  // Creates an iterator representing the end of any collection.
+  Iterator();
+  // Creates an iterator pointing to the beginning of the specified collection.
+  Iterator(JNIEnv* jni, jobject iterable);
+
+  // Move constructor - necessary to be able to return iterator types from
+  // functions.
+  Iterator(Iterator&& other);
+
+  // Move assignment should not be used.
+  Iterator& operator=(Iterator&&) = delete;
+
+  // Advances the iterator one step.
+  Iterator& operator++();
+
+  bool operator==(const Iterator& other) {
+    return (iterator_ == other.iterator_);

[magjed_webrtc, 2016/03/22 15:15:30]

This looks a bit hacky, because you can't compare jobjects this way. I guess you
only care about comparing with the end() iterator, but it would be nice if e.g.
begin() == begin() reliably. Maybe implement this function with something like
this:
if (IsNull(jni, iterator_) || IsNull(jni, other.iterator_))
  return IsNull(jni, iterator_) && IsNull(jni, other.iterator_);
return jni->IsSameObject(value_, other.value_);

[skvlad, 2016/03/23 00:29:27]

This would make two iterators pointing to the same object in two different
collections - or in two places in the same collection - be erroneously treated
as equal.
We can test that the two iterators point to the same collection, and that they
have made the same number of steps - but even that won't work if they have been
obtained at different times and the collection has been modified.
Alternatively, we can have a special case for iterators pointing at begin() -
which still won't solve the general case of comparing two iterators to the same
collection.
Even in standard C++, not all types of iterators can be compared for equality -
for example, comparison for InputIterators
(http://en.cppreference.com/w/cpp/concept/InputIterator) can be undefined. This
iterator can be treated as a more restricted version of the input iterator that
cannot be copied.
In fact we can add a copy constructor and make it exactly satisfy the definition
of an input iterator, but this will likely increase the chance of using it wrong
without giving anything in exchange.

For now I think the best we can do is to make the operation well defined for
comparison with itself (*this == *this) and with the end() iterator - and
undefined for anything else (checked with an RTC_DCHECK()). I've also documented
this behavior in comments.
What do you think? Is there something else we can do to keep it safe without
adding too much complexity?

[magjed_webrtc, 2016/03/23 14:51:25]

This is good, thanks for the detailed explanation!

+  }
+  bool operator!=(const Iterator& other) { return !(*this == other); }
+
+  jobject operator*() { return value_; }
+
+ private:
+  JNIEnv* jni_ = NULL;
+  jobject iterator_ = NULL;
+  jobject value_ = NULL;
+  jmethodID has_next_id_ = NULL;
+  jmethodID next_id_ = NULL;
+
+  RTC_DISALLOW_COPY_AND_ASSIGN(Iterator);
+};
+
+class Iterable {
+ public:
+  explicit Iterable(JNIEnv* jni, jobject iterable)

[magjed_webrtc, 2016/03/22 15:15:30]

Why explicit if you have two arguments?

[skvlad, 2016/03/23 00:29:27]

Oops, my mistake. It used to have one in the previous version of the change -
before I realized I needed the JNIEnv.

+      : jni_(jni), iterable_(iterable) {}

[magjed_webrtc, 2016/03/22 15:15:30]

If you store JNIEnv, I would like to have a thread checker
(webrtc/base/thread_checker.h) to make sure begin() is called on the same thread
as the ctor.

[skvlad, 2016/03/23 00:29:27]

Done.

+
+  Iterator begin() { return Iterator(jni_, iterable_); }
+
+  Iterator end() { return Iterator(); }
+
+ private:
+  JNIEnv* jni_;
+  jobject iterable_;
+
+  RTC_DISALLOW_COPY_AND_ASSIGN(Iterable);
+};
+
 }  // namespace webrtc_jni
 
 #endif  // WEBRTC_API_JAVA_JNI_JNI_HELPERS_H_