Thumbnail generation

[pgaskin]

Reworks the thumbnail generation to be faster and better match Kobo's behavior.

See #16 and #9.

[pgaskin]

Which image resize library/algorithm are we planning to use?

[shermp]

We may have to upscale the full cover, so an algorithm that's only decent for downscaling probably isn't the best idea.

Rez seemed pretty quick, so I would be happy with that. Bilinear or Bicubic perhaps. Or we could make the algorithm selectable in the config file, and the quality purists among us can select a better algorithm if they want to take the performance hit.

[pgaskin]

This is ready for testing (and choosing the resize library/algorithm).

[shermp]

Just added a small update to request the full size cover from Calibre, so we aren't upscaling a tiny thumbnail!

[shermp]

Nickel still appears to be generating its own thumbnails for me. It is very obvious when sending larger numbers of books at a time (eg: > 5 books). It may appear to work when sending one or two books, but I don't think it actually is.

I'm going out in a few minutes, but I'll try and do some more investigation when I get back later.

[NiLuJe]

Yep, confirmed. I'm not getting anything (thumbnails-wise) getting generated by KU.

It's easy enough to check by SSH'ing into the device while it's still connected to Calibre.
With a find /mnt/newonboard/.kobo-images/ -name '*BLAH*', with BLAH being a relevant bit of the book's filename, I used to see the small thumbnails freshly created ;).

Now, zilch. And Nickel indeed generates all three in one go the first time you open the Library (or earlier if one pops up at the front of one of the Home tiles, generally, My Books, which will definitely happen for one book if it's fresh).

[pgaskin]

Ok, I'll test this myself once I get Calibre and my old Kobo Mini set up (probably tomorrow). I've been working on this blindly so far.

Is there anything in the logs?

Update: Oh, I think I know why. I forgot the /mnt/onboard in the path! facepalm It creates the .kobo-images in the current dir now.

[pgaskin]

@NiLuJe @shermp I think it should work fine now.

[NiLuJe]

Oh, right, found a few of 'em in /tmp/ku/.kobo-images :D.

The set isn't complete, though, sometimes I have a LIBRARY_FULL, sometimes both LIBRARY_FULL and LIBRARY_GRID...

Will check again w/ the latest commit ;).

[NiLuJe]

Okay, yeah, I'm only ever getting KU to generate LIBRARY_FULL & LIBRARY_GRID, and it takes quite a bit of time to, without any specific on-screen feedback (I had to check top to confirm that it was pegged at 100% CPU after being puzzled at not seeing them pop-up immediately in my find, but only a few dozen seconds later ;)).

[NiLuJe]

I'm also getting slightly wonky thumbnail sizes:

Given a Calibre cover.jpg @ 1391x2200, I'd expect GRID to be 149x236, but it's 149x352; and LIB_FULL to be 355x561, but it's 355x838

This is possibly a side-effect of what Calibre sends as a source?

[shermp]

Ok, can see why the full cover isn't been created. Just need to add it to the array at for _, cover := range []koboCover{libFull, libGrid}

Not sure about the thumbnail sizes, but it looks to me like the wrong axis is being scaled. Calibre sends a base64 string, and a separate width and height.

[pgaskin]

I'm only ever getting KU to generate LIBRARY_FULL & LIBRARY_GRID

That's all it's currently configured to generate. Should I add FULL to the list of images to generate?

it takes quite a bit of time to scale 'em

The algorithm has not been changed yet.

I'm also getting slightly wonky thumbnail sizes ... This is possibly a side-effect of what Calibre sends as a source?

Possibly. Can you check what Qt does with them?

[shermp]

That's all it's currently configured to generate. Should I add FULL to the list of images to generate?

Yes, because as @NiLuJe discovered, and also confirmed by @davidfor we need all three otherwise Nickel will scale the tiny thumbnail

[NiLuJe]

The numbers I pulled were from my quick'n dirty Python port of Qt's QSize:scaled, but based on the original cover resolution, which is potentially not what's actually happening here ;).

[NiLuJe]

Yep, after a bit of extra logging, resizeKeepAspectRatioByExpanding is doing something wrong.

Note that the full cover should also not be using Qt::KeepAspectRatioByExpanding, but Qt::KeepAspectRatio instead ;).

[pgaskin]

Yep, after a bit of extra logging, resizeKeepAspectRatioByExpanding is doing something wrong.

Could you provide some test data for it?

