Support thumbnail image view #980

monkeyWie · 2020-06-08T16:01:59Z

Reopen PR,now is using github.com/disintegration/imaging lib to handle image,and refactor code.
Regarding the frontend code,I run npm run lint to fix the errors detected by eslint.

o1egl

I tried to run it locally and I see a lot of errors in ui.

o1egl · 2020-06-09T17:38:22Z

http/thumbnail.go

+	if err != nil {
+		return errToStatus(err), err
+	}
+	dstImg := fitResizeImage(srcImg)


Please use https://pkg.go.dev/github.com/disintegration/imaging?tab=doc#FormatFromExtension with a fixed size

o1egl · 2020-06-09T17:39:29Z

http/thumbnail.go

+	err = func() error {
+		switch file.Extension {
+		case ".jpg", ".jpeg":
+			return jpeg.Encode(w, dstImg, nil)
+		case ".png":
+			return png.Encode(w, dstImg)
+		case ".gif":
+			return gif.Encode(w, dstImg, nil)
+		default:
+			return errors.ErrNotExist
+		}
+	}()


Please use https://pkg.go.dev/github.com/disintegration/imaging?tab=doc#FormatFromExtension and https://pkg.go.dev/github.com/disintegration/imaging?tab=doc#Encode functions

o1egl · 2020-06-09T17:44:21Z

http/thumbnail.go

+	return 0, nil
+}
+
+const maxSize = 1080


It doesn't make sense to resize an image to 1080px and display it as a thumbnail

When resize the image resolution, can effectively reduce the size of the picture, for example reduce a 5MB picture to 500KB,so I choose 1080px as the reduce threshold.

If the actual size of the thumbnail is 100x100 for example then why we should return 1080x1080 image for that?

In fact when the width and height < 1080,will return raw image,as below:

func fitResizeImage(srcImage image.Image) image.Image { width := srcImage.Bounds().Dx() height := srcImage.Bounds().Dy() if width > maxSize && width > height { width = maxSize height = 0 } else if height > maxSize && height > width { width = 0 height = maxSize } else { return srcImage } return imaging.Resize(srcImage, width, height, imaging.NearestNeighbor) }

I still do not understand why we should transfer additional data over the network and resize the image in the browser? Since the thumbnail size is fixed on ui, let's do the same on the backend

What do you think of the following changes?

add /preview/thumb/file API for list icon,fixed size 128*128,like this:

change /thumbnail/file API to /preview/big/file for review, other unchanged.

I think we can use single handler with path param /preview/{size}.

Of course,and the most important thing is to preview the picture max size, or keep the 1080px?

Let's keep it 1080px. User can always download original one.

Hi,I have a new commit,please review again.

monkeyWie · 2020-06-10T12:08:41Z

I tried to run it locally and I see a lot of errors in ui.

I found this error,it was caused by merge conflict,I will fix it.

o1egl · 2020-06-10T13:48:19Z

http/thumbnail.go

