Skip to content

fix: use VideoGenerator for video thumbnail generation - #7

Open
shaolun990905 wants to merge 2 commits into
tendant:mainfrom
shaolun990905:fix/video-thumbnail-generation
Open

fix: use VideoGenerator for video thumbnail generation#7
shaolun990905 wants to merge 2 commits into
tendant:mainfrom
shaolun990905:fix/video-thumbnail-generation

Conversation

@shaolun990905

Copy link
Copy Markdown

Problem

After uploading a video to a portfolio, the thumbnail generation service
(simple-thumbnailer) failed silently. Investigation revealed two root causes:

Root Cause 1: Wrong generator used for video files

handleJob in cmd/thumbnail-worker/main.go directly called
img.GenerateThumbnails(), which internally uses imaging.Open() — an
image-only library that cannot handle video files. This caused the error:
open: image: unknown format

The codebase already had a proper img.GetGenerator(mimeType) function that
routes to VideoGenerator (FFmpeg-based frame extraction) for video/* MIME
types, but it was never called from the thumbnail worker entrypoint.

Root Cause 2: Thumbnails stored with wrong MIME type

Even when thumbnails were successfully generated, UploadThumbnailObject was
called with MimeType: source.MimeType (e.g. video/mp4), which caused the
generated JPEG thumbnail to be stored in the database as a video file. The
preview API queries derived content with status = processed and expects
image MIME types for thumbnails, so it returned 404.

Root Cause 3: Empty MIME type fallback

FetchSource calls GetContentMetadata to determine the file's MIME type.
If the metadata service returns an empty string (e.g. due to timing or
service issues), no generator could be selected and the worker fell back to
ImageGenerator, causing the same failure as Root Cause 1.

Changes

  • cmd/thumbnail-worker/main.go: Replace direct call to
    img.GenerateThumbnails() with img.GetGenerator(source.MimeType) to
    correctly dispatch to VideoGenerator for video files and ImageGenerator
    for images
  • cmd/thumbnail-worker/main.go: Fix UploadThumbnailObject call to use
    MimeType: "image/jpeg" and FileName: filepath.Base(thumb.Path) instead
    of inheriting the source video's metadata
  • internal/upload/client.go: Add mimeTypeFromFilename() helper and
    apply it as a fallback when GetContentMetadata returns an empty MIME type,
    inferring type from file extension (.mp4video/mp4, etc.)

Testing

Verified end-to-end: video upload → NATS job → FFmpeg frame extraction →
JPEG thumbnail uploaded with correct MIME type → preview API returns 200
with presigned S3 URL.

问题背景

视频上传到作品集后,缩略图生成服务(simple-thumbnailer)无法正常处理视频文件,
导致前端预览图一直返回 404 错误。经排查发现以下三个根本原因:

根本原因一:视频文件使用了错误的生成器

cmd/thumbnail-worker/main.go 中的 handleJob 直接调用了
img.GenerateThumbnails(),该函数内部使用 imaging.Open()
这是一个仅支持图片格式的库,无法处理视频文件,导致报错:
open: image: unknown format

代码库中已有 img.GetGenerator(mimeType) 函数,可根据 MIME type 路由到
VideoGenerator(基于 FFmpeg 提取视频帧),但 thumbnail worker 入口从未调用它。

根本原因二:缩略图以错误的 MIME type 存储

即使缩略图文件成功生成,UploadThumbnailObject 调用时传入的是
MimeType: source.MimeType(如 video/mp4),导致生成的 JPEG 缩略图在
数据库中被标记为视频文件。Preview API 查询时要求衍生内容状态为 processed
且类型为图片,因此无法匹配,返回 404。

根本原因三:MIME type 为空时缺少兜底处理

FetchSource 通过 GetContentMetadata 获取文件 MIME type。若接口返回空字符串
(如服务时序问题),则无法选择正确的生成器,最终 fallback 到 ImageGenerator
与根本原因一产生相同的失败。

修改内容

  • cmd/thumbnail-worker/main.go:将直接调用 img.GenerateThumbnails()
    改为通过 img.GetGenerator(source.MimeType) 动态选择生成器,视频文件路由到
    VideoGenerator,图片文件路由到 ImageGenerator
  • cmd/thumbnail-worker/main.go:修复 UploadThumbnailObject 调用,
    将 MIME type 改为 image/jpeg,文件名改为缩略图文件自身的名称,
    不再继承原始视频的元数据
  • internal/upload/client.go:新增 mimeTypeFromFilename() 辅助函数,
    GetContentMetadata 返回空 MIME type 时,通过文件扩展名推断类型
    (如 .mp4video/mp4

验证

端到端验证通过:视频上传 → NATS 消息触发任务 → FFmpeg 提取视频帧 →
JPEG 缩略图以正确 MIME type 上传 → Preview API 返回 200 及 S3 预签名 URL。

- Route to VideoGenerator via GetGenerator() based on MIME type instead
  of calling GenerateThumbnails() directly which only supports images
- Fix thumbnail upload MIME type to image/jpeg instead of source video MIME type
- Add mimeTypeFromFilename() fallback when GetContentMetadata returns empty MIME type

@GGGLHHH GGGLHHH left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for the fix on generator routing. I found two issues that should be addressed before merge:

  1. Incorrect thumbnail MIME type is hardcoded
  • In uploadResultsStep, MimeType is always set to image/jpeg.
  • But output format is not always JPEG:
    • image flow keeps source extension (thumb.go)
    • PDF flow outputs .png (pdf_generator.go)
  • This can store wrong Content-Type metadata for uploaded objects.
  • Suggestion: derive MIME from thumb.Path (or pass empty and let existing detectMime handle it).
  1. Unsupported MIME is swallowed by fallback to image generator
  • GetGenerator errors are always ignored and fallback goes to ImageGenerator.
  • For truly unsupported types (e.g. text/plain), this turns a clear permanent failure into an image decode failure, which may be classified as retryable.
  • Suggestion: only fallback to image generator when source MIME is empty; otherwise return the unsupported MIME error directly.

Tests pass (go test ./..., go test -tags nats ./...), but these two behavior regressions should be fixed.

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