feat(android): export redesign, in/out marks on the timeline + export from playback (#617) - #621
Open
badbread wants to merge 3 commits into
Open
feat(android): export redesign, in/out marks on the timeline + export from playback (#617)#621badbread wants to merge 3 commits into
badbread wants to merge 3 commits into
Conversation
…t filename A cancelled export job (server status "cancelled") was never recognized as terminal by ExportJob.isTerminal, so ExportViewModel.startPolling kept polling it forever and the status UI showed a spinner instead of a "Cancelled" state. Add ExportJob.isCancelled and fold it into isTerminal; the poller now stops quietly (no error toast) and the status label shows "Cancelled". ExportOutputFile.filename (the server's real on-disk basename, e.g. .mkv or a whole-job .zip) was never decoded on Android, so saved/shared exports were always named "...mp4" with a hardcoded video/mp4 MIME type regardless of the actual container. Decode filename and derive the extension/MIME type from it at both save sites (Downloads save + cache-then-share), falling back to mp4 only when the server didn't report a filename. Fixes #619 Signed-off-by: badbread <badbread@users.noreply.github.com>
… from playback (#617) Android export was a stand-alone screen whose only entry point was the Download icon on the playback wall, with a time range editable solely by two one-minute +/- steppers. There was no way to express a second-accurate window even though the server accepts one, no export entry on the single-camera playback screen at all, and the route could not be seeded, so the camera and moment being reviewed were dropped on the way in. Phase 1 brings Android in line with the desktop and iOS clients: - In/out selection on the playback timeline. `PlaybackUiState` carries raw `exportSelStartMs`/`exportSelEndMs` epoch millis (ordered only on read, so dragging one handle past the other never swaps which handle the finger holds, matching the desktop controller and the iOS view). `CenteredTimeline` draws the bracket as a translucent amber region with a solid handle at each edge, and a press within grab range of a handle resizes that edge instead of scrubbing. Scrub and pinch are untouched. Touch has no Shift+drag, so "Set in point" / "Set out point" / "Clear selection" place and clear the bracket from the transport menu. - An "Export selection" bar under the video whenever a bracket exists, showing exact start, end and duration to the second, with Clear and Export, mirroring the desktop bar. - A seeded export route: `Routes.EXPORT` now takes optional `cameraId`, `startMs` and `endMs`. Playback passes the bracket, or the hour ending at the playhead when nothing is marked (iOS `exportRange()`); the wall passes the hour ending at its scrub cursor. - The one-minute steppers are gone. Each boundary is a tappable second-accurate field backed by `JumpToDateTimeDialog` (new `preserveSeconds` option so a seeded selection is not rounded off), plus Last 1m/5m/10m/15m chips and a one-SECOND nudge. `start < end` is enforced in the ViewModel and gates Create. - A Cancel button for a running job, binding `DELETE /export/{job_id}` through `CrumbApi`/`CrumbRepository`; the poll loop now also treats a server-side "cancelled" status as terminal. Selection arithmetic lives in a new pure `ExportRange` object shared by the timeline and the export screen so the two cannot drift, with unit tests for normalization, edge seeding/clamping, the export-range fallback, quick-chip math and route arg encoding. Deferred to a follow-up (noted on the PR): the batch clip list via `POST /export/batch`, output options (codec/container/audio/password), and the filmstrip preview. Part of #617 Signed-off-by: badbread <badbread@users.noreply.github.com>
…me' into feat/android-export-redesign Signed-off-by: badbread <badbread@users.noreply.github.com> # Conflicts: # apps/android/app/src/main/java/video/crumb/app/feature/export/ExportViewModel.kt
Owner
Author
|
Rebased onto #620: this branch now carries #620's commit (c759630) via merge 82ab281, so it merges cleanly after #620 (or merging this alone brings both; GitHub will mark #620 merged). Merged branch re-gated on dev2: compileDebugKotlin + testDebugUnitTest green. A combined debug APK (main + #620 + #621) was built from 82ab281 for on-device verification. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Phase 1 of the Android export redesign (#617): bring Android's export flow in line with the desktop and iOS clients.
The problem
Android export was a stand-alone screen (
feature/export/) whose only entry point was the Download icon on the playback wall. The single-camera playback screen had no export entry at all. The route took no arguments, so even the wall entry dropped the camera and moment being reviewed. The time range was editable solely by two[-] [+]steppers moving in one-minute steps, which made a second-accurate window impossible to express, even though the ViewModel already clamps to a 1 s minimum and the server accepts RFC-3339 with fractional seconds. There was also no way to stop a running job.What this does
1. In/out selection on the playback timeline.
PlaybackUiStategains rawexportSelStartMs/exportSelEndMsepoch millis. They are stored exactly as the operator placed them and ordered only when read, so dragging one handle past the other never swaps which handle the finger is holding, the same model as the desktopplayback_timeline_controller(selStartMs/selEndMs, no snapping) and the iOSCenteredTimelineView.CenteredTimelinedraws the bracket as a translucent amber region with a solid handle at each edge (mirrors iOS 2c/2e; newTimelineColors.exportFill/exportHandlein the existing amber family), and a press landing within grab range of a handle resizes that edge instead of scrubbing. Nearest handle wins on overlap, as on iOS.Scrub and pinch behaviour is unchanged: the handle path returns from the gesture before
onScrubStart()is ever called, and it never moves the playhead.2. Touch-friendly marking. There is no Shift+drag on a phone, so Set in point / Set out point / Clear selection live with the other secondary actions (the 3-dot overflow in portrait, a "mark for export" menu on the transport row in landscape). Each places its end at the playhead and seeds the other end from the playhead, matching iOS
setExportEdge; the timeline handles then fine-tune it.3. "Export selection" bar. While a bracket exists, a bar under the video shows the exact start, end and duration to the second, with Clear and Export, mirroring the desktop's bar (same muted amber ground). The end drops its date when the range stays inside one day.
4. Seeded export route.
Routes.EXPORTnow takes optionalcameraId,startMs,endMs, with aRoutes.export(...)builder that percent-encodes the camera id (never form-encoding's+, which a URI reads as a literal plus). Playback passes the marked bracket, or the hour ending at the playhead when nothing is marked (iOSexportRange()); the wall passes the hour ending at its scrub cursor (the wall has no single focused camera, so it seeds the window only).ExportScreenopens pre-filled.5. The one-minute steppers are gone. Each boundary is now a tappable second-accurate field backed by
JumpToDateTimeDialog(new opt-inpreserveSeconds, so opening the picker on a seeded selection does not silently round its seconds off), plus Last 1m / 5m / 10m / 15m quick chips (the desktop export builder's set) and a one-second nudge stepper on each side.start < endis enforced in the ViewModel and now gates Create.6. Cancel a running job.
DELETE /export/{job_id}is bound inCrumbApi/CrumbRepository(an unsuccessful status becomes a failure rather than a silent no-op) and surfaced as a Cancel button while the job is still stoppable. The poll loop also treats a server-sidecancelledstatus as terminal, so it can't spin forever on one, and a deliberate cancel shows as a neutral notice rather than a red failure.Selection arithmetic lives in one new pure object,
feature/export/ExportRange.kt, shared by the timeline and the export screen so the two cannot drift.Deferred to a follow-up (Phase 2, still under #617)
POST /export/batch(Android remains single-shot; desktop/iOS have the list).Notes for review
Models.kt'sExportJob/ExportOutputFileandExportScreen's hardcoded.mp4save names are deliberately untouched (a concurrent change owns those). The cancel path therefore readsstatus == "cancelled"through a local private extension rather than assuming a model-level flag, and composes with anisCancelledproperty landing later.docs/COMPONENT-MAP.mdexport row,docs-site/docs/playback/export.md(new "Marking the moment you want" section) anddocs-site/docs/clients/android.md.Verification
Verified: compile + unit tests green on dev2 (
:app:compileDebugKotlin testDebugUnitTest, BUILD SUCCESSFUL); 27 new unit tests cover selection normalization, edge seeding/clamping/future-clamping, the export-range fallback, range validation, quick-chip math and route arg encoding. On-device visual check recommended (bracket rendering, handle drag vs. scrub, the selection bar in both orientations).Part of #617