Skip to content

feat(video): support batch upload of multiple videos - #109

Open
kavyp12 wants to merge 2 commits into
GargantuaX:mainfrom
kavyp12:feat/multi-video-batch
Open

feat(video): support batch upload of multiple videos#109
kavyp12 wants to merge 2 commits into
GargantuaX:mainfrom
kavyp12:feat/multi-video-batch

Conversation

@kavyp12

@kavyp12 kavyp12 commented Jul 13, 2026

Copy link
Copy Markdown

What

Adds support for uploading and processing multiple videos in one go. Until now the video page handled a single file at a time.

How it works

  • The file input now accepts multiple, and drag-and-drop accepts multiple files.
  • Selecting or dropping more than one video queues them and processes each sequentially through the existing single-file pipeline (setFilerunExport), auto-downloading every result.
  • A small batch-queue UI shows per-file status: pending / processing / done / error.
  • Single-video selection and image handoff keep their original behavior, so preset tuning and manual export still work exactly as before.

Why sequential (not parallel)

Video decode/encode is CPU/GPU-bound, so running one item at a time through the proven pipeline is the correct design rather than a compromise — parallel encodes would just contend for the same resources.

Scope of changes

  • public/video-preview.htmlmultiple on the file input + batch-queue markup and styles
  • src/video-app.js — batch queue, sequential processing, auto-download, graceful fallback to single-file mode

No new dependencies. No changes to the watermark-removal algorithm or the single-file flow.

Selecting or dropping more than one video now queues them and processes
each sequentially through the existing single-file pipeline, auto-
downloading every result. A batch queue UI shows per-file status
(pending / processing / done / error). Single-video selection and image
handoff keep their original behavior.

Sequential processing is intentional: video decode/encode is CPU/GPU
bound, so reusing the proven single-file pipeline one item at a time is
correct rather than a compromise.
@GargantuaX

Copy link
Copy Markdown
Owner

Thanks for the contribution. I tested this commit after merging the current main (v1.0.31) into an isolated checkout.

Validation completed:

  • merge with current main: clean
  • production build: passed
  • full suite: 1,356 passed, 29 skipped, 0 failed
  • browser-level batch test: an invalid video followed by a real video produced error + done, continued after the failure, and emitted the expected download

I found one reproducible queue-state issue. After the first two-file batch completed, selecting a second two-file batch appended the new items to the completed queue. The UI contained four rows and reported 成功 1/4; the second batch itself contained two invalid files, so its correct summary was 成功 0/2. A later single-file selection also leaves the old batch queue visible.

Please reset the completed queue when a new selection starts while batch.processing is false (and hide/clear stale batch UI when returning to single-file mode). Appending remains reasonable only when a batch is actively processing and that behavior is intentional.

The PR currently adds no tests. Please add focused coverage for a second batch after completion, returning to single-file mode, failure-then-continue behavior, and automatic download/resource cleanup.

A finished queue was never cleared, so the next selection appended to it:
the UI kept the old rows and the summary counted them, reporting
成功 1/4 for a second two-file batch instead of 成功 0/2. Returning to
single-file mode also left the stale queue visible, because the clearing
path only ran from the reset button and never from a file selection.

Queue rules now live in src/video/videoBatchQueue.js, free of DOM access
so they can be unit-tested. A new selection drops an idle queue, while
appending is preserved while a batch is actively processing. The
single-file path re-renders the emptied queue to hide stale batch UI.

Adds tests/video/videoBatchQueue.test.js covering a second batch after
completion, returning to single-file mode, failure-then-continue,
automatic download emission, and queue release on reselection.
@kavyp12

kavyp12 commented Jul 20, 2026

Copy link
Copy Markdown
Author

Thanks for the detailed validation — the queue-state issue reproduced exactly as you described, and both symptoms had the same root cause.

Root cause: the queue was cleared in reset(), but reset() is only bound to the reset button — it never ran on a file selection. So a finished queue survived into the next selection: new items were appended to the completed rows, and the summary counted the whole queue (成功 1/4 instead of 成功 0/2). The same gap left the stale queue visible when returning to single-file mode.

Fix:

  • A new selection now drops an idle queue before anything else, so completed rows never leak into the next one — batch or single-file.
  • Appending is preserved while processing is true, since that behaviour is intentional.
  • The single-file path re-renders the emptied queue, which hides the stale batch UI.

Testability: the queue rules moved into src/video/videoBatchQueue.js, free of DOM access, following the existing src/video/* + tests/video/* convention. video-app.js keeps only the DOM wiring and injects processFile / downloadResult / onChange / onError.

Tests (tests/video/videoBatchQueue.test.js) cover the four areas you asked for, plus three guards:

  • a second batch after completion replaces the finished queue (asserts 0/2, not 1/4)
  • returning to single-file mode clears the queue
  • failure-then-continue: a throwing item is marked error and the run continues
  • only successful items emit an automatic download; failed and skipped items emit none
  • a completed queue releases its file references on the next selection
  • files selected while a batch is running still append to the active run
  • processBatchQueue refuses to start a second concurrent run

I checked the tests actually cover the regression rather than just passing: reverting the one-line reset makes 3 of them fail, and restoring it turns them green again.

Verification: production build passes. Full suite here is 1358 passed / 29 skipped / 6 failed — the 6 are the extension-packaging and pnpm pack SDK tests, which fail identically on a clean checkout of main without these changes (no pnpm in my environment), so nothing here is caused by this PR.

Happy to adjust naming or split the module differently if you'd prefer it shaped another way.

@GargantuaX GargantuaX left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Thanks for the follow-up. I revalidated 081bcd0 after applying it cleanly to the current main (1368064), rather than relying on GitHub's stale synthetic merge ref.

The main path is in good shape:

  • the 7 focused queue tests pass;
  • the full current suite passes: 1662 passed, 33 skipped, 0 failed;
  • the production build passes;
  • a real Chromium run with two repository videos completed both queue items and emitted two download events with the expected filenames.

There is one blocking UI-wiring issue when a selection arrives during an active batch. startBatchSelection(batch) correctly preserves an active queue, but handleIncomingFiles() only enqueues when videoFiles.length > 1. Selecting or dropping exactly one video while the batch is running therefore falls through to the single-file path and calls setFile(file) against the shared page state.

Browser reproduction:

  1. Select 119-original.mp4 and 114-original.mp4 together.
  2. Wait until the first queue row is processing and the second is pending.
  3. Select one additional video, 20260615-current-export.mp4.
  4. The queue still shows the first file processing and the second pending, but the main player/metadata/status switch to the new 1280×720 single video (视频已载入 / 准备就绪). The new file is not present in the queue.

This can invalidate/cancel the active job through the shared state.file / jobId while the queue still reports the original item as processing. While batch.processing is true, any incoming video selection (including exactly one video) should either append to the active queue or be explicitly rejected; it must not call the single-file setFile() path.

Please add coverage at the handleIncomingFiles/UI-routing boundary for a one-video selection during an active batch. The current queue-module append test calls startBatchSelection() and enqueueBatchFiles() directly, so it does not exercise the failing integration branch.

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