Files
Motionity/TODO-FIXES.md
T
kawaandClaude Opus 5 9a4d14613f feat: replace the archive.org asm.js encoder with vendored ffmpeg.wasm
MP4/GIF export used to importScripts() an 18.5 MB asm.js ffmpeg build from
https://archive.org/download/ffmpeg_asm/ffmpeg_asm.js: no integrity check, no
pinning, executed in the page, and unavailable offline. vendor.mjs now copies
ffmpeg.wasm out of node_modules, where package-lock.json pins it by hash, and
no CDN fallback is left anywhere in the app.

@ffmpeg/core-st is the single-threaded core, chosen deliberately: the default
@ffmpeg/core is built with pthreads and needs SharedArrayBuffer, which requires
COOP/COEP isolation, which would break the Pixabay, Unsplash and Google Fonts
requests. That core also forces two things worth knowing:

- mainName: 'main' is mandatory. The loader defaults to proxy_main, which only
  the multi-threaded build exports, so load() compiles all 23 MB and then aborts.
- Its main() calls exit(), so an instance survives exactly one command. Reusing
  one dies with "Program terminated with exit(0)", so convertStreams builds and
  tears one down per conversion (~110 ms, and the 23 MB heap comes back in
  between). The teardown also runs on failure: an interrupted run otherwise
  leaves the loader's "running" flag set and wedges every later conversion until
  a page reload.

MP4 encodes with libx264 -crf 23 -pix_fmt yuv420p plus AAC rather than
mpeg4 -b:v 6400k. Same core, better quality per byte, and yuv420p is what makes
it play in Safari and QuickTime.

The two @ffmpeg packages are dependencies, not devDependencies, so the Docker
vendor stage can npm ci --omit=dev without pulling in electron; build.files
excludes them from the asar since src/vendor/ffmpeg/ already carries the copies
the app loads. WITH_FFMPEG=0 now means MP4/GIF export is unavailable and says
so, rather than silently fetching an encoder at run time.

Also deletes src/js/libraries/ffmpeg.min.js, an unreferenced ffmpeg.wasm loader
stub that would have fetched its core from unpkg, and prunes the stale
src/vendor/ffmpeg_asm.js from existing checkouts — src/vendor/ is packaged
whole, so it would have shipped 18.5 MB of dead weight in every installer.

Verified in Chromium against a real MediaRecorder WebM: core loads with
crossOriginIsolated false, MP4 24 KB decoding to 320x240 / 2.00 s, GIF 138 KB,
the two back to back, and the missing-core path reporting correctly.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-12 15:41:40 +02:00

18 KiB
Raw Permalink Blame History

Bugs & Issues — audit + fixes

Audit of src/js, then all findings fixed. Every file below passes node --check. No live browser smoke test was run (a Chrome instance held the Playwright profile lock), so the changes are verified statically only — see Not verified at the end.

Legend: [x] fixed · P0 = feature broken · P1 = wrong data written · P2 = crash on edge path · P3 = silently wrong UI · P4 = perf/leak · P5 = cleanup


P0 — Broken features

  • MP4 export dead-ended and locked the UIsrc/js/functions.js The mp4 branch of downloadRecording was type = 'video/mp4', an implicit global assignment that did nothing: no download, recording stuck at true, the button stuck on "Downloading...", and downloadModal() refusing to reopen until reload. mp4 now goes through convertStreams(blob, 'mp4') like gif, and a new shared resetRecordingUI() unlocks the editor on every exit path (success, unknown format, worker error, FileReader error, MediaRecorder error).

  • Copying keyframes threwsrc/js/events.js canvas.getActiveObject().isEditing ran on null, because selecting keyframes clears the canvas selection — the normal path. The active object is now resolved once and guarded, and empty $.grep results are no longer pushed to the clipboard.

  • canvas.getItemByid typosrc/js/functions.js Lowercase i; TypeError when pasting text/charSpacing keyframes.

  • Ctrl+Z crashed on an empty stack; Ctrl+Shift+Z was a no-opsrc/js/events.js Merged into one guarded handler: shift picks redo, and each branch checks its own stack length. Previously Ctrl+Shift+Z matched both if blocks (redo then undo).

  • Blur slider threw with nothing selectedsrc/js/events.js obj.applyFilters() was outside the if (canvas.getActiveObject()) guard. Also renamed the shadowed x in the blur/noise/chroma find() callbacks.

P1 — Wrong values written

  • height keyframes stored the widthsrc/js/functions.js (3 sites: twice in keyframeChanges, once in crop)
  • var scaleX = obj, scaleX;src/js/text.js; the fabric object was being assigned as a scale factor.
  • NaN letter delay + shadowed global durationsrc/js/text.js delay = i * duration read the hoisted local before its own initialiser. Renamed to step, computed before use, and !(delay > 0) now catches NaN.
  • Every letter animation wrote to the last lettersrc/js/text.js index, animation, start and instance are let per iteration instead of var.
  • Shadow defaults/keyframes stored undefinedsrc/js/functions.js fabric's get() is not a path getter. Added getPropValue() (handles shadow.*) and setDefaultValue() / getDefaultValue(), and routed ~26 direct .defaults.find(...).value = … writes through them — which also creates the entry when a project predates a property instead of throwing.
  • WebGL filter backend clobbered with undefinedsrc/js/init.js, src/js/database.js Both assignments are now conditional on the backend having constructed.

P2 — Crashes on edge paths

  • RangeError: Invalid array lengthsrc/js/functions.js temparr.length = findIndex(...) went negative for a stale keyframe reference. lastKeyframe / nextKeyframe are now index lookups that return false.
  • animate() had no null guardssrc/js/functions.js Object and p_keyframes lookups are resolved once and guarded in every per-frame loop (animate, recordAnimate, the playback update loop, playVideos, playAudio), which also removed dozens of repeated .find() calls.
  • getAssets() recursed synchronously foreversrc/js/database.js Retries on a 250 ms timer, capped at 20 attempts, and rebuilds the asset arrays instead of appending duplicates on re-entry after an import.
  • deleteObject threw and leaked filessrc/js/functions.js The entry was compared to a string so it never matched; now filtered by name. Video elements are also paused and unloaded on delete.
  • Unguarded keyarr[0] / .defaults.find(...)copyKeyframes, updateKeyframe, applyEasing, keyframeProperties, removeKeyframe, checkAnyKeyframe. The four repetitive counterpart blocks were replaced by one KEYFRAME_COUNTERPARTS map, so a missing counterpart is skipped, not fatal.
  • Other unguarded lookupsdeleteAsset, reGroup, renderLayer, renderProp, setDuration, setTimelineZoom, saveLayerName, updateInputs, updatePanel, updateStrokeValues, animateText, scrollIntoView (3 sites), object:modified / object:rotating / mouse:out / mouse:up handlers. importProject validates the payload before touching data.project[0], and line_h/line_v go through a new hideGuides() helper.

