feat(webp): add partial success to conversion

So we only keep images that couldn't be optimized and return the chapter
Belphemur · Aug 28, 2024 · ad35e26 · ad35e26
1 parent d7f55fa
commit ad35e26
Show file tree

Hide file tree

Showing 6 changed files with 117 additions and 30 deletions.
diff --git a/converter/converter.go b/converter/converter.go
@@ -12,6 +12,9 @@ import (
 type Converter interface {
 	// Format of the converter
 	Format() (format constant.ConversionFormat)
+	// ConvertChapter converts a manga chapter to the specified format.
+	//
+	// Returns partial success where some pages are converted and some are not.
 	ConvertChapter(chapter *manga.Chapter, quality uint8, split bool, progress func(message string, current uint32, total uint32)) (*manga.Chapter, error)
 	PrepareConverter() error
 }

diff --git a/converter/converter_test.go b/converter/converter_test.go
@@ -14,34 +14,46 @@ import (
 func TestConvertChapter(t *testing.T) {
 
 	testCases := []struct {
-		name           string
-		genTestChapter func(path string) (*manga.Chapter, error)
-		split          bool
-		expectFailure  []constant.ConversionFormat
+		name                 string
+		genTestChapter       func(path string) (*manga.Chapter, error)
+		split                bool
+		expectFailure        []constant.ConversionFormat
+		expectPartialSuccess []constant.ConversionFormat
 	}{
 		{
-			name:           "All split pages",
-			genTestChapter: genBigPages,
-			split:          true,
-			expectFailure:  []constant.ConversionFormat{},
+			name:                 "All split pages",
+			genTestChapter:       genHugePages,
+			split:                true,
+			expectFailure:        []constant.ConversionFormat{},
+			expectPartialSuccess: []constant.ConversionFormat{},
 		},
 		{
-			name:           "Big Pages, no split",
-			genTestChapter: genBigPages,
-			split:          false,
-			expectFailure:  []constant.ConversionFormat{constant.WebP},
+			name:                 "Big Pages, no split",
+			genTestChapter:       genHugePages,
+			split:                false,
+			expectFailure:        []constant.ConversionFormat{constant.WebP},
+			expectPartialSuccess: []constant.ConversionFormat{},
 		},
 		{
-			name:           "No split pages",
-			genTestChapter: genSmallPages,
-			split:          false,
-			expectFailure:  []constant.ConversionFormat{},
+			name:                 "No split pages",
+			genTestChapter:       genSmallPages,
+			split:                false,
+			expectFailure:        []constant.ConversionFormat{},
+			expectPartialSuccess: []constant.ConversionFormat{},
 		},
 		{
-			name:           "Mix of split and no split pages",
-			genTestChapter: genMixSmallBig,
-			split:          true,
-			expectFailure:  []constant.ConversionFormat{},
+			name:                 "Mix of split and no split pages",
+			genTestChapter:       genMixSmallBig,
+			split:                true,
+			expectFailure:        []constant.ConversionFormat{},
+			expectPartialSuccess: []constant.ConversionFormat{},
+		},
+		{
+			name:                 "Mix of Huge and small page",
+			genTestChapter:       genMixSmallHuge,
+			split:                false,
+			expectFailure:        []constant.ConversionFormat{},
+			expectPartialSuccess: []constant.ConversionFormat{constant.WebP},
 		},
 	}
 	// Load test genTestChapter from testdata
@@ -72,6 +84,10 @@ func TestConvertChapter(t *testing.T) {
 
 					convertedChapter, err := converter.ConvertChapter(chapter, quality, tc.split, progress)
 					if err != nil {
+						if convertedChapter != nil && slices.Contains(tc.expectPartialSuccess, converter.Format()) {
+							t.Logf("Partial success to convert genTestChapter: %v", err)
+							return
+						}
 						if slices.Contains(tc.expectFailure, converter.Format()) {
 							t.Logf("Expected failure to convert genTestChapter: %v", err)
 							return
@@ -85,6 +101,10 @@ func TestConvertChapter(t *testing.T) {
 						t.Fatalf("no pages were converted")
 					}
 
+					if len(convertedChapter.Pages) != len(chapter.Pages) {
+						t.Fatalf("converted chapter has different number of pages")
+					}
+
 					for _, page := range convertedChapter.Pages {
 						if page.Extension != ".webp" {
 							t.Errorf("page %d was not converted to webp format", page.Index)
@@ -96,7 +116,7 @@ func TestConvertChapter(t *testing.T) {
 	}
 }
 
-func genBigPages(path string) (*manga.Chapter, error) {
+func genHugePages(path string) (*manga.Chapter, error) {
 	file, err := os.Open(path)
 	if err != nil {
 		return nil, err
@@ -182,3 +202,32 @@ func genMixSmallBig(path string) (*manga.Chapter, error) {
 		Pages:    pages,
 	}, nil
 }
+
+func genMixSmallHuge(path string) (*manga.Chapter, error) {
+	file, err := os.Open(path)
+	if err != nil {
+		return nil, err
+	}
+	defer file.Close()
+
+	var pages []*manga.Page
+	for i := 0; i < 10; i++ { // Assuming there are 5 pages for the test
+		img := image.NewRGBA(image.Rect(0, 0, 300, 2000*(i+1)))
+		buf := new(bytes.Buffer)
+		err := jpeg.Encode(buf, img, nil)
+		if err != nil {
+			return nil, err
+		}
+		page := &manga.Page{
+			Index:     uint16(i),
+			Contents:  buf,
+			Extension: ".jpg",
+		}
+		pages = append(pages, page)
+	}
+
+	return &manga.Chapter{
+		FilePath: path,
+		Pages:    pages,
+	}, nil
+}
diff --git a/converter/errors/converter_errors.go b/converter/errors/converter_errors.go
@@ -0,0 +1,13 @@
+package errors
+
+type PageIgnoredError struct {
+	s string
+}
+
+func (e *PageIgnoredError) Error() string {
+	return e.s
+}
+
+func NewPageIgnored(text string) error {
+	return &PageIgnoredError{text}
+}
diff --git a/converter/webp/webp_converter.go b/converter/webp/webp_converter.go
@@ -2,8 +2,10 @@ package webp
 
 import (
 	"bytes"
+	"errors"
 	"fmt"
 	"github.com/belphemur/CBZOptimizer/converter/constant"
+	converterrors "github.com/belphemur/CBZOptimizer/converter/errors"
 	"github.com/belphemur/CBZOptimizer/manga"
 	"github.com/oliamb/cutter"
 	"golang.org/x/exp/slices"
@@ -109,25 +111,30 @@ func (converter *Converter) ConvertChapter(chapter *manga.Chapter, quality uint8
 			splitNeeded, img, format, err := converter.checkPageNeedsSplit(page, split)
 			if err != nil {
 				errChan <- err
+				// Partial error in this case, we want the page, but not converting it
+				if img != nil {
+					wgConvertedPages.Add(1)
+					pagesChan <- manga.NewContainer(page, img, format, false)
+				}
 				return
 			}
 
 			if !splitNeeded {
 				wgConvertedPages.Add(1)
-				pagesChan <- manga.NewContainer(page, img, format)
+				pagesChan <- manga.NewContainer(page, img, format, true)
 				return
 			}
 			images, err := converter.cropImage(img)
 			if err != nil {
-				errChan <- fmt.Errorf("error converting page %d of genTestChapter %s to webp: %v", page.Index, chapter.FilePath, err)
+				errChan <- err
 				return
 			}
 
 			atomic.AddUint32(&totalPages, uint32(len(images)-1))
 			for i, img := range images {
 				page := &manga.Page{Index: page.Index, IsSplitted: true, SplitPartIndex: uint16(i)}
 				wgConvertedPages.Add(1)
-				pagesChan <- manga.NewContainer(page, img, "N/A")
+				pagesChan <- manga.NewContainer(page, img, "N/A", true)
 			}
 		}(page)
 	}
@@ -142,8 +149,9 @@ func (converter *Converter) ConvertChapter(chapter *manga.Chapter, quality uint8
 		errList = append(errList, err)
 	}
 
+	var aggregatedError error = nil
 	if len(errList) > 0 {
-		return nil, fmt.Errorf("encountered errors: %v", errList)
+		aggregatedError = errors.Join(errList...)
 	}
 
 	slices.SortFunc(pages, func(a, b *manga.Page) int {
@@ -156,7 +164,7 @@ func (converter *Converter) ConvertChapter(chapter *manga.Chapter, quality uint8
 
 	runtime.GC()
 
-	return chapter, nil
+	return chapter, aggregatedError
 }
 
 func (converter *Converter) cropImage(img image.Image) ([]image.Image, error) {
@@ -203,7 +211,7 @@ func (converter *Converter) checkPageNeedsSplit(page *manga.Page, splitRequested
 	height := bounds.Dy()
 
 	if height >= webpMaxHeight && !splitRequested {
-		return false, img, format, fmt.Errorf("page[%d] height %d exceeds maximum height %d of webp format", page.Index, height, webpMaxHeight)
+		return false, img, format, converterrors.NewPageIgnored(fmt.Sprintf("page %d is too tall to be converted to webp format", page.Index))
 	}
 	return height >= converter.maxHeight && splitRequested, img, format, nil
 }
@@ -212,6 +220,9 @@ func (converter *Converter) convertPage(container *manga.PageContainer, quality
 	if container.Format == "webp" {
 		return container, nil
 	}
+	if !container.IsToBeConverted {
+		return container, nil
+	}
 	converted, err := converter.convert(container.Image, uint(quality))
 	if err != nil {
 		return nil, err

diff --git a/manga/page_container.go b/manga/page_container.go
@@ -10,8 +10,10 @@ type PageContainer struct {
 	Image image.Image
 	// Format is a string representing the format of the image (e.g., "png", "jpeg", "webp").
 	Format string
+	// IsToBeConverted is a boolean flag indicating whether the image needs to be converted to another format.
+	IsToBeConverted bool
 }
 
-func NewContainer(Page *Page, img image.Image, format string) *PageContainer {
-	return &PageContainer{Page: Page, Image: img, Format: format}
+func NewContainer(Page *Page, img image.Image, format string, isToBeConverted bool) *PageContainer {
+	return &PageContainer{Page: Page, Image: img, Format: format, IsToBeConverted: isToBeConverted}
 }
diff --git a/utils/optimize.go b/utils/optimize.go
@@ -1,9 +1,11 @@
 package utils
 
 import (
+	"errors"
 	"fmt"
 	"github.com/belphemur/CBZOptimizer/cbz"
 	"github.com/belphemur/CBZOptimizer/converter"
+	errors2 "github.com/belphemur/CBZOptimizer/converter/errors"
 	"log"
 	"strings"
 )
@@ -38,8 +40,15 @@ func Optimize(options *OptimizeOptions) error {
 		}
 	})
 	if err != nil {
-		return fmt.Errorf("failed to convert chapter: %v", err)
+		var pageIgnoredError *errors2.PageIgnoredError
+		if !errors.As(err, &pageIgnoredError) {
+			return fmt.Errorf("failed to convert chapter: %v", err)
+		}
+	}
+	if convertedChapter == nil {
+		return fmt.Errorf("failed to convert chapter")
 	}
+
 	convertedChapter.SetConverted()
 
 	// Write the converted chapter back to a CBZ file