fix: un-stick federated video posters — emit asset-arrived for the regenerated thumbnail and name it from the history row - #4306
Merged
Conversation
…d name it from the history row (#4162) doPullOneAsset regenerated a pulled video's thumbnail but emitted `asset-arrived` only for the .mp4, so MediaImage's filename-keyed listener never fired for the poster and a federated Creative Director card sat on the "Syncing" placeholder until a remount. The name was also derived from the mp4 basename. That is only coincidentally right for a videoGen clip (`<jobId>.mp4` beside `thumbnail: '<jobId>.jpg'`) and flatly wrong for a stitched timeline final, which videoTimeline/local.js writes as `timeline-<slice>-<ts>.mp4` beside an independent randomUUID history id — so the regenerated file landed under a name no poster URL ever requests. The name now comes from the synced history row's `thumbnail` field (sanitized, since it rode the wire from a peer), falling back to the mp4 stem for the window where the bytes beat the videoHistory metadata category across.
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.
Summary
A federated video poster never un-stuck from
MediaImage's "Syncing" placeholder. Two independent defects indoPullOneAsset(server/services/sharing/peerSyncAssets.js), both fixed here:asset-arrivedonce — for the.mp4— then regenerated the video's thumbnail with no matching emit.MediaImagematches arrivals on filename alone, so a poster<img>that had already 404'd stayed on the placeholder until a remount even though the bytes were on disk. The regenerated thumbnail now gets its ownasset-arrived(kind: 'video-thumbnail').<jobId>.mp4besidethumbnail: '<jobId>.jpg');server/services/videoTimeline/local.jswrites a stitched timeline final astimeline-<projectId-slice>-<ts>.mp4beside an independentrandomUUID()history id, and every poster URL the UI builds is/data/video-thumbnails/<row.id>.jpg. So the regenerated file landed under a name nothing ever requests. The name now comes from the synced history row'sthumbnailfield, resolved by video filename via a newvideoThumbnailNameForVideo()reader.Details worth noting:
thumbnailgoes throughsanitizeAssetFilenamebefore it can become a path segment; a traversal-shaped value falls back to the stem rather than escapingdata/video-thumbnails/.videoHistorymetadata category across — that is the pre-existing behavior, so this is strictly additive and can't regress the clip case.video-history.jsonreaders in this module now share onereadVideoHistoryRows()helper.Closes #4162
Test plan
server/services/sharing/peerSyncAssets.videoThumbnail.test.js(5 cases): timeline-final naming from the history row, videoGen-clip arrival emit, stem fallback when the row has not synced, traversal-shapedthumbnailrejected, and no thumbnail arrival when regeneration returns null (ffmpeg missing). Verified the 3 behavior cases fail against the pre-fix source and pass after.cd server && NODE_ENV=test npx vitest run services/sharing/— 22 files, 579 tests pass.cd server && NODE_ENV=test npm test— 29489 pass. The only failures are 2 pre-existing, environment-dependent cases inroutes/health.test.js, confirmed failing identically on a cleanmaincheckout.