Skip to content

NAT-549: Handle standardized-payload resume safely - #172

Merged
eugene-mux merged 67 commits into
feature/nat-534-hevc-standard-ingestfrom
em/nat-549-generated-resume
Oct 28, 2026
Merged

eugene-mux merged 67 commits into
feature/nat-534-hevc-standard-ingestfrom
em/nat-549-generated-resume

Conversation

@eugene-mux

@eugene-mux eugene-mux commented Oct 8, 2026 •

Copy link
Copy Markdown

Description

Resume standardized uploads using the retained validated payload and confirmed progress. Original and standardized files share chunk transport, including retries of overlapping byte ranges.

Changes

  • Persist standardized payload identity and ownership before transport; fail if completed output is lost or changed instead of regenerating it.
  • Separate payload selection/restoration from transport, preserve completed selections and acknowledged bytes across resumes and force restarts, and fail if the selected file disappears after transport starts.
  • Share acknowledgement validation and bounded retries, and remove preparation release waits and redundant transport flags.
  • Preserve original-file and legacy restoration behavior.
  • Allow portrait video to use landscape encoding with rotation metadata while preserving planned display dimensions.

Validation

  • 297 debug and 297 release unit tests pass; SDK/sample release builds and lint-vital checks pass. Existing tests cover the review changes; five obsolete release-wait/query tests were removed.
  • Earlier live validation: SDR conversion survived process death and pause/relaunch/resume; all 11 uploaded slices matched the retained output and the Mux test asset reached ready. The transport refactor has been validated locally; live uploads were not rerun.

Note

High Risk
Changes core upload persistence, resume blocking, and HTTP acknowledgement semantics for Direct Upload destinations—incorrect behavior could splice wrong bytes or leave destinations unusable.

Overview
This PR reworks resume and restart for uploads that use locally standardized (converted) payloads, and tightens how chunk PUTs are acknowledged and retried.

Resume & persistence: Standardized outputs are tracked with durable state (PayloadIdentity, preparation phase, owned file path, network_started). Progress and the selected file are kept across pause, process death, and forceRestart (now documented as replacing the attempt while keeping payload and acknowledged bytes—not resetting to byte zero). If a completed standardized file is missing, tampered with, or the destination may already have partial bytes, resume fails with UploadResumeBlockedException and can permanently block that Direct Upload URL instead of re-exporting or guessing offsets.

Transport: Chunk upload logic is split: UploadProtocol validates responses (308 Range, success codes), handles bounded Retry-After, and uses a no-redirect HTTP client; per-chunk retries move to UploadJobFactory with overlapping-range retries. ChunkWorker only performs a single PUT and reports progress via one-shot request bodies.

Preparation / encoding: Renames generated → standardized across the pipeline; original selection is persisted atomically before transport. SDR export encodes in landscape encoder dimensions while validation compares display size using rotation metadata (portrait encoding flag removed).

Manager behavior: Cached job restore no longer drops entries when the source file is missing at restore time (handled elsewhere in the new persistence model).

Reviewed by Cursor Bugbot for commit a62c556. Bugbot is set up for automated code reviews on this repo. Configure here.

@eugene-mux
eugene-mux marked this pull request as ready for review October 9, 2026 00:47
@eugene-mux
eugene-mux requested a review from a team as a code owner October 9, 2026 00:47
@eugene-mux
eugene-mux requested a review from daytime-em October 9, 2026 00:47
@eugene-mux
eugene-mux added this pull request to stack #166 October 9, 2026 00:47

@cursor cursor Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stale Bugbot comment from a previous run.

@cursor cursor Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stale Bugbot comment from a previous run.

@eugene-mux
eugene-mux force-pushed the em/nat-549-generated-resume branch from 23c52fc to 1b8e194 Compare October 16, 2026 18:08
@eugene-mux
eugene-mux force-pushed the em/nat-549-generated-resume branch from 1b8e194 to 18e2d78 Compare October 16, 2026 18:09

@cursor cursor Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stale Bugbot comment from a previous run.

Base automatically changed from em/nat-548-upload-preparation to feature/nat-534-hevc-standard-ingest October 22, 2026 21:04

@daytime-em daytime-em left a comment •

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think some of this new transport logic should be simplified. Going line-by-line doesn't make much sense, this is more overall design feedback:

  • Transport Logic can assume that overlapping PUTs are accepted (the same byte ranges can be uploaded multiple times without errors)
  • Generated Payloads and "Original Payloads" can share the same chunk response validation logic
  • We don't need to worry about blocking job resumes while the Transformer's resources are cleared. The Transformer releases MediaCodecs synchronously, and repeatedly re-starting an upload while it standardizes is not likely to be a production case
  • The logic that handles generated-file resume near the top of createUploadJob is kind of hard to follow. It might help to break it into helper functions. This might also help ensure that no work or cases are duplicated.
    • originalSelected is persisted, so you can skip all of the standardization-related logic if the original file was already selected
    • you only need to query() the upload endpoint if you are re-generating a standardized file
    • If standardization didn't finish during the last upload attempt (engine died or something), you can safely delete the standardized file and standardize again. Nothing will have uploaded.. This might already be assumed in code, but it's hard to tell.

Also can we use "Standardized" instead of "Generated?" That would be much clearer and match up with the terminology used elsewhere in the codebase

@daytime-em

daytime-em commented Oct 26, 2026 •

Copy link
Copy Markdown
Collaborator

Also, consider thinking of the upload process in terms of its overall state transitions: If the original file is selected, selection/standardization can't happen again. Once standardization finishes it can't be restarted. Once transport starts you can't standardize again (and have to fail the upload if the file was deleted, whether it's standardized or original). Once a chunk is started, it can be re-sent. Once a chunk is finished successfully, it can't be re-sent.

Following that process a little more literally in createUploadJob might make the function a little more maintainable, especially if sharing the same transport logic for both standardized and original files. The same goes for Upload Persistence, which might wind up simpler as well with fewer flags and checks required to detect orphaned files and blocked standardization attempts

@eugene-mux eugene-mux changed the title NAT-549: Handle generated-payload resume safely NAT-549: Handle standardized-payload resume safely Oct 27, 2026
@eugene-mux

Copy link
Copy Markdown
Author

Thanks Emily, really helpful review! Simplified the flow around the state transitions you described, shared transport and retries, removed the release waits, and switched to “Standardized” naming. Pushed the changes.

@eugene-mux
eugene-mux requested a review from daytime-em October 27, 2026 19:26

@cursor cursor Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stale Bugbot comment from a previous run.

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit f06f88f. Configure here.

@daytime-em daytime-em left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for taking the feedback! LGTM 👍

@eugene-mux
eugene-mux merged commit 93548a8 into feature/nat-534-hevc-standard-ingest Oct 28, 2026
2 checks passed
@github-actions github-actions Bot mentioned this pull request Nov 4, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants