feat: 加速缩略图生成时间 #1818
feat: 加速缩略图生成时间 #1818
Conversation
pkg/filesystem/image.go
Outdated
| } | ||
| } | ||
|
|
||
| if src == "" && file.Size > uint64(model.GetIntSetting("thumb_max_src_size", 31457280)) { |
There was a problem hiding this comment.
This will break max_src_size check.
There was a problem hiding this comment.
('s on purpose as mentioned above)
| @@ -117,11 +117,6 @@ func (fs *FileSystem) generateThumbnail(ctx context.Context, file *model.File) e | |||
| defer cancel() | |||
| // TODO: check file size | |||
|
|
|||
| if file.Size > uint64(model.GetIntSetting("thumb_max_src_size", 31457280)) { | |||
There was a problem hiding this comment.
unhappy path first - better to check max size before obtain workers.
There was a problem hiding this comment.
TLDR: I moved this code to below that local download checking, generating thumbnail shouldn't have a max size limit on.
So you want max size check to be the first priority?
Because by using url method size can be ignored and unlimited.
By letting size check for local (the code below) this could safely confirmes that we're not downloading huge files at local.
If this line puts in first this could break how this generate by url method goes.
For example,
if I increase the max size to 10GB, (e.g. Some kind of movie)
when I failed to get the source file, the logic will fallbacks to local download mode
that's the place that the max size is worth for checking to avoid downloading huge files to local vm (As mentioned above)
In another way, if you check the max size first, this could cause lots of file that over the max size can't generate the thumbnail and forced ppl to increase in settings.
If they don't have big enough hard disk in local, this could cause OOM / full hard disk problem.
Update: I add that only checks for url, if not then it should be local file / file that will fetch to local.
For more, see pipeline.go
pkg/filesystem/image.go
Outdated
| } else { | ||
| ffmpegFormats := strings.Split(model.GetSettingByName("thumb_ffmpeg_exts"), ",") | ||
| vipsFormats := strings.Split(model.GetSettingByName("thumb_vips_exts"), ",") | ||
| // 如果文件格式支持 ffmpeg 或 vips 生成缩略图，则使用源文件链接 |
There was a problem hiding this comment.
Why include VIPS? src is not even used in VIPS generator.
There was a problem hiding this comment.
Thanks for letting me know, didn't notice that when writiing this.
As documented of vips, url can also be used to generate thumbnail
Faked by ChatGPT, fuck
| @@ -136,6 +131,24 @@ func (fs *FileSystem) generateThumbnail(ctx context.Context, file *model.File) e | |||
| src := "" | |||
| if conf.SystemConfig.Mode == "slave" || file.GetPolicy().Type == "local" { | |||
| src = file.SourceName | |||
| } else { | |||
There was a problem hiding this comment.
This does not seems to be a good abstraction:
-
Generator specific logic should keeps in
thumbpackage. Thisimage.goshould be generator-agnostic. -
srcparameter in thumb generator is originally for file path. I understand in ffmpeg it happens can also be file URL, but it will make the interface definition inconsistent.
There was a problem hiding this comment.
ok, I'll move it into thumb package, but this also means it needs sort of changing function sign too.
Fix: #1817
大概
3-5s就能生成了顺便修复下给文件大小为0的文件生成缩图导致上传后不生成