Split bit_rate_set(), one for audio, one for video.

Fixes #572.

As discussed in the issue, there's a risk that toxcore may not hold the
maximum bitrates libvpx supports, if toxcore insists on using integer
type. I initially proposed to have another flag in set(), so that we can
use unsigned type instead. iphydf came up with a better solution, that is
splitting the original functions, one for audio, one for video. Now, we
could safely replace int32_t with uint32_t.

Also: clean video_bit_rate_invalid()

Though this is not a part of issue #572, as it's used in the
toxav_bit_rate_set(), i cleaned the code. As mannol said, there should be
a check. Uint32_t is large enough to hold the maximum bitrates libvpx
supports, but user may pass a value larger than uint while smaller than
uint32_t. Thanks to the reminding from nurupo, it's no longer a stub
function.

Bitrate error enums are shared for both audio and video
https://github.com/TokTok/c-toxcore/pull/578#issuecomment-360095609, just
as iphydf said.

4 files changed, 152 insertions, 113 deletions

diff --git a/auto_tests/toxav_basic_test.c b/auto_tests/toxav_basic_test.c
index 2b77da37..bc55d69c 100644
--- a/auto_tests/toxav_basic_test.c
+++ b/auto_tests/toxav_basic_test.c
@@ -476,19 +476,19 @@ START_TEST(test_AV_flows)
 
         printf("Call started as audio only\n");
         printf("Turning on video for Alice...\n");
-        ck_assert(toxav_bit_rate_set(AliceAV, 0, -1, 1000, NULL));
+        ck_assert(toxav_bit_rate_set_video(AliceAV, 0, 1000, NULL));
 
         iterate_tox(bootstrap, Alice, Bob);
         ck_assert(BobCC.state & TOXAV_FRIEND_CALL_STATE_SENDING_V);
 
         printf("Turning off video for Alice...\n");
-        ck_assert(toxav_bit_rate_set(AliceAV, 0, -1, 0, NULL));
+        ck_assert(toxav_bit_rate_set_video(AliceAV, 0, 0, NULL));
 
         iterate_tox(bootstrap, Alice, Bob);
         ck_assert(!(BobCC.state & TOXAV_FRIEND_CALL_STATE_SENDING_V));
 
         printf("Turning off audio for Alice...\n");
-        ck_assert(toxav_bit_rate_set(AliceAV, 0, 0, -1, NULL));
+        ck_assert(toxav_bit_rate_set_audio(AliceAV, 0, 0, NULL));
 
         iterate_tox(bootstrap, Alice, Bob);
         ck_assert(!(BobCC.state & TOXAV_FRIEND_CALL_STATE_SENDING_A));
