Repository navigation
Migrate Media I guide snippets - #1115
barbaralaw wants to merge 17 commits into
Conversation
|
Here is the summary of changes. You are about to add 16 region tags.
This comment is generated by snippet-bot.
|
| * limitations under the License. | ||
| */ | ||
|
|
||
| package com.example.media |
There was a problem hiding this comment.
can we have android in the package name for these files? com.example.android.media?
There was a problem hiding this comment.
Thanks! I kept com.example.media to match the module: :media declares namespace = "com.example.media", and PlaybackApp.kt on main already uses that package. Most modules in the repo follow the same com.example.<module> pattern (only a handful use com.example.android.*), so the new files stay consistent with the rest of :media.
(Assistant.kt itself is gone. The Media legacy assessment marked that page as legacy, so it stays hardcoded.)
| androidx-lifecycle-viewmodel-ktx = { module = "androidx.lifecycle:lifecycle-viewmodel-ktx", version.ref = "androidx-lifecycle-compose" } | ||
| androidx-lifecycle-viewmodel-navigation3 = { module = "androidx.lifecycle:lifecycle-viewmodel-navigation3", version.ref = "androidx-lifecycle-viewmodel-navigation3" } | ||
| androidx-material-icons-core = { module = "androidx.compose.material:material-icons-core" } | ||
| androidx-media = { module = "androidx.media:media", version.ref = "androidx-media" } |
There was a problem hiding this comment.
I'm wondering if we should add this dependency or not (if this is the old version of the media library). Posted a question on our group chat, curious for your thoughts!
There was a problem hiding this comment.
Good catch. It's removed. The only snippets that needed androidx.media were on the legacy pages (Assistant and the pre-Android 13 media controls), and the legacy assessment dropped those from this PR. The one remaining use was compile-only scaffolding in AudioFocus.kt, outside the region tags, and it now uses the platform android.media.session.MediaController instead. The PR no longer adds androidx-media to libs.versions.toml or media/build.gradle.kts.
|
|
||
| @OptIn(UnstableApi::class) | ||
| // [START android_media_surfaces_mobile_custom_command_buttons] | ||
| class CustomControlsPlaybackService : MediaSessionService() { |
There was a problem hiding this comment.
Could we keep the rendered class name as class PlaybackService : MediaSessionService() inside the region tag and avoid the collision by wrapping the snippet in a private object outside the region tag?
```kotlin
private object CustomControlsSnippet {
// [START android_media_surfaces_mobile_custom_command_buttons]
class PlaybackService : MediaSessionService() {
...
}
// [END android_media_surfaces_mobile_custom_command_buttons]
}
There was a problem hiding this comment.
Done, thanks for the suggestion. SurfacesMobile.kt now wraps the region in private object CustomControlsSnippet, and the rendered class is class PlaybackService : MediaSessionService(), the same as the page.
|
Moving this back to draft while I apply the outcome of the Media legacy assessment and get an internal review. I'll address your comments and re-request your review once it's ready. Thanks for your patience! |
e770b58 to
6591435
Compare
- Remove the Assistant, pre-Android 13 and legacy mobile surfaces snippets. Those pages and sections stay hardcoded on DAC as legacy. - Remove the androidx-media dependency. AudioFocus.kt now uses the framework MediaController in non-rendered code. - Keep the rendered class name PlaybackService by wrapping the custom command buttons snippet in a private object (review feedback). - Reuse playback-app's create_media_session region for the Android TV page instead of a duplicate region. - Open audio_focus_request and playback_service once each, using silent excludes. Rendered output is unchanged. - End natural-language comments inside regions with a full stop.
Show super.onStart() in connect_ui, move the saveToFavorites stub out of the region, drop the unused core-ktx dependency, and register PlaybackService in the module manifest.
6f8ed53 to
efe932e
Compare
kkuan2011
left a comment
kkuan2011
left a comment
There was a problem hiding this comment.
Looks good after comments addressed, thanks!
| private const val ACTION_FAVORITES = "ACTION_FAVORITES" | ||
|
|
||
| // Wrapper avoids a collision with PlaybackService in PlaybackApp.kt. | ||
| private object CustomControlsSnippet { |
There was a problem hiding this comment.
nit: is this private obj wrapper needed?
There was a problem hiding this comment.
Good catch, it's not needed anymore. Removed it. PlaybackService is now a top-level class. The unpublished service that holds on_get_session in PlaybackApp.kt is renamed OnGetSessionService, so the two class names no longer collide. The rendered snippet is unchanged.
| } | ||
|
|
||
| // Remember to release the player and media session in onDestroy. | ||
| override fun onDestroy() { |
There was a problem hiding this comment.
This also uses the old way of overriding the onDestroy method, so let's omit this snippet, and add it to the list to come back to it later
There was a problem hiding this comment.
Done, and added to the tracking sheet
| private lateinit var playerView: PlayerView | ||
|
|
||
| // [START android_media_playback_app_connect_ui] | ||
| override fun onStart() { |
There was a problem hiding this comment.
this also needs to be updated in a later pass to fix the activity lifecycle method override
There was a problem hiding this comment.
Done, and added to the tracking sheet.
…apper Per review, the playback_service (onDestroy override) and connect_ui (Activity onStart override) snippets use outdated lifecycle patterns. They stay hardcoded on the playback-app page and will be reworked and migrated in a follow-up. on_get_session stays migrated. Its unpublished enclosing service is renamed OnGetSessionService, so SurfacesMobile.kt's PlaybackService no longer collides and the private object wrapper around it is removed. Rendered custom_command_buttons text is unchanged.
Summary
Extracts the Kotlin samples from five Media I guides (editing, playback, Android TV, media controls and audio focus) into
:mediaas region-tagged source, so the guides can import them instead of hardcoding them.16 new region tags across 5 pages. The Android TV page reuses playback-app's
create_media_sessionregion because it prints the same block. Of the 18 snippets the pages import, 9 publish exactly what the page shows. The rest differ for a reason listed below.Region tags all begin
android_media_; the lists and tables drop that prefix.Code snippets are for:
List of modifications
editing_set_hdr_mode: the closing)ofImmutableList.of(videoSequence)moves to its own line (formatter).editing_trim_videoandediting_custom_effects: 3 comments end with a full stop.editing_preview_audio_effects: drops theenableOffloadparameter and.setOffloadMode(...), which current Media3 no longer has, so the published code doesn't compile. The builder is re-wrapped (formatter).playback_app_on_get_session: 1 comment ends with a full stop.playback_app_create_media_session.surfaces_mobile_custom_command_buttons: indentation goes from 2 to 4 spaces, and the under-indentedonConnectbody is aligned with its function (formatter). 1 comment ends with a full stop.audio_focus_request: thesetAudioAttributes(...)argument moves onto its own lines (formatter). 5 comments end with a full stop.audio_focus_request_pre_o: adds the missing:inlateinit var afChangeListener: AudioManager.OnAudioFocusChangeListener, so the code compiles. The bare...becomes// ..., and therequestAudioFocusarguments are indented 4 instead of 8 (formatter). 2 comments end with a full stop.audio_focus_change_listener:Handler()becomesHandler(Looper.getMainLooper()), because the no-argument constructor is deprecated and the page text doesn't name it. 7 comments end with a full stop.The per-page tables below give the same changes with the page section for each snippet.
Scope after the Media legacy assessment
This PR was paused while the team decided which Media pages are legacy. Following that assessment:
onGetRootand the pre-Android 13 notification) stays hardcoded.androidx-mediadependency is removed, since only the legacy snippets needed it.How to read the "why" column
spotlessApplynormalizes indentation to 4 spaces and wraps long calls.// ......, which Kotlin will not compile. The region publishes// ....Per page
1. Create a basic video editing app using Media3 Transformer
https://developer.android.com/media/implement/editing-app
8 snippets, 4 match, 4 differ.
editing_set_hdr_mode)ofImmutableList.of(videoSequence)on its own lineediting_trim_videoediting_custom_effectsediting_preview_audio_effectsenableOffloadparameter frombuildAudioSinkand.setOffloadMode(...)fromDefaultAudioSink.Builder; the builder is rewrappedMatching as published:
editing_transcode,editing_built_in_effects,editing_preview_effects,editing_start_transformation.2. Create a basic media player app using Media3 ExoPlayer
https://developer.android.com/media/implement/playback-app
3 snippets (2 new, plus the existing
playback_app_create_exoplayer), 2 match, 1 differs.playback_app_on_get_sessionMediaSessionServiceMatching as published:
playback_app_create_exoplayer,playback_app_create_media_session.playback_app_on_get_sessionsits inside an unpublishedOnGetSessionServiceclass in source, becauseonGetSessionhas to override aMediaSessionServicemethod. The class name is outside the region, so it doesn't clash withSurfacesMobile.kt'sPlaybackService.3. Extend your media app to Android TV
https://developer.android.com/media/implement/surfaces/android-tv
1 snippet, matches as published. The page's MediaSession block is identical to playback-app's, so the page imports
playback_app_create_media_sessioninstead of a duplicate region.4. Media controls
https://developer.android.com/media/implement/surfaces/mobile
1 snippet migrated; the legacy section stays hardcoded (see Scope above).
surfaces_mobile_custom_command_buttonsonConnectbody is indented to match its function; 1 comment gets a full stopThe rendered class name stays
PlaybackService, as on the page, and it's a top-level class.5. Manage audio focus
https://developer.android.com/media/optimize/audio-focus
5 snippets, 2 match, 3 differ.
audio_focus_requestsetAudioAttributes(...)argument moves onto its own lines; 5 comments get a full stopaudio_focus_request_pre_olateinit var afChangeListener AudioManager.OnAudioFocusChangeListenergets its missing:;...→// ...;requestAudioFocusarguments indented 4 instead of 8; 2 comments get a full stop// ..., ktlint wrap, full stopaudio_focus_change_listenerHandler()→Handler(Looper.getMainLooper()); 7 comments get a full stopMatching as published:
audio_focus_abandon,audio_focus_delayed_stop_runnable.The page shows the request setup and
onAudioFocusChangeas one continuous block, but in source the override has to live on a type that implementsAudioManager.OnAudioFocusChangeListener(private inner class AudioFocusHandler).audio_focus_requestis one region, and a silent exclude hides the end of the setup function and the inner-class header between the two parts. The hidden}and{balance each other, so the rendered block is exactly the page's block (plus the changes listed above).Snippets not migrated
MediaSessionServiceandMediaLibraryServiceblocks, because they are in the Android manifest file.MediaSessionServiceKotlin block under Implementing aMediaSessionServiceand the Kotlin block under Connecting to your UI (and their Java versions). They use outdatedServiceandActivitylifecycle overrides (onDestroy,onStart), so they stay hardcoded and will be reworked and migrated in a follow-up.The Java versions of the migrated Kotlin samples aren't migrated. The paired guide-page change removes them, following Kotlin-first guidance.
Other Media I pages not in this PR:
Dependencies
One entry added to
gradle/libs.versions.toml:androidx-media3-transformer, which reuses the existingmedia3version throughversion.ref.:medianow also depends onmedia3-common,media3-effect,media3-session,media3-transformer,media3-uiandguava-android, each used by a snippet in this PR.Live snippet defects this surfaced
Fixed in the migrated regions:
lateinit var afChangeListener AudioManager.OnAudioFocusChangeListeneris missing:.buildAudioSinkstill takesenableOffloadand callsDefaultAudioSink.Builder.setOffloadMode. Neither exists in this module's Media3. Offload is now configured withTrackSelectionParameters.AudioOffloadPreferences, which doesn't belong in this processor-preview sample, so the region publishes the current three-argument override.On pages or sections that stay hardcoded (not fixed here; listed for the page owner):
onStart()never callssuper.onStart(), so the activity throwsSuperNotCalledException. Fix it in the connect UI follow-up.stateActionschain starts withor, so Kotlin ends the statement afterACTION_PLAYand the next lines don't compile.object: MediaSession.Callback()sample is the platformandroid.media.sessioncallback, butsessionin that section is aMediaSessionCompat, whosesetCallbacktakes aMediaSessionCompat.Callback.onGetRootstarts with...and never closes the function}.Transformer.Builder.setTransformationRequest()andTransformationRequest.HDR_MODE_TONE_MAP_HDR_TO_SDRno longer exist in this module's Media3. The successor isComposition.Builder.setHdrMode(HDR_MODE_TONE_MAP_HDR_TO_SDR_USING_OPEN_GL), but porting the block would change what the page teaches while the prose still describes the old flow.MediaFormatblocks callformat.setInt32(...), which isn't a Java API; it should beformat.setInteger(...).Notes for reviewers
class PlaybackServiceinSurfacesMobile.ktis public because its declaration line is inside thesurfaces_mobile_custom_command_buttonsregion; making it private would change the rendered snippet.:media's ownAndroidManifest.xmlnow registers thatPlaybackService, the way an app would: exported, with theandroidx.media3.session.MediaSessionServiceintent filter,foregroundServiceType="mediaPlayback", and theFOREGROUND_SERVICEandFOREGROUND_SERVICE_MEDIA_PLAYBACKpermissions (android:exportedfrom target SDK 31; the service type andFOREGROUND_SERVICE_MEDIA_PLAYBACKfrom 34). It's module configuration, not a snippet; the page's own manifest block stays hardcoded.custom_command_buttons, the requiredonGetSessionoverride sits inside the class, hidden with a silent exclude. The compile-onlysaveToFavorites(...)stub is a private top-level function outside the region.Verification
Rebased on
main(media3 1.11.1)../gradlew :media:spotlessCheckpasses./gradlew :media:compileDebugKotlinpasses./gradlew :media:lintDebugpassesincludecoderegion parser and compared with the live page block; the differences are exactly the ones listed above.