Crash when calling stop() on a mediaController playing ads

[spocky]

Version

Media3 pre-release (alpha, beta or RC not in this list)

More version details

No response

Devices that reproduce the issue

Nvidia Shield

Devices that do not reproduce the issue

No response

Reproducible in the demo app?

Not tested

Reproduction steps

My app is an Android Tv launcher that fetches active sessions to get the list of android.media.session.MediaController media controllers, transforms them to androidx.media3.session.MediaController and uses them to show a "now playing" section, as well as a few controls (play/pause, next, previous, stop).

There's a crash when calling stop() while an ad is currently playing (currently tested on Spotify)

The reason is quite simple looking at the code : in the stop() implementation, the call to createSessionPositionInfo here hardcodes the parameter isPlayingAd to false.
But then, while creating the SessionPositionInfo object, a check here verifies if an ad is currently playing and throws if it doesn't match the passed parameter.

Expected result

No crash when stopping a mediaController even if an Ad is playing.

Actual result

java.lang.IllegalArgumentException
androidx.media3.common.util.Assertions.checkArgument(Assertions.java:40)
androidx.media3.session.SessionPositionInfo.<init>(SessionPositionInfo.java:83)
androidx.media3.session.MediaControllerImplLegacy.createSessionPositionInfo(MediaControllerImplLegacy.java:2487)
androidx.media3.session.MediaControllerImplLegacy.stop(MediaControllerImplLegacy.java:188)
androidx.media3.session.MediaController.stop(MediaController.java:647)

Media

Not applicable. Only depends on an ad playing, so streaming app like Spotify non-premium is probably mandatory.

Bug Report

• You will email the zip file produced by adb bugreport to android-media-github@google.com after filing this issue.

[marcbaechinger]

Thanks for the report and the analysis. That's spot on.

From looking at the code, it looks to me like that, if an app sets MediaMetadataCompat.METADATA_KEY_ADVERTISEMENT then the PositionInfo passed into createSessionInfo in stop() has the isPlayingAd flag set and the adGroupIndex is set to 0 (zero). We convert from the legacy metadata in LegacyConversions.convertToIsPlayingAd(MediaMetdataCompat).

The bug is then that stop blindly hard codes the isPlayingAd flag with false. Instead the value should be derived from the existing PositionInfo. Something like

createSessionPositionInfo(
   controllerInfo.playerInfo.sessionPositionInfo.positionInfo,
   /* isPlayingAd= */ controllerInfo
       .playerInfo
       .sessionPositionInfo
       .positionInfo
       .adGroupIndex
           != C.INDEX_UNSET,
    ... ,
)

This matches to what we already do for other properties passed in there and passes a regression test I created.

Will send for review and update this issue when the fix has landed in the main branch.

[marcbaechinger]

The commit above fixes the problem. I'm closing this issue. Please open a new issue if you still see problems around this.