diff --git a/toxav/toxav.api.h b/toxav/toxav.api.h
index d6f30c67..511b9e2d 100644
--- a/toxav/toxav.api.h
+++ b/toxav/toxav.api.h
@@ -386,40 +386,45 @@ bool call_control(uint32_t friend_number, CALL_CONTROL control) {
  ******************************************************************************/
 
 
+error for bit_rate_set {
+  /**
+   * Synchronization error occurred.
+   */
+  SYNC,
+  /**
+   * The bit rate passed was not one of the supported values.
+   */
+  INVALID_BIT_RATE,
+  /**
+   * The friend_number passed did not designate a valid friend.
+   */
+  FRIEND_NOT_FOUND,
+  /**
+   * This client is currently not in a call with the friend.
+   */
+  FRIEND_NOT_IN_CALL,
+}
+
 namespace bit_rate {
   /**
-   * Set the bit rate to be used in subsequent audio/video frames.
+   * Set the bit rate to be used in subsequent audio frames.
    *
    * @param friend_number The friend number of the friend for which to set the
    * bit rate.
-   * @param audio_bit_rate The new audio bit rate in Kb/sec. Set to 0 to disable
-   * audio sending. Set to -1 to leave unchanged.
-   * @param video_bit_rate The new video bit rate in Kb/sec. Set to 0 to disable
-   * video sending. Set to -1 to leave unchanged.
+   * @param audio_bit_rate The new audio bit rate in Kb/sec. Set to 0 to disable.
    *
    */
-  bool set(uint32_t friend_number, int32_t audio_bit_rate, int32_t video_bit_rate) {
-    /**
-     * Synchronization error occurred.
-     */
-    SYNC,
-    /**
-     * The audio bit rate passed was not one of the supported values.
-     */
-    INVALID_AUDIO_BIT_RATE,
-    /**
-     * The video bit rate passed was not one of the supported values.
-     */
-    INVALID_VIDEO_BIT_RATE,
-    /**
-     * The friend_number passed did not designate a valid friend.
-     */
-    FRIEND_NOT_FOUND,
-    /**
-     * This client is currently not in a call with the friend.
-     */
-    FRIEND_NOT_IN_CALL,
-  }
+
+  bool set_audio(uint32_t friend_number, uint32_t audio_bit_rate) with error for bit_rate_set;
+  /**
+   * Set the bit rate to be used in subsequent video frames.
+   *
+   * @param friend_number The friend number of the friend for which to set the
+   * bit rate.
+   * @param video_bit_rate The new video bit rate in Kb/sec. Set to 0 to disable.
+   *
+   */
+  bool set_video(uint32_t friend_number, uint32_t video_bit_rate) with error for bit_rate_set;
 
   event status {
     /**
diff --git a/toxav/toxav.c b/toxav/toxav.c
index 74836691..27575063 100644
--- a/toxav/toxav.c
+++ b/toxav/toxav.c
@@ -32,6 +32,7 @@
 
 #include <assert.h>
 #include <errno.h>
+#include <limits.h>
 #include <stdlib.h>
 #include <string.h>
 
@@ -524,8 +525,8 @@ END:
 
     return rc == TOXAV_ERR_CALL_CONTROL_OK;
 }
-bool toxav_bit_rate_set(ToxAV *av, uint32_t friend_number, int32_t audio_bit_rate,
-                        int32_t video_bit_rate, TOXAV_ERR_BIT_RATE_SET *error)
+bool toxav_bit_rate_set_audio(ToxAV *av, uint32_t friend_number, uint32_t audio_bit_rate,
+                              TOXAV_ERR_BIT_RATE_SET *error)
 {
     TOXAV_ERR_BIT_RATE_SET rc = TOXAV_ERR_BIT_RATE_SET_OK;
     ToxAVCall *call;
@@ -536,12 +537,7 @@ bool toxav_bit_rate_set(ToxAV *av, uint32_t friend_number, int32_t audio_bit_rat
     }
 
     if (audio_bit_rate > 0 && audio_bit_rate_invalid(audio_bit_rate)) {
-        rc = TOXAV_ERR_BIT_RATE_SET_INVALID_AUDIO_BIT_RATE;
-        goto END;
-    }
-
-    if (video_bit_rate > 0 && video_bit_rate_invalid(video_bit_rate)) {
-        rc = TOXAV_ERR_BIT_RATE_SET_INVALID_VIDEO_BIT_RATE;
+        rc = TOXAV_ERR_BIT_RATE_SET_INVALID_BIT_RATE;
         goto END;
     }
 
@@ -554,84 +550,114 @@ bool toxav_bit_rate_set(ToxAV *av, uint32_t friend_number, int32_t audio_bit_rat
         goto END;
     }
 
-    if (audio_bit_rate >= 0) {
-        LOGGER_DEBUG(av->m->log, "Setting new audio bitrate to: %d", audio_bit_rate);
+    LOGGER_DEBUG(av->m->log, "Setting new audio bitrate to: %d", audio_bit_rate);
+
+    if (call->audio_bit_rate == audio_bit_rate) {
+        LOGGER_DEBUG(av->m->log, "Audio bitrate already set to: %d", audio_bit_rate);
+    } else if (audio_bit_rate == 0) {
+        LOGGER_DEBUG(av->m->log, "Turned off audio sending");
 
-        if (call->audio_bit_rate == audio_bit_rate) {
-            LOGGER_DEBUG(av->m->log, "Audio bitrate already set to: %d", audio_bit_rate);
-        } else if (audio_bit_rate == 0) {
-            LOGGER_DEBUG(av->m->log, "Turned off audio sending");
+        if (msi_change_capabilities(call->msi_call, call->msi_call->
+                                    self_capabilities ^ msi_CapSAudio) != 0) {
+            pthread_mutex_unlock(av->mutex);
+            rc = TOXAV_ERR_BIT_RATE_SET_SYNC;
+            goto END;
+        }
+
+        /* Audio sending is turned off; notify peer */
+        call->audio_bit_rate = 0;
+    } else {
+        pthread_mutex_lock(call->mutex);
 
-            if (msi_change_capabilities(call->msi_call, call->msi_call->
-                                        self_capabilities ^ msi_CapSAudio) != 0) {
+        if (call->audio_bit_rate == 0) {
+            LOGGER_DEBUG(av->m->log, "Turned on audio sending");
+
+            /* The audio has been turned off before this */
+            if (msi_change_capabilities(call->msi_call, call->
+                                        msi_call->self_capabilities | msi_CapSAudio) != 0) {
+                pthread_mutex_unlock(call->mutex);
                 pthread_mutex_unlock(av->mutex);
                 rc = TOXAV_ERR_BIT_RATE_SET_SYNC;
                 goto END;
             }
-
-            /* Audio sending is turned off; notify peer */
-            call->audio_bit_rate = 0;
         } else {
-            pthread_mutex_lock(call->mutex);
+            LOGGER_DEBUG(av->m->log, "Set new audio bit rate %d", audio_bit_rate);
+        }
 
-            if (call->audio_bit_rate == 0) {
-                LOGGER_DEBUG(av->m->log, "Turned on audio sending");
+        call->audio_bit_rate = audio_bit_rate;
+        pthread_mutex_unlock(call->mutex);
+    }
 
-                /* The audio has been turned off before this */
-                if (msi_change_capabilities(call->msi_call, call->
-                                            msi_call->self_capabilities | msi_CapSAudio) != 0) {
-                    pthread_mutex_unlock(call->mutex);
-                    pthread_mutex_unlock(av->mutex);
-                    rc = TOXAV_ERR_BIT_RATE_SET_SYNC;
-                    goto END;
-                }
-            } else {
-                LOGGER_DEBUG(av->m->log, "Set new audio bit rate %d", audio_bit_rate);
-            }
+    pthread_mutex_unlock(av->mutex);
+END:
 
-            call->audio_bit_rate = audio_bit_rate;
-            pthread_mutex_unlock(call->mutex);
-        }
+    if (error) {
+        *error = rc;
     }
 
-    if (video_bit_rate >= 0) {
-        LOGGER_DEBUG(av->m->log, "Setting new video bitrate to: %d", video_bit_rate);
+    return rc == TOXAV_ERR_BIT_RATE_SET_OK;
+}
+bool toxav_bit_rate_set_video(ToxAV *av, uint32_t friend_number, uint32_t video_bit_rate,
+                              TOXAV_ERR_BIT_RATE_SET *error)
+{
+    TOXAV_ERR_BIT_RATE_SET rc = TOXAV_ERR_BIT_RATE_SET_OK;
+    ToxAVCall *call;
 
-        if (call->video_bit_rate == video_bit_rate) {
-            LOGGER_DEBUG(av->m->log, "Video bitrate already set to: %d", video_bit_rate);
-        } else if (video_bit_rate == 0) {
-            LOGGER_DEBUG(av->m->log, "Turned off video sending");
+    if (m_friend_exists(av->m, friend_number) == 0) {
+        rc = TOXAV_ERR_BIT_RATE_SET_FRIEND_NOT_FOUND;
+        goto END;
+    }
 
-            /* Video sending is turned off; notify peer */
-            if (msi_change_capabilities(call->msi_call, call->msi_call->
-                                        self_capabilities ^ msi_CapSVideo) != 0) {
+    if (video_bit_rate > 0 && video_bit_rate_invalid(video_bit_rate)) {
+        rc = TOXAV_ERR_BIT_RATE_SET_INVALID_BIT_RATE;
+        goto END;
+    }
+
+    pthread_mutex_lock(av->mutex);
+    call = call_get(av, friend_number);
+
+    if (call == NULL || !call->active || call->msi_call->state != msi_CallActive) {
+        pthread_mutex_unlock(av->mutex);
+        rc = TOXAV_ERR_BIT_RATE_SET_FRIEND_NOT_IN_CALL;
+        goto END;
+    }
+
+    LOGGER_DEBUG(av->m->log, "Setting new video bitrate to: %d", video_bit_rate);
+
+    if (call->video_bit_rate == video_bit_rate) {
+        LOGGER_DEBUG(av->m->log, "Video bitrate already set to: %d", video_bit_rate);
+    } else if (video_bit_rate == 0) {
+        LOGGER_DEBUG(av->m->log, "Turned off video sending");
+
+        /* Video sending is turned off; notify peer */
+        if (msi_change_capabilities(call->msi_call, call->msi_call->
+                                    self_capabilities ^ msi_CapSVideo) != 0) {
+            pthread_mutex_unlock(av->mutex);
+            rc = TOXAV_ERR_BIT_RATE_SET_SYNC;
+            goto END;
+        }
+
+        call->video_bit_rate = 0;
+    } else {
+        pthread_mutex_lock(call->mutex);
+
+        if (call->video_bit_rate == 0) {
+            LOGGER_DEBUG(av->m->log, "Turned on video sending");
+
+            /* The video has been turned off before this */
+            if (msi_change_capabilities(call->msi_call, call->
+                                        msi_call->self_capabilities | msi_CapSVideo) != 0) {
+                pthread_mutex_unlock(call->mutex);
                 pthread_mutex_unlock(av->mutex);
                 rc = TOXAV_ERR_BIT_RATE_SET_SYNC;
                 goto END;
             }
-
-            call->video_bit_rate = 0;
         } else {
-            pthread_mutex_lock(call->mutex);
-
-            if (call->video_bit_rate == 0) {
-                LOGGER_DEBUG(av->m->log, "Turned on video sending");
-
-                /* The video has been turned off before this */
-                if (msi_change_capabilities(call->msi_call, call->
-                                            msi_call->self_capabilities | msi_CapSVideo) != 0) {
-                    pthread_mutex_unlock(call->mutex);
-                    pthread_mutex_unlock(av->mutex);
-                    rc = TOXAV_ERR_BIT_RATE_SET_SYNC;
-                    goto END;
-                }
-            } else {
-                LOGGER_DEBUG(av->m->log, "Set new video bit rate %d", video_bit_rate);
-            }
-
-            call->video_bit_rate = video_bit_rate;
-            pthread_mutex_unlock(call->mutex);
+            LOGGER_DEBUG(av->m->log, "Set new video bit rate %d", video_bit_rate);
         }
+
+        call->video_bit_rate = video_bit_rate;
+        pthread_mutex_unlock(call->mutex);
     }
 
     pthread_mutex_unlock(av->mutex);
@@ -1020,9 +1046,14 @@ bool audio_bit_rate_invalid(uint32_t bit_rate)
 }
 bool video_bit_rate_invalid(uint32_t bit_rate)
 {
-    (void) bit_rate;
-    /* TODO(mannol): If anyone knows the answer to this one please fill it up */
-    return false;
+    /* https://www.webmproject.org/docs/webm-sdk/structvpx__codec__enc__cfg.html shows the following:
+     * unsigned int rc_target_bitrate
+     * the range of uint varies from platform to platform
+     * though, uint32_t should be large enough to store bitrates,
+     * we may want to prevent from passing overflowed bitrates to libvpx
+     * more in detail, it's the case where bit_rate is larger than uint, but smaller than uint32_t
+     */
+    return bit_rate > UINT_MAX;
 }
 bool invoke_call_state_callback(ToxAV *av, uint32_t friend_number, uint32_t state)
 {
diff --git a/toxav/toxav.h b/toxav/toxav.h
index 2e48d599..f8a683a6 100644
--- a/toxav/toxav.h
+++ b/toxav/toxav.h
@@ -494,14 +494,9 @@ typedef enum TOXAV_ERR_BIT_RATE_SET {
     TOXAV_ERR_BIT_RATE_SET_SYNC,
 
     /**
-     * The audio bit rate passed was not one of the supported values.
+     * The bit rate passed was not one of the supported values.
      */
-    TOXAV_ERR_BIT_RATE_SET_INVALID_AUDIO_BIT_RATE,
-
-    /**
-     * The video bit rate passed was not one of the supported values.
-     */
-    TOXAV_ERR_BIT_RATE_SET_INVALID_VIDEO_BIT_RATE,
+    TOXAV_ERR_BIT_RATE_SET_INVALID_BIT_RATE,
 
     /**
      * The friend_number passed did not designate a valid friend.
@@ -517,18 +512,26 @@ typedef enum TOXAV_ERR_BIT_RATE_SET {
 
 
 /**
- * Set the bit rate to be used in subsequent audio/video frames.
+ * Set the bit rate to be used in subsequent audio frames.
+ *
+ * @param friend_number The friend number of the friend for which to set the
+ * bit rate.
+ * @param audio_bit_rate The new audio bit rate in Kb/sec. Set to 0 to disable.
+ *
+ */
+bool toxav_bit_rate_set_audio(ToxAV *av, uint32_t friend_number, uint32_t audio_bit_rate,
+                              TOXAV_ERR_BIT_RATE_SET *error);
+
+/**
+ * Set the bit rate to be used in subsequent video frames.
  *
  * @param friend_number The friend number of the friend for which to set the
  * bit rate.
- * @param audio_bit_rate The new audio bit rate in Kb/sec. Set to 0 to disable
- * audio sending. Set to -1 to leave unchanged.
- * @param video_bit_rate The new video bit rate in Kb/sec. Set to 0 to disable
- * video sending. Set to -1 to leave unchanged.
+ * @param video_bit_rate The new video bit rate in Kb/sec. Set to 0 to disable.
  *
  */
-bool toxav_bit_rate_set(ToxAV *av, uint32_t friend_number, int32_t audio_bit_rate, int32_t video_bit_rate,
-                        TOXAV_ERR_BIT_RATE_SET *error);
+bool toxav_bit_rate_set_video(ToxAV *av, uint32_t friend_number, uint32_t video_bit_rate,
+                              TOXAV_ERR_BIT_RATE_SET *error);
 
 /**
  * The function type for the bit_rate_status callback. The event is triggered