P3 — Silently wrong behaviour

  • document.onmousedown permanently disabledsrc/js/functions.js dragTimeline installed a return false handler and never removed it. It now only overrides onselectstart, and restores it on mouseup.
  • Keyframe time drifted on every dragsrc/js/functions.js data-time is now always the absolute timeline time (the value every lookup keys off), with the visual offset applied as CSS only. This also fixes keyframes on an expanded row, whose data-time used to be layer-relative so no lookup matched. updateKeyframe's unused third argument is gone.
  • Shift-deselect never removed a keyframethis inside a $.grep callback is not the element.
  • e.shiftDowne.shiftKey.
  • Snap guide never hid — the row-local index was compared against the global .keyframe count.
  • Comparison / typo bugs canvas.getActiveObjects.lengthgetActiveObjects().length · strokeDashArray == [10, 5] → element comparison · chroma setValue(distance)distance * 100 · '#FFFFF''#FFFFFF' · videoPlayer.videoheightvideoHeight (and both thumbnail helpers no longer draw the canvas onto itself) · if (start && play && !paused) in playAudio (play is the global function, always truthy) · #redo gated on redo.length · :last-child():last-child.
  • Paste loop closurevar imgObjlet, so each thumbnail saves its own file.
  • Layers added mid-timeline were shortenedend: duration - currenttimeduration (media layers clamp to min(start + assetDuration, duration)).
  • Malformed HTML — 5 unterminated <img …' strings and a stray </div> in src/js/ui.js.
  • Duplicate DOM idsid="easing" on both wrapper and <select> (the select is now easing-select; #easing select still matches), id="filters-title" ×4 and id='item-text' ×6 became classes, with src/styles.css updated to match. Added the missing <meta charset>.

P4 — Performance, leaks, export gaps

  • save() rebuilt the record canvas on every editsrc/js/functions.js updateRecordCanvas() + autoSave() now run through a 400 ms debounce (schedulePersist). record() still awaits updateRecordCanvas() directly, so an export always captures current state. updateObjectValues no longer calls autoSave() on every keystroke.
  • async forEach raced the snapshotsrc/js/functions.js, src/js/database.js Both filter-stripping loops are sequential for…of in async functions, so toJSON / toDatalessJSON can no longer run mid-strip.
  • O(n²·log n) playbacksrc/js/functions.js buildKeyframeIndex() groups keyframes by id|name once per rendered frame; lastKeyframe, nextKeyframe and checkAnyKeyframe use it instead of sorting a copy of the entire keyframe list per keyframe per frame. The two duplicated inner nextKeyframe copies are gone.
  • Exports lost audio-layer soundsrc/js/recorder.js Audio layers were never routed into the capture stream. All sources (video elements, audio layers, background audio) now mix through one AudioContext into one destination track — necessary because MediaRecorder only records the first audio track. Audio layers start/stop on their p_keyframes boundaries.
  • Export timingsrc/js/recorder.js The render clock starts after recorder.start() (it used to run before, losing the first frames), stops when the animation clock covers duration rather than on a wall-clock setTimeout, and has a duration + 5s safety stop.
  • Leaks — AudioContexts are closed and stream tracks stopped on export end; object URLs are revoked (downloadRecording, PostBlob, exportProject, importProject); the browser scroll handler is unbound before rebinding; sortable is initialised once instead of per rendered layer; deleted videos are paused and unloaded; category/audio grids are cleared before repopulating.

P5 — Dead code, config, cleanup

  • Unreachable code calling an undefined waitForEventsrc/js/recorder.js initRecorder / recordFrame / exportRecording and ~180 lines of commented-out abandoned experiments were removed; the file is now just the live export path.
  • Placeholder API keyssrc/js/init.js HAS_FONTS_KEY / HAS_PIXABAY_KEY gate the requests. Without a fonts key the pickers fall back to the bundled families instead of staying empty; without a Pixabay key search shows an explanatory message instead of 401-ing.
  • var timeout collided with recorder.js's timeout() — removed (it was unused).
  • Implicit globalssrcfreeze, osc, type, newcolorkeyframe (deleted, never read).
  • Duplicate / unreachable branches — the second shadow.blur arm in all three setValue copies, the duplicated objectCaching key, the three copies of the keyframe sort comparator (now sortKeyframes()), and the two identical crop blocks in events.js (now updateCropBounds()).
  • Converter was unfinishedsrc/js/converter.js The workerReady handshake never fired (buffersReady was never set); worker readiness is now tracked across calls, since the worker is created once and reused. Added onerror handling, an empty-result guard, and a user-visible failure path.
  • Miscoverlay() no longer reuses the artboard's overlay id · getItemById recurses so nested groups are found and stops at the first hit · fabric.Lottie guards this.canvas before the object is added · newLottieAnimation no longer multiplies an ms duration by 1000 · lottie layers get a colour · calculateTextWidth honours the requested font · changeFont / loadImage / loadVideo / handleLottieUpload have failure paths · checkFilter no longer tests for a non-existent video fabric type · AnimatedText.assignTo applies text/props · animate(currenttime, false)animate(false, currenttime) (3 sites — text animation changes never refreshed) · exportProject uses a Blob instead of a data: URL · clearProject reloads after the deletes resolve · readTextFile handles blob-URL status 0 and errors · .catch added to the Localbase promises · opacity inputs clamp the input, not the wrapper · arrow-key nudge calls setCoords() and is suppressed while typing · layer reorder shortcuts accept Ctrl as well as Cmd · alignment guides ignore hidden objects · alignObject saves and dropped its console.log.


Frame-accurate export (new)

The real-time capture path was the root cause of "heavy scenes drop frames" and "video layers are sampled at the wrong moment". It is now the fallback, not the default.

src/js/render.js — offline renderer

No clock is attached to the render. For each of duration × 30 frames:

  1. every video layer is seeked to the exact frame time and the code awaits seeked before drawing (a bare currentTime = is async — drawing straight after captures the previous frame, which is why real-time exports smeared video layers);
  2. lottie layers are advanced to the same time;
  3. recordAnimate(time) lays out the frame, canvasrecord.renderAll() draws it;
  4. a VideoFrame is built from the canvas with an explicit timestamp and pushed through a VideoEncoder (VP9, falling back to VP8), with the encoder queue held at ≤ 8 frames so memory stays bounded.

Audio is mixed in one OfflineAudioContext pass — every audio layer, every video layer's soundtrack and the background track, each placed at its own start/trimstart/end with its own gain — then encoded to Opus with AudioEncoder. Sample-accurate, and unlike the live path it does not depend on playback keeping up.

Both tracks are sorted by timestamp and muxed into one WebM.

recordAnimate() skips playVideos() while offlinerender is set, so real-time playback cannot fight the explicit seeking.

src/js/webm-writer2.js — extended

Was single-track video only, and unreferenced by index.html. Now loaded, and:

  • optional second Opus track (A_OPUS + 19-byte OpusHead CodecPrivate, CodecDelay, SeekPreRoll, Audio element with sample rate/channels);
  • addFrame(chunk, trackNumber) accepts EncodedAudioChunk as well as EncodedVideoChunk; addFrameToCluster no longer hardcodes track 1.

Three bugs fixed in the vendored library along the way:

  • MAX_CLUSTER_DURATION_MSEC was 5000000 (~58 days), so every frame went into a single cluster and block timecodes — a signed 16-bit offset — overflowed past ~32 s. Now 5000 ms, and clusters close on a video keyframe.
  • The header buffer was a fixed 256 bytes, which a second TrackEntry overflows (ArrayBufferDataStream's pos lies beyond end of buffer). Now 1024.
  • instanceof Uint8Array for byte payloads fails across a realm boundary; ArrayBuffer.isView is now accepted too.

Safety net

The muxer is hand-rolled, so renderFrameAccurate() loads the finished blob into a <video> element and checks it decodes before handing it over. Any failure — no WebCodecs, unsupported codec, encoder error, failed verification — returns null and record() transparently falls back to the real-time encoder.

Tests — test/webm-muxer.test.js

Runs in plain Node (no deps): node test/webm-muxer.test.js

Loads the muxer in a VM context, feeds fake encoded chunks, then parses the resulting bytes with a small EBML reader and asserts the EBML header, Segment, one or two TrackEntry with the right CodecID/TrackType, a 19-byte OpusHead, multiple clusters, every block present on the right track, block timecodes inside signed 16-bit range, ascending cluster timecodes, and that cluster timecode + block timecode reconstructs the input timestamps to within 1 ms. Covers video-only and video+audio at 75 s (13 clusters, 3000 blocks).

Both cases pass. The 256-byte header bug above was caught by this test, not by inspection.


Left in place deliberately

  • src/js/encode-worker.js is still unreferenced. It was the old File-System-Access-API sketch; render.js supersedes it and buffers to memory instead of requiring a save-file picker. Deleting it is a product decision.
  • MP4 and GIF go through ffmpeg.wasm (converter.js), vendored out of node_modules and pinned by package-lock.json. This replaced the asm.js worker loaded from archive.org, which had no integrity check and needed the network. See "ffmpeg.wasm migration" below.

ffmpeg.wasm migration (new)

MP4/GIF export used to importScripts() an 18.5 MB asm.js ffmpeg build from https://archive.org/download/ffmpeg_asm/ffmpeg_asm.js — no integrity check, no pinning, and executed in the page. scripts/vendor.mjs now copies ffmpeg.wasm out of node_modules, where package-lock.json pins it by hash. There is no CDN fallback left anywhere in the app.

Three things this ran into, all of which cost a debugging round:

  • @ffmpeg/core needs SharedArrayBuffer. The default core is built with pthreads, which requires COOP/COEP cross-origin isolation, which would break the Pixabay, Unsplash and Google Fonts requests. @ffmpeg/core-st — the single-threaded build — is used instead. Verified: the core loads with crossOriginIsolated === false and SharedArrayBuffer undefined.
  • mainName: 'main' is mandatory with that core. The loader defaults to proxy_main, which only the multi-threaded build exports. Without it, load() compiles all 23 MB and then aborts with Cannot call unknown function proxy_main.
  • One conversion per load. The single-threaded core's main calls exit(), so a second run() on the same instance dies with Program terminated with exit(0). convertStreams therefore builds and tears down an instance per conversion — measured at ~110 ms, and it returns the 23 MB heap in between. The teardown also runs on failure: an interrupted run otherwise leaves the loader's internal "running" flag set and every later conversion fails with can only run one command at a time until the page is reloaded.

MP4 now encodes with libx264 -crf 23 -pix_fmt yuv420p plus AAC audio rather than mpeg4 -b:v 6400k. Same core, better quality per byte, and yuv420p is what makes it play in Safari and QuickTime.

WITH_FFMPEG=0 no longer means "download it at run time" — it means MP4/GIF export is unavailable, and converter.js says so instead of failing obscurely.

Verified in Chromium against a real MediaRecorder WebM: MP4 24 KB with an ftypisom header that decodes to 320x240 / 2.00 s, GIF 138 KB with a GIF89a header, the two run back to back, and the missing-core path produces the right message.


Not verified

node --check passes on every script; the muxer is covered by the Node test above. The app itself was not loaded in a browser (a running Chrome instance held the Playwright profile lock), so the following still needs a manual pass:

  1. cd src && python -m http.server 8765, open http://localhost:8765
  2. Add a shape, keyframe it, drag the keyframe, scrub, undo/redo
  3. Export as WEBM with a video layer and an audio layer present — confirm the console shows Rendering n% (frame-accurate path) and not Falling back to the real-time encoder
  4. Play the result: check audio is in sync and the video layer is not smeared