Android refactoring + rtc stats #58

vona-ben · 2023-08-28T09:38:56Z

Note: A dev branch should be created so we can merge this after review

Complete refactoring of Android code. Using a similar approach of OPENTOK network library JS.

Added NetworkQualityTest class is a utility for testing network quality and gathering statistics during video streaming sessions using the OpenTok platform. It allows monitoring various network and media statistics, such as video bitrate, audio quality, round-trip time, and more.

The NetworkQualityTest will send regular callbacks which can be handled using NetworkQualityTestCallbackListener.

The MainActivityclass is a sample that shows how to use this class and how to handle the callback.

Better estimation of availableOutgoingBitrate

juliobecerragomez · 2023-08-30T14:23:55Z

Android-Sample/app/src/main/java/com/opentok/qualitystats/sample/NetworkQualityTest.java

+                }
+                jitter = rtcStatObject.optDouble(JITTER, -1);
+            } else if (CANDIDATE_PAIR.equals(statType)) {
+                boolean isNominated = rtcStatObject.optBoolean(IS_NOMINATED, false);


I think several candidates may be nominated. Why not using the selectedCandidatePairId from the transport stats?

https://developer.mozilla.org/en-US/docs/Web/API/RTCIceCandidatePairStats/selected

goncalocostamendes · 2023-08-30T16:21:49Z

Android-Sample/app/src/main/java/com/opentok/qualitystats/sample/MainActivity.java

-    private AlertDialog dialog;
+public class MainActivity extends Activity {
+    static final String LOGTAG = "quality-stats-demo";
+    private static final String SESSION_ID = "YOUR_SESSION_ID";


For the most distracted users, let's add some documentation on the README about this. By not being empty anymore the app will no longer alert the user and will try to connect for the duration of the timeout.

goncalocostamendes · 2023-08-30T16:32:50Z

Android-Sample/README.md

-    mHandler.postDelayed(statsRunnable, TEST_DURATION * 1000);
-}
-```
+    To stop the test, you can call stopTest. If the test is halted before 5 seconds, an error will be triggered.


nit: change "you can call stopTest" to "you can call NetworkQualityTest.stopTest()"

goncalocostamendes · 2023-08-30T16:34:27Z

Android-Sample/README.md

@@ -1,7 +1,11 @@
 OpenTok Android Network Test Sample
 ===================================

-This sample shows how to use this OpenTok Android SDK to determine the appropriate audio and video
+# NetworkQualityTest
+


nit: Link to NetworkQualityTest README

goncalocostamendes · 2023-08-30T16:41:58Z

Android-Sample/app/README.md

+
+6. **Clean Up**
+
+   To ensure proper resource release, you should handle cleanup when needed. In your `onPause`


If I am not mistaken, onPause() is triggered when app goes to the background. Video is off, but audio is still on. Having that in mind, don't know if there is the need to stop the test (of course, knowing without video, the stats will go up). Your call

goncalocostamendes · 2023-08-30T16:43:10Z

Android-Sample/app/src/main/AndroidManifest.xml

@@ -9,6 +8,8 @@
    <uses-permission android:name="android.permission.MODIFY_AUDIO_SETTINGS" />
    <uses-permission android:name="android.permission.WRITE_EXTERNAL_STORAGE" />


WRITE_EXTERNAL_STORAGE permission does not have to be requested on the java side as well? On java side only see camera, internet and audio.

goncalocostamendes · 2023-08-30T16:49:57Z

Android-Sample/app/src/main/java/com/opentok/qualitystats/sample/MainActivity.java

+                        Log.d(LOGTAG, "Audio Packet Lost Ratio  " + stats.getAudioPacketLostRatio() * 100 + "%");
+                        Log.d(LOGTAG, "Video Packet Lost Ratio  " + stats.getVideoPacketLostRatio() * 100 + "%");
+                        Log.d(LOGTAG, "Jitter: " + stats.getJitter());
+                        Log.d(LOGTAG, "Quality Limitation Reason: " + stats.getQualityLimitationReason());


Maybe we can change the previous logs with Log.d(LOGTAG, sentVideoInfo);. In the future if we have to change something, we only have to change it in one place

goncalocostamendes · 2023-08-30T16:53:39Z

Android-Sample/app/src/main/java/com/opentok/qualitystats/sample/MainActivity.java

+                    }
+                });
+        // Start the quality test
+        networkQualityTest.startTest();


Should we only start the test when we start publishing? What if the user does not provide the permissions, or takes a lot of time to do so, and the test finishes without even having published? (Based on what I understood, if not explicitly stopped the test only takes 30s)

I moved permission to the MainActivity and only run the test now if the user accepted the permissions.

goncalocostamendes · 2023-08-30T16:56:35Z

Android-Sample/README.md

+    ```java
+    new NetworkQualityTestCallbackListener() {
+        @Override
+        public void onQualityTestResults(String recommendedSetting) {


Can the recommendedSetting be do not use video or something similar? If the minimum conditions are not met?

There is a threshold where the video is not recommended, added it to the docs and comments

goncalocostamendes

Overall looks great.
I left some comments. The majority are nit.
Tomorrow I will test the app to do a final validation

jeffswartz · 2023-08-30T20:37:15Z

Android-Sample/README.md

+    - `onQualityTestResults`: Receive the final test results and the recommended setting based on quality thresholds.
+    - `onQualityTestStatsUpdate`: Receive real-time statistics updates during the test.
+    - `onError`: Handle errors that occur during the test.


Please provide more details on the API:

onQualityTestResults() -- What are the possible values of recommendedSetting, and what do they mean?

onQualityTestStatsUpdate() -- What are the properties of CallbackQualityStats, and what do they mean?

onError() -- What are the possible errors, and what do they mean?

jeffswartz · 2023-08-30T20:38:16Z

Android-Sample/README.md

-   * Voice-only -- Your bandwidth is too low for video.
+    You can configure the following parameters:
+
+    - `sessionId`: The session ID of your OpenTok session.


Mention that this should be a routed session (right?) And mention that this session should only be used for the test (not for communications).

jeffswartz · 2023-08-30T20:39:29Z

Android-Sample/README.md

+    - `resolution`: The camera capture resolution for the publisher. Defaults to `HIGH` if not specified.
+    - `testDurationSec`: The duration of the test in seconds. Defaults to `30 sec` if not specified.


What are possible values for resolution and testDurationSec.

jeffswartz · 2023-08-30T20:46:58Z

Android-Sample/app/README.md

+The `NetworkQualityTest` class is a utility for testing network quality and gathering statistics
+during video streaming sessions using the OpenTok platform. It allows you to monitor various network
+and media statistics, such as video bitrate, audio quality, round-trip time, and more.


Add a link to the docs on the callback methods (see my comments on the main README).

Ben and others added 23 commits August 4, 2023 22:45

WIP update android stats

9991e9d

Remove credentiels

4596d6d

Clean-up

ae91fa8

Refactoring

e768d53

Fix subscriber stas

269ddc3

Typo in annotation

42a4008

Fix buiding settings

fb10474

Better estimation of availableOutgoingBitrate

629217f

Merge pull request #1 from josemrecio/android-rtc-stats-update

ba8ba1c

Better estimation of availableOutgoingBitrate

[WIP] Refactoring

415d9a3

WIP refactoring

1c3e1f8

WIP refactoring

459eaaa

WIP refactoring

e4b1268

WIP refactoring

7a0db43

WIP refactoring

7c92790

Refactoring

65342e0

WIP refactoring

6c70386

WIP refactoring

74bc865

WIP refactoring

21264fb

Remove lombodk

6c653e6

clean up

e643cf0

Add a method to stop the test

92969ad

Clean-up

f59dc3c

vona-ben changed the title ~~Android rtc stats~~ Android refactoring + rtc stats Aug 30, 2023

juliobecerragomez reviewed Aug 30, 2023

View reviewed changes

goncalocostamendes reviewed Aug 30, 2023

View reviewed changes

jeffswartz requested changes Aug 30, 2023

View reviewed changes

jeffswartz reviewed Aug 30, 2023

View reviewed changes

vona-ben changed the base branch from master to development August 31, 2023 03:30

Ben added 3 commits September 8, 2023 16:22

Fix permission issues + PR comments

3e449f0

Fix permission issues

894918b

Clean-up

e6f0326

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Android refactoring + rtc stats #58

Android refactoring + rtc stats #58

vona-ben commented Aug 28, 2023 •

edited

juliobecerragomez Aug 30, 2023 •

edited

goncalocostamendes Aug 30, 2023

goncalocostamendes Aug 30, 2023

goncalocostamendes Aug 30, 2023

goncalocostamendes Aug 30, 2023 •

edited

goncalocostamendes Aug 30, 2023

goncalocostamendes Aug 30, 2023

goncalocostamendes Aug 30, 2023

vona-ben Sep 8, 2023

goncalocostamendes Aug 30, 2023

vona-ben Sep 8, 2023

goncalocostamendes left a comment

jeffswartz Aug 30, 2023

jeffswartz Aug 30, 2023

jeffswartz Aug 30, 2023

jeffswartz Aug 30, 2023


		6. Clean Up

		To ensure proper resource release, you should handle cleanup when needed. In your `onPause`

		@@ -9,6 +8,8 @@
		<uses-permission android:name="android.permission.MODIFY_AUDIO_SETTINGS" />
		<uses-permission android:name="android.permission.WRITE_EXTERNAL_STORAGE" />

		- `resolution`: The camera capture resolution for the publisher. Defaults to `HIGH` if not specified.
		- `testDurationSec`: The duration of the test in seconds. Defaults to `30 sec` if not specified.

Android refactoring + rtc stats #58

Are you sure you want to change the base?

Android refactoring + rtc stats #58

Conversation

vona-ben commented Aug 28, 2023 • edited

juliobecerragomez Aug 30, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

goncalocostamendes Aug 30, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

goncalocostamendes left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vona-ben commented Aug 28, 2023 •

edited

juliobecerragomez Aug 30, 2023 •

edited

goncalocostamendes Aug 30, 2023 •

edited