From 3de4584a45f7c3fe9a1ebd8519b22a23157b0c55 Mon Sep 17 00:00:00 2001 From: David Christofas Date: Mon, 23 Mar 2020 11:52:33 +0100 Subject: [PATCH] use predefined resolutions for thumbnail generation --- pkg/command/server.go | 2 +- pkg/config/config.go | 20 ++++++---- pkg/flagset/flagset.go | 10 ++++- pkg/server/grpc/server.go | 1 + pkg/service/v0/service.go | 35 ++++++++++------- pkg/thumbnails/resolution/resolution_test.go | 14 +++++++ pkg/thumbnails/resolution/resolutions.go | 39 +++++++++++++------ pkg/thumbnails/resolution/resolutions_test.go | 27 +++++++++++-- pkg/thumbnails/storage/filesystem.go | 3 +- pkg/thumbnails/storage/inmemory.go | 2 +- pkg/thumbnails/storage/storage.go | 9 +++-- pkg/thumbnails/thumbnails.go | 19 +++++---- 12 files changed, 125 insertions(+), 56 deletions(-) diff --git a/pkg/command/server.go b/pkg/command/server.go index af6cafecf..9967701ef 100644 --- a/pkg/command/server.go +++ b/pkg/command/server.go @@ -29,11 +29,11 @@ func Server(cfg *config.Config) *cli.Command { Usage: "Start integrated server", Flags: flagset.ServerWithConfig(cfg), Before: func(c *cli.Context) error { + cfg.Thumbnail.Resolutions = c.StringSlice("thumbnail-resolution") return nil }, Action: func(c *cli.Context) error { logger := NewLogger(cfg) - if cfg.Tracing.Enabled { switch t := cfg.Tracing.Type; t { case "agent": diff --git a/pkg/config/config.go b/pkg/config/config.go index e84b514b9..37286518a 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -33,13 +33,12 @@ type Tracing struct { // Config combines all available configuration parts. type Config struct { - File string - Log Log - Debug Debug - Server Server - Tracing Tracing - FileSystemStorage FileSystemStorage - WebDavSource WebDavSource + File string + Log Log + Debug Debug + Server Server + Tracing Tracing + Thumbnail Thumbnail } // FileSystemStorage defines the available filesystem storage configuration. @@ -52,6 +51,13 @@ type WebDavSource struct { BaseURL string } +// Thumbnail defines the available thumbnail related configuration. +type Thumbnail struct { + Resolutions []string + FileSystemStorage FileSystemStorage + WebDavSource WebDavSource +} + // New initializes a new configuration with or without defaults. func New() *Config { return &Config{} diff --git a/pkg/flagset/flagset.go b/pkg/flagset/flagset.go index 2f34817b1..19b3f9506 100644 --- a/pkg/flagset/flagset.go +++ b/pkg/flagset/flagset.go @@ -142,14 +142,20 @@ func ServerWithConfig(cfg *config.Config) []cli.Flag { Value: filepath.Join(os.TempDir(), "ocis-thumbnails/"), Usage: "Root path of the filesystem storage directory", EnvVars: []string{"THUMBNAILS_FILESYSTEMSTORAGE_ROOT"}, - Destination: &cfg.FileSystemStorage.RootDirectory, + Destination: &cfg.Thumbnail.FileSystemStorage.RootDirectory, }, &cli.StringFlag{ Name: "webdavsource-baseurl", Value: "http://localhost:9140/remote.php/webdav/", Usage: "Base url for a webdav api", EnvVars: []string{"THUMBNAILS_WEBDAVSOURCE_BASEURL"}, - Destination: &cfg.WebDavSource.BaseURL, + Destination: &cfg.Thumbnail.WebDavSource.BaseURL, + }, + &cli.StringSliceFlag{ + Name: "thumbnail-resolution", + Value: cli.NewStringSlice("16x16", "32x32", "64x64", "128x128"), + Usage: "--thumbnail-resolution 16x16 [--thumbnail-resolution 32x32]", + EnvVars: []string{"THUMBNAILS_RESOLUTIONs"}, }, } } diff --git a/pkg/server/grpc/server.go b/pkg/server/grpc/server.go index 96c64e1d0..37a79b755 100644 --- a/pkg/server/grpc/server.go +++ b/pkg/server/grpc/server.go @@ -24,6 +24,7 @@ func NewService(opts ...Option) grpc.Service { { thumbnail = svc.NewService( svc.Config(options.Config), + svc.Logger(options.Logger), ) thumbnail = svc.NewInstrument(thumbnail, options.Metrics) thumbnail = svc.NewLogging(thumbnail, options.Logger) diff --git a/pkg/service/v0/service.go b/pkg/service/v0/service.go index 3a2f973ac..faf22ad1e 100644 --- a/pkg/service/v0/service.go +++ b/pkg/service/v0/service.go @@ -8,23 +8,29 @@ import ( v0proto "github.com/owncloud/ocis-thumbnails/pkg/proto/v0" "github.com/owncloud/ocis-thumbnails/pkg/thumbnails" "github.com/owncloud/ocis-thumbnails/pkg/thumbnails/imgsource" + "github.com/owncloud/ocis-thumbnails/pkg/thumbnails/resolution" "github.com/owncloud/ocis-thumbnails/pkg/thumbnails/storage" ) // NewService returns a service implementation for Service. func NewService(opts ...Option) v0proto.ThumbnailServiceHandler { options := newOptions(opts...) - + logger := options.Logger + resolutions, err := resolution.Init(options.Config.Thumbnail.Resolutions) + if err != nil { + logger.Fatal().Err(err).Msg("resolutions not configured correctly") + } svc := Thumbnail{ manager: thumbnails.NewSimpleManager( storage.NewFileSystemStorage( - options.Config.FileSystemStorage, - options.Logger, + options.Config.Thumbnail.FileSystemStorage, + logger, ), - options.Logger, + logger, ), - source: imgsource.NewWebDavSource(options.Config.WebDavSource), - logger: options.Logger, + resolutions: resolutions, + source: imgsource.NewWebDavSource(options.Config.Thumbnail.WebDavSource), + logger: logger, } return svc @@ -32,9 +38,10 @@ func NewService(opts ...Option) v0proto.ThumbnailServiceHandler { // Thumbnail implements the GRPC handler. type Thumbnail struct { - manager thumbnails.Manager - source imgsource.Source - logger log.Logger + manager thumbnails.Manager + resolutions resolution.Resolutions + source imgsource.Source + logger log.Logger } // GetThumbnail retrieves a thumbnail for an image @@ -44,12 +51,12 @@ func (g Thumbnail) GetThumbnail(ctx context.Context, req *v0proto.GetRequest, rs // TODO: better error responses return fmt.Errorf("can't be encoded. filetype %s not supported", req.Filetype.String()) } + r := g.resolutions.ClosestMatch(int(req.Width), int(req.Height)) tCtx := thumbnails.Context{ - Width: int(req.Width), - Height: int(req.Height), - ImagePath: req.Filepath, - Encoder: encoder, - ETag: req.Etag, + Resolution: r, + ImagePath: req.Filepath, + Encoder: encoder, + ETag: req.Etag, } thumbnail := g.manager.GetStored(tCtx) diff --git a/pkg/thumbnails/resolution/resolution_test.go b/pkg/thumbnails/resolution/resolution_test.go index 18d6e4676..b8c3e6c88 100644 --- a/pkg/thumbnails/resolution/resolution_test.go +++ b/pkg/thumbnails/resolution/resolution_test.go @@ -9,6 +9,20 @@ func TestParseWithEmptyString(t *testing.T) { } } +func TestParseWithInvalidWidth(t *testing.T) { + _, err := Parse("invalidx42") + if err == nil { + t.Error("Parse with invalid width should return an error.") + } +} + +func TestParseWithInvalidHeight(t *testing.T) { + _, err := Parse("42xinvalid") + if err == nil { + t.Error("Parse with invalid height should return an error.") + } +} + func TestParse(t *testing.T) { rStr := "42x23" r, _ := Parse(rStr) diff --git a/pkg/thumbnails/resolution/resolutions.go b/pkg/thumbnails/resolution/resolutions.go index a64bd9eed..c17b47135 100644 --- a/pkg/thumbnails/resolution/resolutions.go +++ b/pkg/thumbnails/resolution/resolutions.go @@ -3,6 +3,7 @@ package resolution import ( "fmt" "math" + "sort" ) // Init creates an instance of Resolutions from resolution strings. @@ -15,6 +16,16 @@ func Init(rStrs []string) (Resolutions, error) { } rs = append(rs, r) } + sort.Slice(rs, func(i, j int) bool { + left := rs[i] + right := rs[j] + + leftSize := left.Width * left.Height + rightSize := right.Width * right.Height + + return leftSize < rightSize + }) + return rs, nil } @@ -22,31 +33,37 @@ func Init(rStrs []string) (Resolutions, error) { type Resolutions []Resolution // ClosestMatch returns the resolution which is closest to the provided resolution. +// If there is no exact match the resolution will be the next higher one. +// If the given resolution is bigger than all available resolutions the biggest available one is used. func (r Resolutions) ClosestMatch(width, height int) Resolution { if len(r) == 0 { return Resolution{Width: width, Height: height} } isLandscape := width > height - givenLen := math.Max(float64(width), float64(height)) + givenLen := int(math.Max(float64(width), float64(height))) // Initialize with the first resolution - match := r[0] - matchLen := dimensionLength(match, isLandscape) - minDiff := math.Abs(givenLen - float64(matchLen)) + var match Resolution + minDiff := math.MaxInt32 for i := 1; i < len(r); i++ { - r := r[i] - rLen := dimensionLength(r, isLandscape) - diff := math.Abs(givenLen - float64(rLen)) - - if diff <= minDiff { - minDiff = diff - match = r + current := r[i] + len := dimensionLength(current, isLandscape) + diff := givenLen - len + if diff > 0 { continue } + absDiff := int(math.Abs(float64(diff))) + if absDiff < minDiff { + minDiff = absDiff + match = current + } } + if match == (Resolution{}) { + match = r[len(r)-1] + } return match } diff --git a/pkg/thumbnails/resolution/resolutions_test.go b/pkg/thumbnails/resolution/resolutions_test.go index 37b0e8367..4ec67c43d 100644 --- a/pkg/thumbnails/resolution/resolutions_test.go +++ b/pkg/thumbnails/resolution/resolutions_test.go @@ -52,6 +52,25 @@ func TestInitWithMultipleResolutions(t *testing.T) { } } +func TestInitWithMultipleResolutionsShouldBeSorted(t *testing.T) { + rStrs := []string{"32x32", "64x64", "16x16", "128x128"} + rs, err := Init(rStrs) + if err != nil { + t.Errorf("Init with valid parameter should not fail. Error: %s.\n", err.Error()) + } + + for i := 0; i < len(rs)-1; i++ { + current := rs[i] + currentSize := current.Width * current.Height + next := rs[i] + nextSize := next.Width * next.Height + + if currentSize > nextSize { + t.Error("Resolutions are not sorted.") + } + + } +} func TestClosestMatchWithEmptyResolutions(t *testing.T) { rs, _ := Init(nil) width := 24 @@ -66,12 +85,12 @@ func TestClosestMatchWithEmptyResolutions(t *testing.T) { func TestClosestMatch(t *testing.T) { rs, _ := Init([]string{"16x16", "24x24", "32x32", "64x64", "128x128"}) table := [][]int{ - []int{17, 17, 16, 16}, - []int{12, 17, 16, 16}, + []int{17, 17, 24, 24}, + []int{12, 17, 24, 24}, []int{24, 24, 24, 24}, []int{20, 20, 24, 24}, - []int{20, 80, 64, 64}, - []int{80, 20, 64, 64}, + []int{20, 80, 128, 128}, + []int{80, 20, 128, 128}, []int{48, 48, 64, 64}, []int{1024, 1024, 128, 128}, } diff --git a/pkg/thumbnails/storage/filesystem.go b/pkg/thumbnails/storage/filesystem.go index e59b77170..c8f604245 100644 --- a/pkg/thumbnails/storage/filesystem.go +++ b/pkg/thumbnails/storage/filesystem.go @@ -6,7 +6,6 @@ import ( "io/ioutil" "os" "path/filepath" - "strconv" "github.com/owncloud/ocis-pkg/v2/log" "github.com/owncloud/ocis-thumbnails/pkg/config" @@ -68,7 +67,7 @@ func (s FileSystem) Set(key string, img []byte) error { func (s FileSystem) BuildKey(ctx Context) string { etag := ctx.ETag filetype := ctx.Types[0] - filename := strconv.Itoa(ctx.Width) + "x" + strconv.Itoa(ctx.Height) + "." + filetype + filename := ctx.Resolution.String() + "." + filetype key := new(bytes.Buffer) key.WriteString(etag[:2]) diff --git a/pkg/thumbnails/storage/inmemory.go b/pkg/thumbnails/storage/inmemory.go index c8f02a9ae..74a78befd 100644 --- a/pkg/thumbnails/storage/inmemory.go +++ b/pkg/thumbnails/storage/inmemory.go @@ -32,7 +32,7 @@ func (s InMemory) Set(key string, thumbnail []byte) error { func (s InMemory) BuildKey(ctx Context) string { parts := []string{ ctx.ETag, - string(ctx.Width) + "x" + string(ctx.Height), + ctx.Resolution.String(), strings.Join(ctx.Types, ","), } return strings.Join(parts, "+") diff --git a/pkg/thumbnails/storage/storage.go b/pkg/thumbnails/storage/storage.go index 8f5f3d9de..68de3c9ea 100644 --- a/pkg/thumbnails/storage/storage.go +++ b/pkg/thumbnails/storage/storage.go @@ -1,11 +1,12 @@ package storage +import "github.com/owncloud/ocis-thumbnails/pkg/thumbnails/resolution" + // Context combines different attributes needed for storage operations. type Context struct { - ETag string - Types []string - Width int - Height int + ETag string + Types []string + Resolution resolution.Resolution } // Storage defines the interface for a thumbnail store. diff --git a/pkg/thumbnails/thumbnails.go b/pkg/thumbnails/thumbnails.go index 76223228d..289d03462 100644 --- a/pkg/thumbnails/thumbnails.go +++ b/pkg/thumbnails/thumbnails.go @@ -6,16 +6,16 @@ import ( "github.com/nfnt/resize" "github.com/owncloud/ocis-pkg/v2/log" + "github.com/owncloud/ocis-thumbnails/pkg/thumbnails/resolution" "github.com/owncloud/ocis-thumbnails/pkg/thumbnails/storage" ) // Context bundles information needed to generate a thumbnail for afile type Context struct { - Width int - Height int - ImagePath string - Encoder Encoder - ETag string + Resolution resolution.Resolution + ImagePath string + Encoder Encoder + ETag string } // Manager is responsible for generating thumbnails @@ -69,16 +69,15 @@ func (s SimpleManager) GetStored(ctx Context) []byte { } func (s SimpleManager) generate(ctx Context, img image.Image) image.Image { - thumbnail := resize.Thumbnail(uint(ctx.Width), uint(ctx.Height), img, resize.Lanczos2) + thumbnail := resize.Thumbnail(uint(ctx.Resolution.Width), uint(ctx.Resolution.Height), img, resize.Lanczos2) return thumbnail } func mapToStorageContext(ctx Context) storage.Context { sCtx := storage.Context{ - ETag: ctx.ETag, - Width: ctx.Width, - Height: ctx.Height, - Types: ctx.Encoder.Types(), + ETag: ctx.ETag, + Resolution: ctx.Resolution, + Types: ctx.Encoder.Types(), } return sCtx }