From c2f809ed2cffd7ebbd815ae2faf161d3e08536f0 Mon Sep 17 00:00:00 2001 From: Copilot <198982749+Copilot@users.noreply.github.com> Date: Fri, 3 Jul 2026 09:15:56 -0400 Subject: [PATCH] fix(memory): use staging folder to avoid keeping everything in memory Co-authored-by: Belphemur <197810+Belphemur@users.noreply.github.com> Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Fixes #203 --- .gitattributes | 1 + .github/copilot-instructions.md | 14 +- README.md | 2 +- go.sum | 2 - internal/cbz/cbz_creator.go | 22 +++- internal/manga/chapter.go | 24 +++- internal/manga/page.go | 72 ++++++++++- internal/utils/optimize.go | 9 ++ internal/utils/optimize_integration_test.go | 7 + .../optimize_large_file_integration_test.go | 121 ++++++++++++++++++ pkg/converter/webp/webp_converter.go | 89 ++++++++++--- pkg/converter/webp/webp_converter_test.go | 19 ++- testdata/large/large_chapter.cbz | 3 + 13 files changed, 352 insertions(+), 33 deletions(-) create mode 100644 .gitattributes create mode 100644 internal/utils/optimize_large_file_integration_test.go create mode 100644 testdata/large/large_chapter.cbz diff --git a/.gitattributes b/.gitattributes new file mode 100644 index 0000000..5014420 --- /dev/null +++ b/.gitattributes @@ -0,0 +1 @@ +testdata/large/*.cbz filter=lfs diff=lfs merge=lfs -text diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index 8303721..ab73d0e 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -95,6 +95,15 @@ go test -v ./pkg/converter/... go test -v ./internal/utils/... ``` +A large-file integration test (`TestOptimizeIntegration_LargeFile`) exercises the +optimize pipeline against a ~1GB CBZ fixture stored via Git LFS +(`testdata/large/large_chapter.cbz`, tracked in `.gitattributes`). It is skipped +by default; fetch the fixture with `git lfs pull` and opt in with: + +```bash +CBZ_RUN_LARGE_FILE_TEST=1 go test -v ./internal/utils/... -run TestOptimizeIntegration_LargeFile +``` + ### Linting ```bash @@ -281,8 +290,9 @@ Releases are automated via goreleaser: ## Performance Considerations -- **Parallelism:** Use `--parallelism` flag to control concurrent chapter processing -- **Memory:** Large images are processed in-memory; consider system RAM when setting parallelism +- **Parallelism:** Use `--parallelism` flag to control concurrent chapter processing. Regardless of this value, the total number of pages converted at the same time (i.e. concurrent `cwebp` processes) is capped process-wide to the number of CPU cores, so raising parallelism spreads that budget across more chapters instead of multiplying resource usage. +- **Memory:** Original page images are loaded in memory, but converted pages are staged to a temporary folder on disk as soon as they're produced (see `manga.Page.TempFilePath` / `manga.Chapter.TempDir`) instead of being held in memory until the whole chapter is written out. The staging folder is cleaned up once the chapter has been written to its final CBZ file. +- **Disk:** Since converted pages are staged on disk, ensure enough free space (roughly the size of the converted chapter) is available in the OS temp directory. - **Timeouts:** Use `--timeout` flag to prevent hanging on problematic files - **WebP Quality:** Balance quality (0-100) vs file size; default is 85 diff --git a/README.md b/README.md index 04bcb8b..5e16c3a 100644 --- a/README.md +++ b/README.md @@ -87,7 +87,7 @@ docker run -v /path/to/comics:/comics ghcr.io/belphemur/cbzoptimizer:latest watc ### Flags - `--quality`, `-q`: Quality for conversion (0-100). Default is 85. -- `--parallelism`, `-n`: Number of chapters to convert in parallel. Default is 2. +- `--parallelism`, `-n`: Number of chapters to convert in parallel. Default is 2. Regardless of this value, the total number of pages converted at the same time (i.e. concurrent `cwebp` processes) is capped to the number of CPU cores, so increasing parallelism spreads that budget across more chapters rather than multiplying resource usage. - `--override`, `-o`: Override the original files. For CBZ files, overwrites the original. For CBR files, deletes the original CBR and creates a new CBZ. Default is false. - `--split`, `-s`: Split long pages into smaller chunks. Default is false. - `--format`, `-f`: Format to convert the images to (currently supports: webp). Default is webp. diff --git a/go.sum b/go.sum index 59fd2db..f1f5095 100644 --- a/go.sum +++ b/go.sum @@ -52,8 +52,6 @@ github.com/envoyproxy/go-control-plane v0.9.1-0.20191026205805-5f8ba28d4473/go.m github.com/envoyproxy/protoc-gen-validate v0.1.0/go.mod h1:iSmxcyjqTsJpI2R4NaDN7+kN2VEUnK/pcBlmesArF7c= github.com/frankban/quicktest v1.14.6 h1:7Xjx+VpznH+oBnejlPUj8oUpdxnVs4f8XU8WnHkI4W8= github.com/frankban/quicktest v1.14.6/go.mod h1:4ptaffx2x8+WTWXmUCuVU6aPUX1/Mz7zb5vbUoiM6w0= -github.com/fsnotify/fsnotify v1.9.0 h1:2Ml+OJNzbYCTzsxtv8vKSFD9PbJjmhYF14k/jKC7S9k= -github.com/fsnotify/fsnotify v1.9.0/go.mod h1:8jBTzvmWwFyi3Pb8djgCCO5IBqzKJ/Jwo8TRcHyHii0= github.com/fsnotify/fsnotify v1.10.1 h1:b0/UzAf9yR5rhf3RPm9gf3ehBPpf0oZKIjtpKrx59Ho= github.com/fsnotify/fsnotify v1.10.1/go.mod h1:TLheqan6HD6GBK6PrDWyDPBaEV8LspOxvPSjC+bVfgo= github.com/go-gl/glfw v0.0.0-20190409004039-e6da0acd62b1/go.mod h1:vR7hzQXu2zJy9AVAgeJqvqgH9Q5CA+iKCZ2gyEVpxRU= diff --git a/internal/cbz/cbz_creator.go b/internal/cbz/cbz_creator.go index 94f92ea..9ce0463 100644 --- a/internal/cbz/cbz_creator.go +++ b/internal/cbz/cbz_creator.go @@ -3,6 +3,7 @@ package cbz import ( "archive/zip" "fmt" + "io" "os" "time" @@ -56,7 +57,7 @@ func WriteChapterToCBZ(chapter *manga.Chapter, outputFilePath string) error { Bool("is_splitted", page.IsSplitted). Uint16("split_part", page.SplitPartIndex). Str("filename", fileName). - Int("size", len(page.Contents.Bytes())). + Uint64("size", page.Size). Msg("Writing page to CBZ archive") // Create a new file in the ZIP archive @@ -70,17 +71,30 @@ func WriteChapterToCBZ(chapter *manga.Chapter, outputFilePath string) error { return fmt.Errorf("failed to create file in .cbz: %w", err) } - // Write the page contents to the file - bytesWritten, err := fileWriter.Write(page.Contents.Bytes()) + // Stream the page contents into the archive. This transparently + // handles pages staged on disk (see manga.Page.TempFilePath) so we + // never need to hold the whole chapter's contents in memory at once. + pageReader, err := page.Open() + if err != nil { + log.Error().Str("output_path", outputFilePath).Str("filename", fileName).Err(err).Msg("Failed to open page contents") + return fmt.Errorf("failed to open page contents: %w", err) + } + + bytesWritten, err := io.Copy(fileWriter, pageReader) + closeErr := pageReader.Close() if err != nil { log.Error().Str("output_path", outputFilePath).Str("filename", fileName).Err(err).Msg("Failed to write page contents") return fmt.Errorf("failed to write page contents: %w", err) } + if closeErr != nil { + log.Error().Str("output_path", outputFilePath).Str("filename", fileName).Err(closeErr).Msg("Failed to close page contents reader") + return fmt.Errorf("failed to close page contents reader: %w", closeErr) + } log.Debug(). Str("output_path", outputFilePath). Str("filename", fileName). - Int("bytes_written", bytesWritten). + Int64("bytes_written", bytesWritten). Msg("Page written successfully") } diff --git a/internal/manga/chapter.go b/internal/manga/chapter.go index 57a09d0..7f6acaf 100644 --- a/internal/manga/chapter.go +++ b/internal/manga/chapter.go @@ -1,6 +1,10 @@ package manga -import "time" +import ( + "fmt" + "os" + "time" +) type Chapter struct { // FilePath is the path to the chapter's directory. @@ -13,6 +17,11 @@ type Chapter struct { IsConverted bool // ConvertedTime is a pointer to a time.Time object that indicates when the chapter was converted. Nil mean not converted. ConvertedTime time.Time + // TempDir, when non-empty, is a staging temp folder holding converted + // page contents on disk (see Page.TempFilePath) rather than fully in + // memory. It should be removed via Cleanup once the chapter has been + // written out (or is no longer needed). + TempDir string } // SetConverted sets the IsConverted field to true and sets the ConvertedTime field to the current time. @@ -20,3 +29,16 @@ func (chapter *Chapter) SetConverted() { chapter.IsConverted = true chapter.ConvertedTime = time.Now() } + +// Cleanup removes the chapter's staging temp folder (if any), releasing any +// page contents that were staged to disk during conversion. +func (chapter *Chapter) Cleanup() error { + if chapter.TempDir == "" { + return nil + } + if err := os.RemoveAll(chapter.TempDir); err != nil { + return fmt.Errorf("failed to remove staging temp folder: %w", err) + } + chapter.TempDir = "" + return nil +} diff --git a/internal/manga/page.go b/internal/manga/page.go index b687768..6faf8e9 100644 --- a/internal/manga/page.go +++ b/internal/manga/page.go @@ -1,6 +1,13 @@ package manga -import "bytes" +import ( + "bytes" + "fmt" + "io" + "os" + + "github.com/rs/zerolog/log" +) type Page struct { // Index of the page in the chapter. @@ -9,10 +16,71 @@ type Page struct { Extension string `json:"extension" jsonschema:"description=Extension of the page image."` // Size of the page in bytes Size uint64 `json:"-"` - // Contents of the page + // Contents of the page. Nil when the page contents have been staged to + // disk (see TempFilePath) to bound memory usage. Contents *bytes.Buffer `json:"-"` + // TempFilePath, when non-empty, points to a file on disk (in a staging + // temp folder) holding the page contents instead of keeping them fully + // in memory. Use Open() to transparently read the page contents + // regardless of where they are stored. + TempFilePath string `json:"-"` // IsSplitted tell us if the page was cropped to multiple pieces IsSplitted bool `json:"is_cropped" jsonschema:"description=Was this page cropped."` // SplitPartIndex represent the index of the crop if the page was cropped SplitPartIndex uint16 `json:"crop_part_index" jsonschema:"description=Index of the crop if the image was cropped."` } + +// Open returns a reader for the page contents, transparently handling +// whether the contents are held in memory (Contents) or staged on disk +// (TempFilePath). The caller is responsible for closing the returned +// reader. +func (page *Page) Open() (io.ReadCloser, error) { + if page.TempFilePath != "" { + file, err := os.Open(page.TempFilePath) + if err != nil { + return nil, fmt.Errorf("failed to open staged page contents: %w", err) + } + return file, nil + } + if page.Contents == nil { + return nil, fmt.Errorf("page has no contents: neither TempFilePath nor Contents is set") + } + return io.NopCloser(bytes.NewReader(page.Contents.Bytes())), nil +} + +// Stage writes the given content to a file in tempDir instead of keeping it +// in memory, updating Extension, Size and TempFilePath accordingly and +// clearing Contents. This is used after converting a page so that only the +// pages currently being processed are held in memory, bounding memory usage +// for chapters with many/large pages. +func (page *Page) Stage(tempDir string, content *bytes.Buffer, extension string) error { + file, err := os.CreateTemp(tempDir, "page-*.tmp") + if err != nil { + return fmt.Errorf("failed to create staging file: %w", err) + } + fileName := file.Name() + + written, writeErr := file.Write(content.Bytes()) + closeErr := file.Close() + if writeErr == nil && closeErr == nil && written == content.Len() { + page.TempFilePath = fileName + page.Extension = extension + page.Size = uint64(written) + page.Contents = nil + return nil + } + + // Something went wrong: remove the incomplete/partial staging file + // rather than leaving corrupted data behind on disk. + if removeErr := os.Remove(fileName); removeErr != nil { + log.Warn().Str("file", fileName).Err(removeErr).Msg("Failed to remove incomplete staging file") + } + + if writeErr != nil { + return fmt.Errorf("failed to write staging file: %w", writeErr) + } + if closeErr != nil { + return fmt.Errorf("failed to close staging file: %w", closeErr) + } + return fmt.Errorf("short write to staging file: wrote %d of %d bytes", written, content.Len()) +} diff --git a/internal/utils/optimize.go b/internal/utils/optimize.go index 6a8b912..d5fc3a0 100644 --- a/internal/utils/optimize.go +++ b/internal/utils/optimize.go @@ -91,6 +91,15 @@ func Optimize(options *OptimizeOptions) error { return fmt.Errorf("failed to convert chapter") } + // Clean up the staging temp folder (if any) used to hold converted page + // contents on disk once we're done with the chapter, regardless of + // success or failure below. + defer func() { + if cleanupErr := convertedChapter.Cleanup(); cleanupErr != nil { + log.Warn().Str("file", chapter.FilePath).Err(cleanupErr).Msg("Failed to remove staging temp folder") + } + }() + log.Debug(). Str("file", chapter.FilePath). Int("original_pages", len(chapter.Pages)). diff --git a/internal/utils/optimize_integration_test.go b/internal/utils/optimize_integration_test.go index 505ad30..20421cc 100644 --- a/internal/utils/optimize_integration_test.go +++ b/internal/utils/optimize_integration_test.go @@ -51,6 +51,13 @@ func TestOptimizeIntegration(t *testing.T) { if err != nil { return err } + // Skip the "large" fixtures directory: it holds the Git LFS-tracked + // fixture used exclusively by TestOptimizeIntegration_LargeFile, + // which may only be a small LFS pointer file if the content wasn't + // fetched, and shouldn't be exercised by this generic test. + if info.IsDir() && filepath.Base(path) == "large" { + return filepath.SkipDir + } if !info.IsDir() { fileName := strings.ToLower(info.Name()) if (strings.HasSuffix(fileName, ".cbz") || strings.HasSuffix(fileName, ".cbr")) && !strings.Contains(fileName, "converted") { diff --git a/internal/utils/optimize_large_file_integration_test.go b/internal/utils/optimize_large_file_integration_test.go new file mode 100644 index 0000000..1bf8d55 --- /dev/null +++ b/internal/utils/optimize_large_file_integration_test.go @@ -0,0 +1,121 @@ +package utils + +import ( + "fmt" + "io" + "os" + "path/filepath" + "runtime" + "strings" + "testing" + + "github.com/belphemur/CBZOptimizer/v2/internal/utils/errs" + "github.com/belphemur/CBZOptimizer/v2/pkg/converter" + "github.com/belphemur/CBZOptimizer/v2/pkg/converter/constant" + "github.com/rs/zerolog/log" +) + +// largeTestFile is a ~1GB synthetic CBZ fixture stored via Git LFS (see +// .gitattributes). It is used to exercise the optimize pipeline with a +// chapter large enough to make in-memory-only handling impractical, and to +// validate that converted pages are streamed to/from a staging temp folder +// (see manga.Page.TempFilePath / manga.Chapter.TempDir) instead of blowing +// up memory usage. +const largeTestFile = "../../testdata/large/large_chapter.cbz" + +// TestOptimizeIntegration_LargeFile is opt-in (set CBZ_RUN_LARGE_FILE_TEST=1) +// since it processes a ~1GB fixture and can take a while to run. It is +// automatically skipped if the fixture is unavailable (e.g. Git LFS content +// wasn't fetched, leaving only a pointer file) or in short mode. +func TestOptimizeIntegration_LargeFile(t *testing.T) { + if testing.Short() { + t.Skip("Skipping large file integration test in short mode") + } + if os.Getenv("CBZ_RUN_LARGE_FILE_TEST") == "" { + t.Skip("Skipping large file integration test; set CBZ_RUN_LARGE_FILE_TEST=1 to run it") + } + + info, err := os.Stat(largeTestFile) + if err != nil { + t.Skipf("large test fixture not found: %v", err) + } + // If Git LFS content wasn't fetched (e.g. `actions/checkout` without + // `lfs: true`), the file on disk is just a small pointer text file + // rather than the real ~1GB fixture. Detect and skip gracefully instead + // of failing the whole suite. + const minExpectedSize = 500 * 1024 * 1024 // 500MB + if info.Size() < minExpectedSize { + t.Skipf("large test fixture looks like a Git LFS pointer (size=%d), skipping; run `git lfs pull`", info.Size()) + } + + tempDir, err := os.MkdirTemp("", "test_optimize_large_file") + if err != nil { + t.Fatal(err) + } + defer errs.CaptureGeneric(&err, os.RemoveAll, tempDir, "failed to remove temporary directory") + + converterInstance, err := converter.Get(constant.WebP) + if err != nil { + t.Skip("WebP converter not available, skipping large file integration test") + } + if err := converterInstance.PrepareConverter(); err != nil { + t.Skip("Failed to prepare WebP converter, skipping large file integration test") + } + + cbzFile := filepath.Join(tempDir, "large_chapter.cbz") + if err := copyFile(largeTestFile, cbzFile); err != nil { + t.Fatal(err) + } + + var memBefore, memAfter runtime.MemStats + runtime.GC() + runtime.ReadMemStats(&memBefore) + + options := &OptimizeOptions{ + ChapterConverter: converterInstance, + Path: cbzFile, + Quality: 85, + Override: false, + Split: true, + } + + err = Optimize(options) + if err != nil { + t.Fatalf("failed to optimize large chapter: %v", err) + } + + runtime.GC() + runtime.ReadMemStats(&memAfter) + log.Info(). + Uint64("heap_alloc_before", memBefore.HeapAlloc). + Uint64("heap_alloc_after", memAfter.HeapAlloc). + Int64("input_size", info.Size()). + Msg("Large file integration test memory usage") + + outputFile := strings.TrimSuffix(cbzFile, ".cbz") + "_converted.cbz" + if _, err := os.Stat(outputFile); err != nil { + t.Fatalf("expected converted output file %s to exist: %v", outputFile, err) + } +} + +// copyFile copies src to dst using streaming file I/O so that the whole +// file content is never held in memory at once, which matters for the +// large fixture used by this test. +func copyFile(src, dst string) (err error) { + in, err := os.Open(src) + if err != nil { + return fmt.Errorf("failed to open source file: %w", err) + } + defer errs.Capture(&err, in.Close, "failed to close source file") + + out, err := os.Create(dst) + if err != nil { + return fmt.Errorf("failed to create destination file: %w", err) + } + defer errs.Capture(&err, out.Close, "failed to close destination file") + + if _, err := io.Copy(out, in); err != nil { + return fmt.Errorf("failed to copy file contents: %w", err) + } + return nil +} diff --git a/pkg/converter/webp/webp_converter.go b/pkg/converter/webp/webp_converter.go index ad63b49..78b3dcf 100644 --- a/pkg/converter/webp/webp_converter.go +++ b/pkg/converter/webp/webp_converter.go @@ -9,6 +9,7 @@ import ( _ "image/gif" _ "image/jpeg" "image/png" + "os" "runtime" "sync" "sync/atomic" @@ -28,6 +29,15 @@ type Converter struct { maxHeight int cropHeight int isPrepared bool + // pageWorkerGuard limits how many pages are converted (i.e. how many + // cwebp binary processes are spawned) concurrently across the whole + // application, regardless of how many chapters are being processed in + // parallel (see the --parallelism flag). Without this shared limit, the + // per-chapter worker pool (sized to runtime.NumCPU()) would be + // multiplied by the number of chapters processed concurrently, leading + // to CPU/memory oversubscription since every page conversion spawns an + // external cwebp process. + pageWorkerGuard chan struct{} } func (converter *Converter) Format() (format constant.ConversionFormat) { @@ -37,9 +47,10 @@ func (converter *Converter) Format() (format constant.ConversionFormat) { func New() *Converter { return &Converter{ //maxHeight: 16383 / 2, - maxHeight: 4000, - cropHeight: 2000, - isPrepared: false, + maxHeight: 4000, + cropHeight: 2000, + isPrepared: false, + pageWorkerGuard: make(chan struct{}, runtime.NumCPU()), } } @@ -70,6 +81,18 @@ func (converter *Converter) ConvertChapter(ctx context.Context, chapter *manga.C return nil, err } + // Stage converted pages on disk in a temp folder instead of keeping them + // all in memory until the whole chapter is written out. This bounds + // memory usage for chapters with many/large pages; the caller is + // responsible for calling chapter.Cleanup() once the chapter has been + // written to its final CBZ file. + tempDir, err := os.MkdirTemp("", "cbzoptimizer-*") + if err != nil { + log.Error().Str("chapter", chapter.FilePath).Err(err).Msg("Failed to create staging temp folder") + return nil, fmt.Errorf("failed to create staging temp folder: %w", err) + } + chapter.TempDir = tempDir + var wgConvertedPages sync.WaitGroup maxGoroutines := runtime.NumCPU() @@ -80,7 +103,12 @@ func (converter *Converter) ConvertChapter(ctx context.Context, chapter *manga.C var wgPages sync.WaitGroup wgPages.Add(len(chapter.Pages)) - guard := make(chan struct{}, maxGoroutines) + // guard is shared across all chapters processed concurrently by this + // converter instance (see pageWorkerGuard doc comment) so that the total + // number of concurrently running cwebp processes stays bounded to + // runtime.NumCPU(), no matter how many chapters are being converted in + // parallel at the same time. + guard := converter.pageWorkerGuard pagesMutex := sync.Mutex{} var pages []*manga.Page var totalPages = uint32(len(chapter.Pages)) @@ -89,13 +117,25 @@ func (converter *Converter) ConvertChapter(ctx context.Context, chapter *manga.C Str("chapter", chapter.FilePath). Int("total_pages", len(chapter.Pages)). Int("worker_count", maxGoroutines). + Str("staging_dir", tempDir). Msg("Initialized conversion worker pool") + // failWithCleanup removes the staging temp folder before returning a + // fatal error for which no chapter is returned to the caller (and thus + // nobody else will clean it up). + failWithCleanup := func(err error) (*manga.Chapter, error) { + if removeErr := os.RemoveAll(tempDir); removeErr != nil { + log.Warn().Str("chapter", chapter.FilePath).Err(removeErr).Msg("Failed to remove staging temp folder after error") + } + chapter.TempDir = "" + return nil, err + } + // Check if context is already cancelled select { case <-ctx.Done(): log.Warn().Str("chapter", chapter.FilePath).Msg("Chapter conversion cancelled due to timeout") - return nil, ctx.Err() + return failWithCleanup(ctx.Err()) default: } @@ -122,7 +162,7 @@ func (converter *Converter) ConvertChapter(ctx context.Context, chapter *manga.C default: } - convertedPage, err := converter.convertPage(pageToConvert, quality) + convertedPage, err := converter.convertPage(pageToConvert, quality, tempDir) if err != nil { if convertedPage == nil { select { @@ -142,9 +182,14 @@ func (converter *Converter) ConvertChapter(ctx context.Context, chapter *manga.C } return } - convertedPage.Page.Contents = buffer - convertedPage.Page.Extension = ".png" - convertedPage.Page.Size = uint64(buffer.Len()) + if err := convertedPage.Page.Stage(tempDir, buffer, ".png"); err != nil { + select { + case errChan <- err: + case <-ctx.Done(): + return + } + return + } } pagesMutex.Lock() defer pagesMutex.Unlock() @@ -160,7 +205,7 @@ func (converter *Converter) ConvertChapter(ctx context.Context, chapter *manga.C select { case <-ctx.Done(): log.Warn().Str("chapter", chapter.FilePath).Msg("Chapter conversion cancelled due to timeout") - return nil, ctx.Err() + return failWithCleanup(ctx.Err()) default: } @@ -251,11 +296,13 @@ func (converter *Converter) ConvertChapter(ctx context.Context, chapter *manga.C // Conversion completed successfully case <-ctx.Done(): log.Warn().Str("chapter", chapter.FilePath).Msg("Chapter conversion cancelled due to timeout") - return nil, ctx.Err() + return failWithCleanup(ctx.Err()) } close(errChan) - close(guard) + // Note: guard is the converter's shared pageWorkerGuard and must not be + // closed here since it is reused by subsequent (and possibly concurrent) + // ConvertChapter calls. var errList []error for err := range errChan { @@ -402,7 +449,7 @@ func (converter *Converter) checkPageNeedsSplit(page *manga.Page, splitRequested return needsSplit, img, format, nil } -func (converter *Converter) convertPage(container *manga.PageContainer, quality uint8) (*manga.PageContainer, error) { +func (converter *Converter) convertPage(container *manga.PageContainer, quality uint8, tempDir string) (*manga.PageContainer, error) { log.Debug(). Uint16("page_index", container.Page.Index). Str("format", container.Format). @@ -439,12 +486,22 @@ func (converter *Converter) convertPage(container *manga.PageContainer, quality return nil, err } - container.SetConverted(converted, ".webp") + originalSize := container.Page.Size + convertedSize := converted.Len() + + if err := container.Page.Stage(tempDir, converted, ".webp"); err != nil { + log.Error(). + Uint16("page_index", container.Page.Index). + Err(err). + Msg("Failed to stage converted page to disk") + return nil, err + } + container.HasBeenConverted = true log.Debug(). Uint16("page_index", container.Page.Index). - Int("original_size", len(container.Page.Contents.Bytes())). - Int("converted_size", len(converted.Bytes())). + Uint64("original_size", originalSize). + Int("converted_size", convertedSize). Msg("Page conversion completed") return container, nil diff --git a/pkg/converter/webp/webp_converter_test.go b/pkg/converter/webp/webp_converter_test.go index 187dcfc..8f07727 100644 --- a/pkg/converter/webp/webp_converter_test.go +++ b/pkg/converter/webp/webp_converter_test.go @@ -9,6 +9,7 @@ import ( "image/gif" "image/jpeg" "image/png" + "io" "sync" "testing" "time" @@ -86,11 +87,19 @@ func createTestPage(t *testing.T, index int, width, height int, format string) * } func validateConvertedImage(t *testing.T, page *manga.Page) { - require.NotNil(t, page.Contents) - require.Greater(t, page.Contents.Len(), 0) + require.True(t, page.Contents != nil || page.TempFilePath != "", "page should have contents in memory or staged on disk") + require.Greater(t, page.Size, uint64(0)) + + reader, err := page.Open() + require.NoError(t, err) + defer reader.Close() + + data, err := io.ReadAll(reader) + require.NoError(t, err) + require.Greater(t, len(data), 0) // Try to decode the image - img, format, err := image.Decode(bytes.NewReader(page.Contents.Bytes())) + img, format, err := image.Decode(bytes.NewReader(data)) require.NoError(t, err, "Failed to decode converted image") if page.Extension == ".webp" { @@ -291,7 +300,7 @@ func TestConverter_convertPage(t *testing.T) { require.NoError(t, err) container := manga.NewContainer(page, img, tt.format, tt.isToBeConverted) - converted, err := converter.convertPage(container, 80) + converted, err := converter.convertPage(container, 80, t.TempDir()) if tt.expectError { assert.Error(t, err) @@ -327,7 +336,7 @@ func TestConverter_convertPage_EncodingError(t *testing.T) { container := manga.NewContainer(corruptedPage, nil, "png", true) - converted, err := converter.convertPage(container, 80) + converted, err := converter.convertPage(container, 80, t.TempDir()) // This should return nil container and error because encoding will fail with nil image assert.Error(t, err) diff --git a/testdata/large/large_chapter.cbz b/testdata/large/large_chapter.cbz new file mode 100644 index 0000000..cba3b67 --- /dev/null +++ b/testdata/large/large_chapter.cbz @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:8880c8909521c3716a339a39a29a4bf1114921a8b4651ff8ca04da7ce3a54b6a +size 1001668690