Skip to content

Keep video messages editable at the limit and fix recorder controls - #92

Open
elaylama wants to merge 11 commits into
TelegramMessenger:masterfrom
elaylama:fix/video-recording-editor
Open

elaylama wants to merge 11 commits into
TelegramMessenger:masterfrom
elaylama:fix/video-recording-editor

Conversation

@elaylama

@elaylama elaylama commented Oct 25, 2026 •

Copy link
Copy Markdown
Contributor

Key change

Prevent accidental sends at the 60-second limit and let users review, trim and explicitly send their video. Reaching the limit now pauses in the editor instead of automatically sending the entire unedited recording. Users can send only the selected fragment, discard it, or shorten it and choose Record More.

The recorder also handles the actions around that pause:

  • Send preserves the complete recording. If the native writer is still finishing the paused segment, Send waits for it before composing the result. The first take is not discarded as too short, and the latest resumed segment is included. Repeated Send requests do not finalize the writer again; late completions after discard or writer failure are ignored.
  • Record More continues from the retained duration. Trimming a 60-second video to 10 seconds starts continuation from 10, avoiding an immediate pause caused by the stale 60-second value. Record More remains disabled until the selection is shorter than the limit.
  • The paused editor stays usable. Both trim handles work in both directions; click/drag seeking and Play/Pause stay within the selection. Confirmations release pointer events, dismissal cleans up the overlay/player, and the camera status observer survives dismissal to process late terminal events.

This intentionally changes the previous macOS limit behavior. Releasing the original recording gesture does not send the paused video.

Reason

Previously, reaching 60 seconds chose Send for the user, leaving no opportunity to review or trim. The editor also had pointer-routing and video-playback failures. A further race allowed Send to inspect the recording before pause finalization had added the current segment: a complete first take could produce no finished video, or a resumed take could omit its newest segment.

The follow-up fixes that race and replaces a resume test that manually supplied the expected duration with a test of the actual pipeline. Related #82 optimizes thumbnail allocation in the same preview flow; the original editor failures reproduced with and without it. This PR does not include that optimization or a dictation implementation.

Screenshot/video result (before/after)

Before

Before: video timeline without playback controls

After

After: playback controls and selected fragment

These native AppKit fixture captures use the same generated placeholder, 10-second duration, 2–8 second selection and 4-second position. They show editor controls, not the new timing scenarios. Earlier full-client captures show the editor playing and paused.

Timing and submission behavior are compared by regression output rather than static screenshots:

Scenario Before After
Controller receives 60 seconds 1 submission, 1 reset, no paused editor 0 submissions/resets, 1 paused editor update
Send before the first paused writer finishes Stopped; no finished video One complete 60-second result
Resume after real trim/export from 60 to 10 seconds Old test manually supplied 10 and could miss a regression Actual resume must publish 10 before recording status; removing that publication fails with 60

Confirmation

Environment: macOS 27.0.1 (26A434), Apple M4 Pro arm64, Xcode 27.1 / Swift 6.4; Telegram Master Test 13.1.0 (283357), Debug arm64. Master base: e1883098; latest code: 1a6e4032.

  • 29 focused native tests pass, including eight actual-pipeline cases: pending first/second segment, ordinary/repeated Send, real trim/resume, discard, premature resume and writer failure with late completion. The first Send regression failed on three assertions before the fix while the old 22 tests passed. A targeted mutation of production resume() fails at 60 versus the expected 10 seconds; the source was restored before the final passing run.
  • Full Debug arm64 build and installed signature verification pass. The tested/installed source matches the committed code by SHA-256.
  • Latest manual client check: I confirmed Send after 60 seconds and continuation after trimming with both follow-up changes installed. Earlier client checks separately confirmed ordinary/trimmed delivery, limit-to-editor without automatic sending, playback, explicit Send/phone playback, window/camera cleanup and three repeated editor/discard cycles.
  • Scope: automated pipeline tests use generated files and controlled writer callbacks with real composition/trim/export/resume. Controller tests use capture/view probes. They do not use a physical camera, microphone, account or network. The manual run does not measure how often the native writer race occurs. Device-disconnect/error paths and a separate held-gesture release run were not newly confirmed. Results are local, not GitHub CI. Local Xcode/signing/profile adjustments are excluded; sending and recording pre-upload were enabled. No app-wide performance gain is claimed.

Reproduction and scope · Verification

  • I confirm that I have tested the affected behavior and verified that it works properly, or verified the content and links for a documentation-only change.
@elaylama

Copy link
Copy Markdown
Contributor Author

I tested #82 and #92 together in the full signed-in Telegram Master Test client on current master 272d85dd. Preview images/thumbnails, both trim handles, seeking, full/selected Play/Pause and repeated No/Discard/new-recording cycles all work correctly.

I also confirmed that seeking and the playhead stay inside the range selected with the left and right trim handles. I added two real-client screenshots to the description: playing and paused, separate from the native Before/After comparison.

The combined full build, signature verification and all 15 editor tests pass.

@macosswift

Copy link
Copy Markdown
Collaborator

Issues:

REQUIRED CHANGE: VideoRecorderModalController.swift:195 - keep the pipeline status observer (or call stopCapture) when the modal closes; the camera now stays on until deinit (major, CONFIRMED)

@elaylama
elaylama marked this pull request as draft October 27, 2026 09:01
@elaylama