+	}
+
+	if file.IsDir || file.Type != "image" {
+		return http.StatusNotFound, nil


How about returning StatusNotImplemented code?

Suggested change

return http.StatusNotFound, nil

return http.StatusNotImplemented, nil

o1egl

I would rewrite it to something like that. With that we can keep this handler flexible to support different file types:

const (
	sizeThumb = "thumb"
	sizeBig = "big"
)

type imageProcessor func(img image.Image) (image.Image, error)

func previewHandler(w http.ResponseWriter, r *http.Request, d *data) (int, error) {
	if !d.user.Perm.Download {
		return http.StatusAccepted, nil
	}

	file, err := files.NewFileInfo(files.FileOptions{
		Fs:      d.user.Fs,
		Path:    r.URL.Path,
		Modify:  d.user.Perm.Modify,
		Expand:  true,
		Checker: d,
	})
	if err != nil {
		return errToStatus(err), err
	}

	size := mux.Vars(r)["size"]
	switch file.Type {
	case "image":
		return handleImagePreview(w, file, size)
	default:
		return http.StatusNotImplemented, fmt.Errorf("can't create preview for %s type", file.Type)
	}
}

func handleImagePreview(w http.ResponseWriter, file *files.FileInfo, size string) (int, error) {
	var imgProcessor imageProcessor
	switch size {
	case sizeBig:
		imgProcessor = func(img image.Image) (image.Image, error) {
			return imaging.Fit(img, 1080, 1080, imaging.Lanczos), nil
		}
	case sizeThumb:
		imgProcessor = func(img image.Image) (image.Image, error) {
			return imaging.Thumbnail(img, 128, 128, imaging.Box), nil
		}
	default:
		return http.StatusBadRequest, fmt.Errorf("unsupported preview size %s", size)
	}

	fd, err := file.Fs.Open(file.Path)
	if err != nil {
		return errToStatus(err), err
	}
	defer fd.Close()
	format, err := imaging.FormatFromExtension(file.Extension)
	if err != nil {
		return http.StatusNotImplemented, err
	}
	img, err := imaging.Decode(fd, imaging.AutoOrientation(true))
	if err != nil {
		return errToStatus(err), err
	}
	img, err = imgProcessor(img)
	if err != nil {
		return errToStatus(err), err
	}
	if imaging.Encode(w, img, format) != nil {
		return errToStatus(err), err
	}
	return 0, nil
}

o1egl · 2020-06-15T13:34:17Z

http/prewiew.go

+	}
+	var dstImage image.Image
+	if size == sizeBig {
+		dstImage = fitResizeImage(srcImg, 1080)


Please use imaging.Fit()

oh,this pretty good!

o1egl · 2020-06-15T13:36:20Z

http/prewiew.go

+	if size == sizeBig {
+		dstImage = fitResizeImage(srcImg, 1080)
+	} else {
+		dstImage = fitResizeImage(srcImg, 128)


Please use imaging .Thumbnail()

Also I would consider using one of those filters. We don't need super sharp image for thumbnails

- Linear Bilinear resampling filter, produces a smooth output. Faster than cubic filters. - Box Simple and fast averaging filter appropriate for downscaling. When upscaling it's similar to NearestNeighbor. - NearestNeighbor Fastest resampling filter, no antialiasing.

o1egl · 2020-06-15T15:55:16Z

http/http.go

@@ -59,6 +59,8 @@ func NewHandler(store *storage.Storage, server *settings.Server) (http.Handler,
 	api.Handle("/settings", monkey(settingsPutHandler, "")).Methods("PUT")

 	api.PathPrefix("/raw").Handler(monkey(rawHandler, "/api/raw")).Methods("GET")
+	api.PathPrefix("/preview/thumb").Handler(monkey(previewThumbHandler, "/api/preview/thumb")).Methods("GET")


Use path params

Suggested change

api.PathPrefix("/preview/thumb").Handler(monkey(previewThumbHandler, "/api/preview/thumb")).Methods("GET")

api.PathPrefix("/preview/{size}").Handler(monkey(previewThumbHandler, "/api/preview")).Methods("GET")

If use path params,I need trim r.URL.Path,like /thumb/path/file trim to /path/file,do you have any good suggestions for this?

You can define filepath as another param.

o1egl · 2020-06-15T16:06:16Z

http/prewiew.go

+const sizeThumb = "thumb"
+const sizeBig = "big"


Please group constants

Suggested change

const sizeThumb = "thumb"

const sizeBig = "big"

const (

sizeThumb = "thumb"

sizeBig = "big"

)

o1egl · 2020-06-15T16:11:11Z

http/prewiew.go

+	})
+}
+
+func previewFileHandler(w http.ResponseWriter, r *http.Request, file *files.FileInfo, size string) (int, error) {


Suggested change

func previewFileHandler(w http.ResponseWriter, r *http.Request, file *files.FileInfo, size string) (int, error) {

func imagePreviewHandler(w http.ResponseWriter, r *http.Request, file *files.FileInfo, size string) (int, error) {

o1egl · 2020-06-15T16:15:20Z

http/prewiew.go

+	if r.URL.Query().Get("inline") == "true" {
+		w.Header().Set("Content-Disposition", "inline")
+	} else {
+		// As per RFC6266 section 4.3
+		w.Header().Set("Content-Disposition", "attachment; filename*=utf-8''"+url.PathEscape(file.Name))
+	}


I think it should be done just before encoding process

monkeyWie · 2020-06-22T14:27:16Z

@o1egl I have new submits,please review again.

o1egl

@monkeyWie it still doesn't work. The ui is broken

o1egl · 2020-06-22T17:01:01Z

http/preview.go

+	}
+
+	// Resolve file path from URL
+	path := "/" + strings.Join(strings.Split(r.URL.Path, "/")[2:], "/")


Why not to use the path as a variable like the size? Now it looks like a real magic :)

Do you mean like this?

api.PathPrefix("/preview/{size}/{path}")

This way doesn't work when the path like photos/xxx.jpg,the request URI is /preview/big/photos/xxx.jpg.

Use regexp for that /preview/{size}{path:.*}

I got it👍

http/preview.go

monkeyWie · 2020-06-23T09:55:16Z

@monkeyWie it still doesn't work. The ui is broken

Can't build frontend?

o1egl · 2020-06-23T11:27:28Z

@monkeyWie it still doesn't work. The ui is broken

Can't build frontend?

UI is broken. It displays blank page.

monkeyWie · 2020-06-24T02:32:40Z

@monkeyWie it still doesn't work. The ui is broken

Can't build frontend?

UI is broken. It displays blank page.

I test no problem,can you rebuild frontend again?

o1egl

minor changes

o1egl · 2020-06-24T07:30:27Z

frontend/src/components/files/Preview.vue

+    thumbnail () {
+      if (this.req.type === 'image') {
+        return `${baseURL}/api/preview/big${this.req.path}?auth=${this.jwt}`
+      } else {


else is redundant

o1egl · 2020-06-24T07:31:25Z

frontend/src/components/files/Preview.vue

@@ -86,8 +86,15 @@ export default {
    download () {
      return `${baseURL}/api/raw${this.req.path}?auth=${this.jwt}`
    },
+    thumbnail () {


Suggested change

thumbnail () {

previewUrl() {

o1egl · 2020-06-24T07:36:43Z

http/raw.go

@@ -22,7 +22,7 @@ func parseQueryFiles(r *http.Request, f *files.FileInfo, _ *users.User) ([]strin
 		fileSlice = append(fileSlice, f.Path)
 	} else {
 		for _, name := range names {
-			name, err := url.QueryUnescape(strings.Replace(name, "+", "%2B", -1)) //nolint:shadow
+			name, err := url.QueryUnescape(strings.Replace(name, "+", "%2B", -1)) // nolint:shadow


this change is not correct. there should not be a space in nolint comment

o1egl · 2020-06-24T07:36:52Z

http/raw.go

@@ -35,7 +35,7 @@ func parseQueryFiles(r *http.Request, f *files.FileInfo, _ *users.User) ([]strin
 	return fileSlice, nil
 }

-//nolint: goconst
+// nolint: goconst


monkeyWie · 2020-06-24T15:08:56Z

@o1egl I think it's ok

o1egl requested changes Jun 9, 2020

View reviewed changes

o1egl requested changes Jun 10, 2020

View reviewed changes

monkeyWie force-pushed the feature/thumbnails branch from 89d525a to 9df4990 Compare June 14, 2020 13:34

o1egl requested changes Jun 15, 2020

View reviewed changes

monkeyWie force-pushed the feature/thumbnails branch from e398fbb to 4c5fb06 Compare June 22, 2020 14:18

o1egl reviewed Jun 22, 2020

View reviewed changes

monkeyWie added 4 commits June 24, 2020 09:18

refactor: preview handle router use path params

bee2ef7

refactor: revert front code format

fde88fe

refactor: fix go lint

3543eab

refactor: preview router use regex

07b2302

monkeyWie force-pushed the feature/thumbnails branch from d5bf839 to 07b2302 Compare June 24, 2020 01:19

monkeyWie added 2 commits June 24, 2020 09:46

fix: handle unsupported thumbnail

7b8b91e

refactor: fix golint check

223182c

o1egl requested changes Jun 24, 2020

View reviewed changes

refactor: fix nolint comment

81c412e

o1egl approved these changes Jun 25, 2020

View reviewed changes

o1egl merged commit 6b0d49b into filebrowser:master Jun 25, 2020

o1egl mentioned this pull request Jun 25, 2020

Video & Picture Thumbnails #978

Closed

	return http.StatusNotFound, nil
	return http.StatusNotImplemented, nil

	api.PathPrefix("/preview/thumb").Handler(monkey(previewThumbHandler, "/api/preview/thumb")).Methods("GET")
	api.PathPrefix("/preview/{size}").Handler(monkey(previewThumbHandler, "/api/preview")).Methods("GET")

	func previewFileHandler(w http.ResponseWriter, r http.Request, file files.FileInfo, size string) (int, error) {
	func imagePreviewHandler(w http.ResponseWriter, r http.Request, file files.FileInfo, size string) (int, error) {

Support thumbnail image view #980

Support thumbnail image view #980

Conversation

monkeyWie commented Jun 8, 2020

o1egl left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

monkeyWie Jun 10, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

monkeyWie commented Jun 10, 2020

o1egl Jun 10, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

o1egl left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

monkeyWie commented Jun 22, 2020

o1egl left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

monkeyWie commented Jun 23, 2020

o1egl commented Jun 23, 2020

monkeyWie commented Jun 24, 2020

o1egl left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

monkeyWie commented Jun 24, 2020

monkeyWie Jun 10, 2020 •

edited

Loading

o1egl Jun 10, 2020 •

edited

Loading