From 6ceb9c73dd0dd89dbacbddbc7a10083691e3a9b8 Mon Sep 17 00:00:00 2001 From: WithoutPants <53250216+WithoutPants@users.noreply.github.com> Date: Sun, 20 Mar 2022 17:48:52 +1100 Subject: [PATCH] Don't generate thumbnails for webp (#2388) * Don't generate thumbnails for animated webp * Debug log when writing thumbnail to disk --- internal/api/routes_image.go | 7 +- internal/manager/task_scan_image.go | 6 +- pkg/ffmpeg/image.go | 12 +--- pkg/image/thumbnail.go | 26 ++++++-- pkg/image/webp.go | 46 +++++++++++++ pkg/image/webp_internal_test.go | 66 +++++++++++++++++++ .../components/Changelog/versions/v0140.md | 1 + 7 files changed, 148 insertions(+), 16 deletions(-) create mode 100644 pkg/image/webp.go create mode 100644 pkg/image/webp_internal_test.go diff --git a/internal/api/routes_image.go b/internal/api/routes_image.go index f27590d4d..49dd2a056 100644 --- a/internal/api/routes_image.go +++ b/internal/api/routes_image.go @@ -2,6 +2,7 @@ package api import ( "context" + "errors" "net/http" "strconv" @@ -46,7 +47,10 @@ func (rs imageRoutes) Thumbnail(w http.ResponseWriter, r *http.Request) { encoder := image.NewThumbnailEncoder(manager.GetInstance().FFMPEG) data, err := encoder.GetThumbnail(img, models.DefaultGthumbWidth) if err != nil { - logger.Errorf("error generating thumbnail for image: %s", err.Error()) + // don't log for unsupported image format + if !errors.Is(err, image.ErrNotSupportedForThumbnail) { + logger.Errorf("error generating thumbnail for image: %s", err.Error()) + } // backwards compatibility - fallback to original image instead rs.Image(w, r) @@ -55,6 +59,7 @@ func (rs imageRoutes) Thumbnail(w http.ResponseWriter, r *http.Request) { // write the generated thumbnail to disk if enabled if manager.GetInstance().Config.IsWriteImageThumbnails() { + logger.Debugf("writing thumbnail to disk: %s", img.Path) if err := fsutil.WriteFile(filepath, data); err != nil { logger.Errorf("error writing thumbnail for image %s: %s", img.Path, err) } diff --git a/internal/manager/task_scan_image.go b/internal/manager/task_scan_image.go index ab35d5e92..9994c81c2 100644 --- a/internal/manager/task_scan_image.go +++ b/internal/manager/task_scan_image.go @@ -3,6 +3,7 @@ package manager import ( "context" "database/sql" + "errors" "path/filepath" "time" @@ -155,7 +156,10 @@ func (t *ScanTask) generateThumbnail(i *models.Image) { data, err := encoder.GetThumbnail(i, models.DefaultGthumbWidth) if err != nil { - logger.Errorf("error getting thumbnail for image %s: %s", i.Path, err.Error()) + // don't log for animated images + if !errors.Is(err, image.ErrNotSupportedForThumbnail) { + logger.Errorf("error getting thumbnail for image %s: %s", i.Path, err.Error()) + } return } diff --git a/pkg/ffmpeg/image.go b/pkg/ffmpeg/image.go index 68fb90379..86f199c44 100644 --- a/pkg/ffmpeg/image.go +++ b/pkg/ffmpeg/image.go @@ -2,20 +2,14 @@ package ffmpeg import ( "bytes" - "errors" "fmt" ) -var ErrUnsupportedFormat = errors.New("unsupported image format") - -func (e *Encoder) ImageThumbnail(image *bytes.Buffer, format *string, maxDimensions int, path string) ([]byte, error) { +func (e *Encoder) ImageThumbnail(image *bytes.Buffer, format string, maxDimensions int, path string) ([]byte, error) { // ffmpeg spends a long sniffing image format when data is piped through stdio, so we pass the format explicitly instead - ffmpegformat := "" - if format == nil { - return nil, ErrUnsupportedFormat - } + var ffmpegformat string - switch *format { + switch format { case "jpeg": ffmpegformat = "mjpeg" case "png": diff --git a/pkg/image/thumbnail.go b/pkg/image/thumbnail.go index ea03d7357..c364b04c0 100644 --- a/pkg/image/thumbnail.go +++ b/pkg/image/thumbnail.go @@ -3,6 +3,8 @@ package image import ( "bytes" "errors" + "fmt" + "image" "os/exec" "runtime" "sync" @@ -14,7 +16,10 @@ import ( var vipsPath string var once sync.Once -var ErrUnsupportedFormat = errors.New("unsupported image format") +var ( + // ErrNotSupportedForThumbnail is returned if the image format is not supported for thumbnail generation + ErrNotSupportedForThumbnail = errors.New("unsupported image format for thumbnail") +) type ThumbnailEncoder struct { ffmpeg ffmpeg.Encoder @@ -45,7 +50,7 @@ func NewThumbnailEncoder(ffmpegEncoder ffmpeg.Encoder) ThumbnailEncoder { // GetThumbnail returns the thumbnail image of the provided image resized to // the provided max size. It resizes based on the largest X/Y direction. // It returns nil and an error if an error occurs reading, decoding or encoding -// the image. +// the image, or if the image is not suitable for thumbnails. func (e *ThumbnailEncoder) GetThumbnail(img *models.Image, maxSize int) ([]byte, error) { reader, err := openSourceImage(img.Path) if err != nil { @@ -57,13 +62,24 @@ func (e *ThumbnailEncoder) GetThumbnail(img *models.Image, maxSize int) ([]byte, return nil, err } - _, format, err := DecodeSourceImage(img) + data := buf.Bytes() + + // use NewBufferString to copy the buffer, rather than reuse it + _, format, err := image.DecodeConfig(bytes.NewBufferString(string(data))) if err != nil { return nil, err } - if format != nil && *format == "gif" { - return buf.Bytes(), nil + animated := format == formatGif + + // #2266 - if image is webp, then determine if it is animated + if format == formatWebP { + animated = isWebPAnimated(data) + } + + // #2266 - don't generate a thumbnail for animated images + if animated { + return nil, fmt.Errorf("%w: %s", ErrNotSupportedForThumbnail, format) } // vips has issues loading files from stdin on Windows diff --git a/pkg/image/webp.go b/pkg/image/webp.go new file mode 100644 index 000000000..0cfb30291 --- /dev/null +++ b/pkg/image/webp.go @@ -0,0 +1,46 @@ +package image + +import ( + "bytes" +) + +const ( + formatWebP = "webp" + formatGif = "gif" +) + +// https://developers.google.com/speed/webp/docs/riff_container +func isWebPAnimated(buf []byte) bool { + const ( + webPHeaderStart = 8 + webPHeaderEnd = 12 + webPHeader = "WEBP" + + animationHeaderLoc = 16 + minAnimSignatureIndex = 20 + + maxSize = 48 + ) + + // truncate the buffer to the max size + if len(buf) > maxSize { + buf = buf[:maxSize] + } + + isWebp := len(buf) >= webPHeaderEnd && string(buf[webPHeaderStart:webPHeaderEnd]) == "WEBP" // is WEBP + + if isWebp { + const animBit byte = 1 << 1 + if len(buf) > minAnimSignatureIndex { + // Animation Bit is set and ANIM header is present + return (buf[animationHeaderLoc]&animBit == animBit) && containsAnimSignature(buf[minAnimSignatureIndex:]) + } + } + return false +} + +// https://developers.google.com/speed/webp/docs/riff_container#animation +func containsAnimSignature(buf []byte) bool { + index := bytes.Index(buf, []byte("ANIM")) + return index != -1 +} diff --git a/pkg/image/webp_internal_test.go b/pkg/image/webp_internal_test.go new file mode 100644 index 000000000..cd2fbbf8b --- /dev/null +++ b/pkg/image/webp_internal_test.go @@ -0,0 +1,66 @@ +package image + +import "testing" + +func Test_isWebPAnimated(t *testing.T) { + tests := []struct { + name string + buf []byte + want bool + }{ + { + "basic animated", + []byte{ + 0x52, 0x49, 0x46, 0x46, 0xb2, 0x3a, 0x17, 0x00, 0x57, 0x45, 0x42, 0x50, 0x56, 0x50, 0x38, 0x58, + 0x0a, 0x00, 0x00, 0x00, 0x12, 0x00, 0x00, 0x00, 0x7f, 0x02, 0x00, 0x55, 0x01, 0x00, 0x41, 0x4e, + 0x49, 0x4d, 0x06, 0x00, 0x00, 0x00, 0xff, 0xff, 0xff, 0xff, 0x00, 0x00, 0x41, 0x4e, 0x4d, 0x46, + }, + true, + }, + { + "static webp", + []byte{ + 0x52, 0x49, 0x46, 0x46, 0x68, 0x76, 0x00, 0x00, 0x57, 0x45, 0x42, 0x50, 0x56, 0x50, 0x38, 0x20, + 0x5c, 0x76, 0x00, 0x00, 0xd2, 0xbe, 0x01, 0x9d, 0x01, 0x2a, 0x26, 0x02, 0x70, 0x01, 0x3e, 0xd5, + 0x4e, 0x97, 0x43, 0xa2, 0x06, 0x16, 0xd1, 0xb4, 0x88, 0x03, 0x51, 0x39, 0xb7, 0x13, 0x33, 0x75, + }, + false, + }, + { + "false animated bit", + []byte{ + 0x52, 0x49, 0x46, 0x46, 0xb2, 0x3a, 0x17, 0x00, 0x57, 0x45, 0x42, 0x50, 0x56, 0x50, 0x38, 0x58, + 0x09, 0x00, 0x00, 0x00, 0x12, 0x00, 0x00, 0x00, 0x7f, 0x02, 0x00, 0x55, 0x01, 0x00, 0x41, 0x4e, + 0x49, 0x4d, 0x06, 0x00, 0x00, 0x00, 0xff, 0xff, 0xff, 0xff, 0x00, 0x00, 0x41, 0x4e, 0x4d, 0x46, + }, + false, + }, + { + "ANIM out of range", + []byte{ + 0x52, 0x49, 0x46, 0x46, 0xb2, 0x3a, 0x17, 0x00, 0x57, 0x45, 0x42, 0x50, 0x56, 0x50, 0x38, 0x58, + 0x0a, 0x00, 0x00, 0x00, 0x12, 0x00, 0x00, 0x00, 0x7f, 0x02, 0x00, 0x55, 0x01, 0x00, 0x3e, 0xd5, + 0x4e, 0x97, 0x43, 0xa2, 0x06, 0x16, 0xd1, 0xb4, 0x88, 0x03, 0x51, 0x39, 0xb7, 0x13, 0x33, 0x75, + 0x41, 0x4e, 0x49, 0x4d, 0x41, 0x4e, 0x49, 0x4d, 0x41, 0x4e, 0x49, 0x4d, 0x41, 0x4e, 0x49, 0x4d, + }, + false, + }, + { + "not webp", + []byte{ + 0x52, 0x49, 0x46, 0x46, 0xb2, 0x3a, 0x17, 0x00, 0x58, 0x45, 0x42, 0x50, 0x56, 0x50, 0x38, 0x58, + 0x0a, 0x00, 0x00, 0x00, 0x12, 0x00, 0x00, 0x00, 0x7f, 0x02, 0x00, 0x55, 0x01, 0x00, 0x3e, 0xd5, + 0x4e, 0x97, 0x43, 0xa2, 0x06, 0x16, 0xd1, 0xb4, 0x88, 0x03, 0x51, 0x39, 0xb7, 0x13, 0x33, 0x75, + 0x41, 0x4e, 0x49, 0x4d, 0x41, 0x4e, 0x49, 0x4d, 0x41, 0x4e, 0x49, 0x4d, 0x41, 0x4e, 0x49, 0x4d, + }, + false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := isWebPAnimated(tt.buf); got != tt.want { + t.Errorf("isWebPAnimated() = %v, want %v", got, tt.want) + } + }) + } +} diff --git a/ui/v2.5/src/components/Changelog/versions/v0140.md b/ui/v2.5/src/components/Changelog/versions/v0140.md index 8d3e1a33d..2ff70066b 100644 --- a/ui/v2.5/src/components/Changelog/versions/v0140.md +++ b/ui/v2.5/src/components/Changelog/versions/v0140.md @@ -3,6 +3,7 @@ * Improved autotag performance. ([#2368](https://github.com/stashapp/stash/pull/2368)) ### 🐛 Bug fixes +* Don't generate jpg thumbnails for animated webp files. ([#2388](https://github.com/stashapp/stash/pull/2388)) * Removed warnings and incorrect error message in json scrapers. ([#2375](https://github.com/stashapp/stash/pull/2375)) * Ensure identify continues using other scrapers if a scrape returns no results. ([#2375](https://github.com/stashapp/stash/pull/2375)) * Continue trying to identify scene if scraper fails. ([#2375](https://github.com/stashapp/stash/pull/2375)) \ No newline at end of file