Repository navigation
Migrate Media I guide snippets - #1115
barbaralaw wants to merge 11 commits into
Conversation
|
Here is the summary of changes. You are about to add 18 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
This branch has not been deployed
2 participants
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.18 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 20 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_playback_serviceandplayback_app_on_get_session: 3 comments end with a full stop.playback_app_connect_ui: addssuper.onStart()as the first line ofonStart(), because anActivitythat overridesonStart()must call it or the app crashes. Indentation goes from 2 to 4 spaces (formatter).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
5 snippets (4 new, plus the existing
playback_app_create_exoplayer), 2 match, 3 differ.playback_app_playback_serviceMediaSessionServiceplayback_app_on_get_sessionMediaSessionServiceplayback_app_connect_uisuper.onStart()as the first line ofonStart(). Indentation 2 → 4 spacesMatching as published:
playback_app_create_exoplayer,playback_app_create_media_session.playback_app_on_get_sessionsits insideplayback_app_playback_servicein source. The outer region hides it with a silent exclude so each tag is opened only once, and both render as the page shows.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. The region sits inside aprivate object CustomControlsSnippetwrapper (outside the tags) to avoid clashing with playback-app'sPlaybackService.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.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.onStart()never callssuper.onStart(), so the activity throwsSuperNotCalledException. The region adds the call.On pages or sections that stay hardcoded (not fixed here; listed for the page owner):
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 PlaybackServiceinPlaybackApp.ktis public because its declaration line is inside theplayback_app_playback_serviceregion; 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.