[TV] Make the app a first-class media-session citizen - #5740
Conversation
Generated by 🚫 Danger |
|
Claude finished @sztomek's task in 5m 41s —— View job Code review: TV media-session wiringRead the diff against
Overall: the approach is right — reusing the shared services through the runtime toggle instead of adding TV-specific plumbing, and the manifest block is a faithful copy of 1.
|
| Timber.plant(TimberDebugTree()) | ||
| } | ||
| notificationHelper.setupNotificationChannels() | ||
| PlaybackServiceToggle.ensureCorrectServiceEnabled(this) |
There was a problem hiding this comment.
FeatureFlag is never initialised on TV, so this always reads the compile-time default.
PlaybackServiceToggle.ensureCorrectServiceEnabled() → FeatureFlag.isEnabled(Feature.MEDIA3_SESSION), and FeatureFlag.isEnabled falls back to feature.defaultValue when no providers are registered:
findProviderForFeature<FeatureProvider>(feature)?.isEnabled(feature) ?: feature.defaultValueFeatureFlag.initialize(...) is only called from AppLifecycleObserver.setup() (line 115) and AutomotiveApplication. TvApplication calls neither — there is no FeatureFlag/FeatureProvider reference anywhere under tv/src. Feature.MEDIA3_SESSION.defaultValue is isDebugOrPrototypeBuild, so:
- debug / prototype TV builds → media3
PlaybackServiceenabled (this is what the emulator run in the description verified), - release TV builds →
LegacyPlaybackServiceenabled, and the Firebase remote flag and the dev toggle have no effect on TV at all.
The good news is it's self-consistent: MediaSessionManager.useMedia3Session reads the same uninitialised flag, so the enabled component and the session type can't disagree. But the release path (legacy MediaSessionCompat on TV) is the one that ships and is the one that hasn't been exercised.
Two suggestions:
- Either initialise the flags on TV the way wear does —
appLifecycleObserver.setup()immediately before this call (wear:PocketCastsWearApplication.setupApp()lines 109–111) — or state in the PR that TV deliberately ships the compile-time default, and verify aprototypebuild too, since the legacy service is what release users get. - Whichever you pick, keep/insert
FeatureFlag.initializebeforeensureCorrectServiceEnabled. If it is ever added after this line, the toggle would enable a component chosen from the default whileMediaSessionManager.useMedia3Session(lazy, first read instartObserving()) resolves from the initialised providers — a mismatch for that launch.
| <uses-permission android:name="android.permission.WAKE_LOCK" /> | ||
| <uses-permission android:name="android.permission.POST_NOTIFICATIONS" /> | ||
| <uses-permission android:name="android.permission.FOREGROUND_SERVICE" /> | ||
| <uses-permission android:name="android.permission.FOREGROUND_SERVICE_MEDIA_PLAYBACK" /> |
There was a problem hiding this comment.
Two notes on this permission block (neither blocking):
-
POST_NOTIFICATIONSis already contributed by a library manifest —modules/services/repositories/src/main/AndroidManifest.xmlline 5 declares it, so it merges into the TV APK regardless. Harmless to repeat it explicitly, just don't count on this line as the thing that fixed anything. Separately: nothing intv/srcrequests the runtime permission (nohasNotificationsPermission()/ request flow), so on API 33+ the playback foreground-service notification is posted but not displayed. That doesn't stop the FGS or the session — worth knowing before claiming the notification surface works. -
FOREGROUND_SERVICE_DATA_SYNC— the PR calls this out as out of scope, and I agree it's pre-existing, but note the gap is slightly sharper than "TV has no foreground-service permissions": the samerepositorieslibrary manifest merges inandroidx.work.impl.foreground.SystemForegroundServicewithandroid:foregroundServiceType="dataSync", so the TV APK declares a dataSync FGS with no matching permission. Reachable callers areDownloadEpisodeWorker(wrapssetForegroundAsync(...).get()in atry/catch→ degrades safely),FixDownloadsWorkerandOpmlImportTask(unguardedsetForeground(...)). TV doesn't trigger any of them today, so deferring is defensible — it's just a one-line addition if you'd rather close it here.
|
Tested this branch (4bbda40) on real hardware — a Philips 4K A1 (Android 11),installDebugProd . Results for the items listed as not-yet-verified: Startup and component state ✅ Media session ✅ Background continuation ✅ Remote media-key control ✅ Screen-off — behaves differently to a phone, and I don't think it's a defect Not tested: media resumption after reboot, Assistant voice control, and the release/legacy path — this was a debug build, so per the review comment MEDIA3_SESSION resolved to its isDebugOrPrototypeBuild default and only the media3 service was exercised. Happy to run a prototype build against LegacyPlaybackService if that's useful. One incidental note: the app installed under user User 10 on this device rather than the owner profile. Didn't seem to affect anything, just flagging it. |
|
While testing this branch I noticed there's still no way to control playback without leaving whatever you're browsing. My closed #5737 had a mini-player drawer on the remote's Options/MENU key — transport controls slide in over the current screen, Menu or Back dismisses. I've since rebased it onto current main and cut it down to reuse TvSeekBar and the existing tv_nothing_playing_* strings rather than the hand-rolled controls it originally had. It's one new file plus a small hook in TvScaffold. Is that something you'd want? Happy to open it as its own PR, or drop it if you have designs for this already. |
Description
The Android TV app already plays audio (through the in-process ExoPlayer) and ships a full Now Playing screen, but it was not a first-class media-session citizen: the TV manifest never declared
PlaybackService/LegacyPlaybackService, soMediaSessionManager.startServiceIfNeeded()could never resolve a media-browser service (resolveMediaBrowserServiceComponent()returned null and logged "No enabled media browser service found in manifest"). The consequence was no systemMediaSession, no foreground service, and therefore:MediaBrowsercontent tree for the system/Assistant.This PR wires TV up the same way the
wearmodule does — reusing the shared services via the runtime toggle rather than adding anything TV-specific:tv/src/main/AndroidManifest.xml): declaresPlaybackService(media3) andLegacyPlaybackService, bothenabled="false"withforegroundServiceType="mediaPlayback"and the media-browser / media3 intent-filters, plus theWAKE_LOCK,POST_NOTIFICATIONS,FOREGROUND_SERVICEandFOREGROUND_SERVICE_MEDIA_PLAYBACKpermissions they need.TvApplication.onCreate(): sets up the notification channels (the foreground service posts on the "Playback" channel — without itstartForegroundthrows on Android 8+) and callsPlaybackServiceToggle.ensureCorrectServiceEnabled(), which enables the correct service based on theMEDIA3_SESSIONfeature flag, before the existingplaybackManager.setup().Deliberately out of scope
MediaButtonReceiver— only forwards external broadcastACTION_MEDIA_BUTTONevents (Tasker/Automate); TV remote keys reach the session directly.wearomits it too, so we match wear.FOREGROUND_SERVICE_DATA_SYNC—wear/appdeclare it for WorkManager foreground workers. It's unrelated to media sessions and a pre-existing gap (TV had no foreground-service permissions at all before), so it's left for a separate change.Fixes PCDROID-727 https://linear.app/a8c/issue/PCDROID-727/mediasession-support
Testing Instructions
./gradlew :tv:installDebug).enabledComponents: …PlaybackService,disabledComponents: …LegacyPlaybackService.Smoke-tested on a Google TV emulator (
sdk_google_atv64_arm64, leanback)Startup wiring only — the behavioral outcomes below still need verifying on a real device:
FATAL EXCEPTIONfrom the newonCreate()calls.enabledComponents: PlaybackService,disabledComponents: LegacyPlaybackService—PlaybackServiceToggleran and enabled the media3 service.Not yet verified (steps 4–5 above): actual background / screen-off continuation, remote media-key / Assistant control, and media resumption on a real TV device.
Screenshots or Screencast
No UI change — the Now Playing screen already exists; this PR only wires up the media-session/foreground-service infrastructure. The verification artifact is the runtime component-enabled state above.
Checklist
./gradlew spotlessApplyto automatically apply formatting/linting)modules/services/localization/src/main/res/values/strings.xmlI have tested any UI changes...