Note that the full cover should also not be using Qt::KeepAspectRatioByExpanding, but Qt::KeepAspectRatio instead ;).

Good catch. I'll implement that next.

[shermp]

On the full cover, I just had a thought that I might make generating it a configurable option.

Point of that is, there are probably others, like me, who disable showing the cover on the sleep screen, so the full cover isn't used (unless it's used else where that I don't know of). Not using the full cover means requesting a smaller image from Calibre, which means a smaller transfer, and quicker downscaling.

Thoughts?

[NiLuJe]

If Point is an int, doesn't the division need to explicitly cast to a float so we get anything useful out of it, instead of 1 or 0?

[pgaskin]

@NiLuJe Yes. TBH, I haven't actually got my head completely wrapped around the logic behind the Qt equations (and now I realize that my unit tests didn't include fractional ratios).

[NiLuJe]

@geek1011 : This simple change made it easier to grok on my end, but that may be fairly subjective ^^:

diff --git a/kobo-uncaged/util.go b/kobo-uncaged/util.go
index 6f9d37a..ddbc894 100644
--- a/kobo-uncaged/util.go
+++ b/kobo-uncaged/util.go
@@ -79,7 +79,8 @@ func resizeKeepAspectRatio(sz image.Point, bounds image.Point, expand bool) imag
        }
 
        var useHeight bool
-       rw := int(float64(bounds.Y) * float64(sz.X) / float64(sz.Y))
+       ar := float64(sz.X) / float64(sz.Y)
+       rw := int(float64(bounds.Y) * ar)
 
        if !expand {
                useHeight = rw <= bounds.X
@@ -90,7 +91,7 @@ func resizeKeepAspectRatio(sz image.Point, bounds image.Point, expand bool) imag
        if useHeight {
                return image.Pt(rw, bounds.Y)
        }
-       return image.Pt(bounds.X, int(float64(bounds.X)*float64(sz.Y)/float64(sz.X)))
+       return image.Pt(bounds.X, int(float64(bounds.X) / ar))
 }
 
 // hashedImageParts returns the parts needed for constructing the path to the

EDIT:
Sidebar: not sure we really need double precision here, float32 would probably be enough ;). Probably won't change much performance wise, though, so, eh.

[NiLuJe]

As for my bit of logging:

diff --git a/kobo-uncaged/main.go b/kobo-uncaged/main.go
index 365ab7e..05c4563 100644
--- a/kobo-uncaged/main.go
+++ b/kobo-uncaged/main.go
@@ -498,6 +498,7 @@ func (ku *KoboUncaged) saveCoverImage(contentID string, size image.Point, imgB64
        for _, cover := range []koboCover{fullCover, libFull, libGrid} {
                nsz := cover.Resize(ku.device, sz)
                nimg := imaging.Resize(img, nsz.X, nsz.Y, imaging.Linear)
+               log.Printf("Resized %dx%d cover to %dx%d -> %dx%d => %dx%d\n", sz.X, sz.Y, cover.Size(ku.device).X, cover.Size(ku.device).Y, nsz.X, nsz.Y, nimg.Bounds().Size().X, nimg.Bounds().Size().Y)
                nfn := filepath.Join(imgDir, cover.RelPath(imgID))
 
                if err := os.MkdirAll(filepath.Dir(nfn), 0755); err != nil {

And success:

May 22 05:36:37 KoboUNCaGED[2769]: 2019/05/22 05:36:37 Resized 903x1429 cover to 1080x1429 -> 903x1429 => 903x1429
May 22 05:37:05 KoboUNCaGED[2769]: 2019/05/22 05:37:05 Resized 903x1429 cover to 355x530 -> 355x561 => 355x561
May 22 05:37:23 KoboUNCaGED[2769]: 2019/05/22 05:37:23 Resized 903x1429 cover to 149x223 -> 149x235 => 149x235

Well, almost, because we could essentially skip the scaling pass for the full cover if sz == nsz, like in that example ;).

That should hopefully be the most common case with sane Calibre libraries, and would alleviate some of @shermp's concerns ;).

[shermp]

Fixed an issue with the WaitGroup. It actually does what it was supposed to now!

I REALLY wouldn't recommend sending large numbers of books at the moment. It is slooooow. Stick to three max.

[shermp]

@NiLuJe , @geek1011 if you are happy with this PR, I'm happy to merge it, and we can move on to performance optimizations.

[shermp]

I'll take the thumbs up as confirmation :)

Thank you both very much.