Copy link
Copy Markdown
Contributor Author

Fixed in 39b2e335: the pipeline status observer stays active until deinit, allowing late completion, discard or failure to stop capture after dismissal. Preview/timer cleanup is unchanged.

  • Tests: the three new controller-source regression cases fail before the fix; all 18 focused tests and the full Debug arm64 build pass on master 605ade9e. The regression fixture uses capture/view probes, not hardware.
  • Device check: six capture sessions reported isRunning=true at start and false at stop. Manual pause/discard closed the recorder and switched off the camera indicator. This ran on e537320b + the fix; recorder code is identical in the final build, but the hardware check was not repeated there.
  • Limits: cancellation required manual interaction because UI automation could not expose the recorder window. A local-only guard disabled media sending and recording pre-upload; it is excluded from the PR. Normal send/upload/export remains untested. An earlier unguarded run auto-completed recordings into Saved Messages, as documented in the verification notes.

Keeping this PR in Draft.

@elaylama

Copy link
Copy Markdown
Contributor Author

Manual follow-up on master 605ade9e + #92 in Telegram Master Test 13.1.0 (283357), Debug arm64, macOS 27.0.1:

  1. Recorded 5–10 seconds, paused, and moved both trim handles inward and back without sticking.
  2. Checked full and selected-fragment playback, repeated Play/Pause, and click/drag seeking. Playback and the playhead stayed within the selected range.
  3. Chose No in the discard confirmation; the editor remained usable.
  4. Confirmed discard: the recorder window closed, the camera indicator switched off, and the main interface accepted clicks.
  5. Repeated the cycle three times; new recordings started normally, with no stale windows or confirmations.

All five checks passed. This follow-up covers video editing and cancellation. Media sending and recording pre-upload remained disabled by the local-only guard, which is excluded from the PR. Normal send/upload/export and completion at the recording time limit were not tested. Keeping the PR in Draft.

@elaylama elaylama changed the title Fix video recording trim controls, playback and dismissal Oct 27, 2026
@elaylama
elaylama marked this pull request as ready for review October 27, 2026 12:16
@elaylama

Copy link
Copy Markdown
Contributor Author

Updated the PR with an intentional change at the 60-second limit: recording now pauses in the editor instead of sending the entire video automatically. This prevents accidental sends caused by reaching the limit and lets users review or trim the video and explicitly send only the selected fragment, or discard it.

All 22 focused native tests, the full Debug arm64 build and signature checks pass on master e1883098 + code d33bf763. I also manually confirmed the new 60-second editor, playback, trimming, explicit delivery/playback on the phone and camera/window cleanup after sending. Ordinary and trimmed sending had also passed before this change. The description and verification file distinguish native tests, manual checks and remaining coverage limits.

Ready for review.

Keep Send pending while the native writer finishes so the final segment is preserved. Replace the manually primed resume case with real pipeline trim/resume coverage.

Verified with 29 native tests, a targeted resume mutation, a full Debug arm64 build and manual Send/resume checks.
Record the failing race regression, 29 passing native tests, targeted resume mutation and full Debug build. Separate the latest manual Send/resume confirmation from earlier physical camera and delivery checks.
@elaylama

Copy link
Copy Markdown
Contributor Author

Updated the pending-pause Send path and strengthened resume coverage.

  • Send now waits for the current segment to finish, preserving the complete take, including the newest segment after Record More. Repeated Send does not finish the writer twice; discard/error paths reject late completion.
  • The resume regression now exercises the real pipeline: pause a generated 60-second video, actually trim/export it to 10 seconds, then resume and observe the retained duration before recording status. The old test manually supplied that duration and could hide a regression.

Verification: 29 native tests pass and the full Debug arm64 build passes. The new Send test fails before the fix; removing the production resume duration publication makes the new resume test fail at 60 instead of 10. I also confirmed Send after 60 seconds and continuation after trimming in the installed client.

The 60-second limit still opens the editor without sending: users can review, trim and choose what to send, avoiding accidental submission of the full recording. Automated tests use controlled writer I/O, not physical capture or delivery; the manual check does not measure race frequency. Device-disconnect/error paths were not newly confirmed. Details and scope are recorded in the PR description and verification file.

@elaylama

Copy link
Copy Markdown
Contributor Author

Fixed the camera shutdown issue. The pipeline status observer stays active after viewWillDisappear and is disposed in deinit, so completion, discard and failure still call stopCapture() after the modal closes.

The three regression tests failed before the fix and now pass with the controller still retained. All 29 focused native tests and the full Debug arm64 build pass. I also manually checked pause/discard in the client: the recorder closed and the camera indicator switched off. The retained-controller tests use capture probes.

@macosswift

macosswift commented Oct 31, 2026 •

Copy link
Copy Markdown
Collaborator

Issues:

  • VideoRecorderModalController.swift:134-138: VideoRecorderModalController is also used by the Stories recorder, whose StoryRecordingView only handles .recording status and has no paused editor, so a story video message now silently pauses at 60 seconds with a frozen timer and is no longer sent, where it used to be sent and closed automatically.

  • VideoRecorderModalController.swift:40: sendMedia is now dead: the only code that called it (the automatic submission at the 60-second limit) was removed, but the stored closure and the init parameter remain and both ChatController and StoryModalController still build and pass it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

2 participants