Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feature(thumbnails): add the ability to define custom image processors #7409

Merged
merged 5 commits into from
Oct 17, 2023
Merged

feature(thumbnails): add the ability to define custom image processors #7409

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
edbcb8a
feature(thumbnails): add the ability to define custom image processors
fschade Oct 4, 2023
ce0361f
fix(ci): add exported member comment
fschade Oct 5, 2023
d491d3c
docs(thumbnails): mention processors in readme
fschade Oct 5, 2023
a514837
fix: codacy and code review feedback
fschade Oct 15, 2023
689c1da
fix: thumbnail readme markdown
fschade Oct 16, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 22 additions & 0 deletions changelog/unreleased/thumbnail-processors.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
Enhancement: Thumbnail generation with image processors

Thumbnails can now be changed during creation, previously the images were always scaled to fit the given frame,
but it could happen that the images were cut off because they could not be placed better due to the aspect ratio.

This pr introduces the possibility of specifying how the behavior should be, following processors are available

* resize
* fit
* fill
* thumbnail

the processor can be applied by adding the processor query param to the request, e.g. `processor=fit`, `processor=fill`, ...

to find out more how the individual processors work please read https://github.com/disintegration/imaging

if no processor is provided it behaves the same as before (resize for gif's and thumbnail for all other)

https://github.com/owncloud/ocis/pull/7409
https://github.com/owncloud/enterprise/issues/6057
https://github.com/owncloud/ocis/issues/5179
https://github.com/owncloud/web/issues/7728
116 changes: 63 additions & 53 deletions protogen/gen/ocis/services/thumbnails/v0/thumbnails.pb.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 4 additions & 2 deletions protogen/proto/ocis/services/thumbnails/v0/thumbnails.proto
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,9 +47,11 @@ message GetThumbnailRequest {
int32 width = 3;
// The height of the thumbnail
int32 height = 4;
// Indicates which image processor to use
string processor = 5;
oneof source {
ocis.messages.thumbnails.v0.WebdavSource webdav_source = 5;
ocis.messages.thumbnails.v0.CS3Source cs3_source = 6;
ocis.messages.thumbnails.v0.WebdavSource webdav_source = 6;
ocis.messages.thumbnails.v0.CS3Source cs3_source = 7;
}
}

Expand Down
23 changes: 17 additions & 6 deletions services/thumbnails/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,9 @@ The relevant environment variables defining file locations are:
- (2) `STORAGE_USERS_OCIS_ROOT`
- (3) `THUMBNAILS_FILESYSTEMSTORAGE_ROOT`

(1) ... Having a default set by the Infinite Scale code, but if defined, used as base path for other services.
(2) ... Source files, defaults to (1) plus path component, but can be freely defined if required.
(3) ... Target files, defaults to (1) plus path component, but can be freely defined if required.
(1) ... Having a default set by the Infinite Scale code, but if defined, used as base path for other services.
(2) ... Source files, defaults to (1) plus path component, but can be freely defined if required.
(3) ... Target files, defaults to (1) plus path component, but can be freely defined if required.

For details and defaults for these environment variables see the ocis admin documentation.

Expand Down Expand Up @@ -45,9 +45,20 @@ Various resolutions can be defined via `THUMBNAILS_RESOLUTIONS`. A requestor can

Example:

Requested: 18x12
Available: 30x20, 15x10, 9x6
Returned: 15x10
Requested: 18x12
Available: 30x20, 15x10, 9x6
Returned: 15x10
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why does it return the next-smaller instead of the next-bigger resolution? Would look nicer if clients down-scale a bit instead of up-scale?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not sure, i have to have a look into the codebase, to be honest, i do not know every thumbnail service detail. please create a issue if we should change something.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, I’ll play with it and I’ll open new issues if needed.


## Thumbnail Processors

Image generation can be configured by defining different processors, following processors are available:

* `resize`
* `fit`
* `fill`
* `thumbnail`

To apply one of those, a query parameter has to be added to the request, e.g. `?processor=fit`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the default behavior in case this parameter isn't provided?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I recall correctly, this is optional, means behaviour equals before if not added.
Maybe to write: Image generation can optionally be configured...
But @fschade can clarify best.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if no processor is provided it uses the default behaviour as before (Resize fort gif, Thumbnail for everything else)


## Deleting Thumbnails

Expand Down
11 changes: 6 additions & 5 deletions services/thumbnails/pkg/service/grpc/v0/service.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,16 +16,17 @@ import (
"github.com/cs3org/reva/v2/pkg/storagespace"
"github.com/cs3org/reva/v2/pkg/utils"
"github.com/golang-jwt/jwt/v4"
"github.com/pkg/errors"
merrors "go-micro.dev/v4/errors"
"google.golang.org/grpc/metadata"

"github.com/owncloud/ocis/v2/ocis-pkg/log"
thumbnailssvc "github.com/owncloud/ocis/v2/protogen/gen/ocis/services/thumbnails/v0"
"github.com/owncloud/ocis/v2/services/thumbnails/pkg/preprocessor"
"github.com/owncloud/ocis/v2/services/thumbnails/pkg/service/grpc/v0/decorators"
tjwt "github.com/owncloud/ocis/v2/services/thumbnails/pkg/service/jwt"
"github.com/owncloud/ocis/v2/services/thumbnails/pkg/thumbnail"
"github.com/owncloud/ocis/v2/services/thumbnails/pkg/thumbnail/imgsource"
"github.com/pkg/errors"
merrors "go-micro.dev/v4/errors"
"google.golang.org/grpc/metadata"
)

// NewService returns a service implementation for Service.
Expand Down Expand Up @@ -124,7 +125,7 @@ func (g Thumbnail) handleCS3Source(ctx context.Context, req *thumbnailssvc.GetTh
if tType == "" {
tType = req.GetThumbnailType().String()
}
tr, err := thumbnail.PrepareRequest(int(req.Width), int(req.Height), tType, sRes.GetInfo().GetChecksum().GetSum())
tr, err := thumbnail.PrepareRequest(int(req.Width), int(req.Height), tType, sRes.GetInfo().GetChecksum().GetSum(), req.Processor)
if err != nil {
return "", merrors.BadRequest(g.serviceID, err.Error())
}
Expand Down Expand Up @@ -207,7 +208,7 @@ func (g Thumbnail) handleWebdavSource(ctx context.Context, req *thumbnailssvc.Ge
if tType == "" {
tType = req.GetThumbnailType().String()
}
tr, err := thumbnail.PrepareRequest(int(req.Width), int(req.Height), tType, sRes.GetInfo().GetChecksum().GetSum())
tr, err := thumbnail.PrepareRequest(int(req.Width), int(req.Height), tType, sRes.GetInfo().GetChecksum().GetSum(), req.Processor)
if err != nil {
return "", merrors.BadRequest(g.serviceID, err.Error())
}
Expand Down
4 changes: 2 additions & 2 deletions services/thumbnails/pkg/thumbnail/encoding.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,8 +114,8 @@ func EncoderForType(fileType string) (Encoder, error) {
}

// GetExtForMime return the supported extension by mime
func GetExtForMime(mime string) string {
ext := strings.TrimPrefix(strings.TrimSpace(strings.ToLower(mime)), "image/")
func GetExtForMime(fileType string) string {
ext := strings.TrimPrefix(strings.TrimSpace(strings.ToLower(fileType)), "image/")
switch ext {
case typeJpg, typeJpeg, typePng, typeGif:
return ext
Expand Down
Loading