Allow setting repeat mode with Intent arguments

[hubert-mazur]

Add a new boolean command line optional argument to allow the playback to run in loop. This would help to use the app in the automated video tests. The change does not enable the option view for switching the loop mode in the UI as providing an extra argument implies force, not optional, loop playback.

[marcbaechinger]

Mind making a REPEAT_MODE_EXTRA and a REPEAT_MODE_EXTRA_VALUE_NONE, REPEAT_MODE_EXTRA_VALUE_ONE, REPEAT_MODE_EXTRA_VALUE_ALL so we cover the entire repeat mode API?

I understand the idea is to start the app directly into the PlayerActivity from the command line. We have documented the Intent arguments we support here: https://developer.android.com/media/media3/exoplayer/demo-application#3-firing

I will document this on DAC once we have landed that change. The aim is to have an addition like the following

--es repeat_mode NONE | ONE | ALL

WDYT?

See https://gist.github.com/tsohr/5711945#file-gistfile1-txt-L81

[marcbaechinger]

Thanks for your PR! Please take a look at my suggestion in the comment!

[hubert-mazur]

Thanks for the feedback! I'll do the changes and upload the second version soon.

[hubert-mazur]

Hi, I uploaded the v2 of the change. Let me briefly describe the idea.

Per suggestions, I changed the optional argument repeat_mode to a String that should receive one of the following options which correspond to the Player API: NONE|ONE|ALL.

If provided, it is validated and set appropriately. In case of malicious input, the default (NONE) will be set.
Setting the player mode will only happen in case where the optional argument was provided. Otherwise nothing is set. I thought that this approach is the best, since it is an optional argument and if none provided, no action, even a default one should be taken.

Please take a look and share your thoughts.

[icbaker]

I would suggest making this stricter:

• Only accept the exact strings "NONE", "ONE" and "ALL"
• Fail if an unrecognized string is received

The issue with silently falling back to a default is it can be quite confusing if someone expects --es repeat_mode all or --es repeat_mode REPEAT_MODE_ALL to work and there's no signal (and nothing in logcat) to explain why it didn't.

You can see an existing example of an 'invalid parse' of an intent extra resulting in a failure by trying this:

$ adb shell am start -a androidx.media3.demo.main.action.VIEW \
       -d  "https://html5demos.com/assets/dizzy.mp4" \
       --es drm_scheme widevine \
       --esa drm_key_request_properties foo,bar,baz

This fails with an exception like:

java.lang.RuntimeException: Unable to resume activity {androidx.media3.demo.main/androidx.media3.demo.main.PlayerActivity}: java.lang.ArrayIndexOutOfBoundsException: length=3; index=3
...
Caused by: java.lang.ArrayIndexOutOfBoundsException: length=3; index=3
                                                                                                    	at androidx.media3.demo.main.IntentUtil.populateDrmPropertiesFromIntent(IntentUtil.java:178)

Because we assume the array has an even length here:

media/demos/main/src/main/java/androidx/media3/demo/main/IntentUtil.java

Lines 176 to 178 in 8ba44ad

	for (int i = 0; i < keyRequestPropertiesArray.length; i += 2) {
	headers.put(keyRequestPropertiesArray[i], keyRequestPropertiesArray[i + 1]);
	}

[hubert-mazur]

Ah, that way. Okay, this seems reasonable to fail if the argument is invalid. In that case, how about throwing an IllegalArgumentException out from getPlayerRepeatMode if the option can't get a valid match? This won't let the run app and confuse the user.

[icbaker]

Yep, sounds reasonable. IMO you can also remove the enum and just have a string-switch in a static method on IntentUtil:

private static @Player.RepeatMode int parseRepeatMode(String repeatMode) {
  switch (repeatMode) {
    case "ALL":
      return Player.REPEAT_MODE_ALL
    ...
    default:
      throw new IllegalArgumentException("Unrecognized repeat mode: '" + repeatMode "');
}

[hubert-mazur]

Thanks Ian for the hints! I pushed the changes.

[marcbaechinger]

I'm going to send this for internal review now. You may see some more commits being added as I make changes in response to review feedback. Please refrain from pushing any more substantive changes as it will complicate the internal review - thanks!