Cover image/thumbnail discussions #16
Comments
|
From #9 (review) regarding my findings about image size in the firmware:
The images are then scaled to fit within those bounds using Qt::SmoothTransformation (bilinear) and Note that device class does not directly correspond to the device with that codename; it is the first in it's series. |
|
From #3 (comment): Here are the results of the image resize library benchmark: It seems that rez is significantly faster on amd64 due to SIMD, imaging's nearestneighbour is the fastest in general, but rez's bilinear is quite close. Bilinear is slightly better quality though. |
|
It looks like to handle covers properly, we really need to save the full cover as well, according to the investigations @NiLuJe made. This was something I originally had, but canned due to performance concerns, With a faster library though, this could still probably be done. |
|
Do double-check that, as it may just be temporary insanity on the part of my device ;). |
|
If it turns out we do need the full cover, I understand the source is requested from Calibre? What happens if we fudge that and request a fullscreen-sized "thumbnail"? Only having to do downscaling might be a tad faster? EDIT: In which case, NearestNeighbor is not completely terrible if it's downscale only. It's what CRe had been using since basically forever in KOReader, and it took a random test on a high-dpi non-eInk screen to realize that it was less than great when upscaling and/or downscaling in some circumstances ;p. I'd obviously prefer something else from a quality standpoint, but, to be fair, on eInk, and for downscaling only, it's really not completely awful ;). |
That's what I was originally doing. But yes, we request a cover size from Calibre, and it delivers. Although I'm not certain what calibre does when the cover image available is smaller than we request... |
|
I'd imagine a QImage scale to honor the requested dimensions, so, smooth & fast, and on the server's side. Hopefully while honoring AR ;). |
I wish. It seems the cover dimensions that we send are treated as a maximum. I just checked. |
|
Gah. Well, that's not ideal... Nickel will scale it at display time,
though, so it's not a complete loss...
…On Tue, May 21, 2019, 06:31 Sherman Perry ***@***.***> wrote:
I'd imagine a QImage scale to honor the requested dimensions, so, smooth &
fast, and on the server's side. Hopefully while honoring AR ;).
I wish. It seems the cover dimensions that we send are treated as a
maximum. I just checked.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#16?email_source=notifications&email_token=AAA3KZRZMNDLVIKIM6TS7DTPWN3I5A5CNFSM4HOHEFPKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODV2W5CI#issuecomment-494235273>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAA3KZQIOTR5EHN5EWGSYXTPWN3I5ANCNFSM4HOHEFPA>
.
|
|
For the record, as it was mentioned in the pull request discussion, the cover image size in calibre KoboTouch driver comes from observation. Sideload a book and let the device generate the covers and check the sizes. I check them occasionally, but not for a few firmware versions, The one I do need to check is the Forma as I suspect I have the screen size rather than the image size. Also, be careful if you don't create the full screen cover but do create the others. The last time I checked, the firmware won't generate the full screen cover if the smaller covers exist. One of the smaller ones gets used and it looks terrible. Of course, the space saving might be considered a good thing. |
|
@davidfor : Thanks for the confirmation about the missing thumbnails, I knew this sounded familiar ;). Spoiler alert about the Forma: Nickel is doing it wrong, and generating (and downloading for store-bought KePubs) stuff that's KA1 sized, which is just plain wrong. |
|
@geek1011 : Okay, confirmed that, at least for downloaded thumbnails for KePubs, that's indeed def thumb_scale_qt(orig_dims, dims, expand=False):
w = orig_dims[0]
h = orig_dims[1]
scaled_width = dims[0]
scaled_height = dims[1]
rw = int(float(scaled_height) * float(w) / float(h) + 0.5)
if expand:
useHeight = (rw >= scaled_width)
else:
useHeight = (rw <= scaled_width)
if useHeight:
return (rw, scaled_height)
else:
return (scaled_width, int(float(scaled_width) * float(h) / float(w) + 0.5));
print
cover_dims = (1197, 1872)
print("FULL: {} -> {}".format(cover_dims, thumb_scale_qt(cover_dims, (1080, 1429), False)))
print("LIB_FULL: {} -> {} -> {}".format(cover_dims, thumb_scale_qt(cover_dims, (355, 530), False), thumb_scale_qt(cover_dims, (355, 530), True)))
print("GRID_FULL: {} -> {} -> {}".format(cover_dims, thumb_scale_qt(cover_dims, (149, 223), False), thumb_scale_qt(cover_dims, (149, 223), True)))Yields Which matches that 355x555 I mentioned earlier ;). I'm going to sideload that book manually to see if Nickel internally behaves differently when it's generating the thumbnails instead of downloading them, but, as we said before, I prefer this behavior anyway, so, it's basically just FOR SCIENCE§! :D. EDIT: i.e., While the on-device files are 355x473 & 149x198 ;). |
|
I got around to putting @geek1011 benchmark results (ARM) into spreadsheet to sort them, and convert to units I can better understand (milliseconds). First, I'm really glad I decided not to use Bild! Second, compared to Bild, the algorithm currently used is way faster. Still took over 5000 ms/op though, which is definitely unacceptable. Assuming the benchmark is representative, and not an outlier, I would be inclined to use nfnt/resize. In the grand scheme of things, it isn't that much slower than rez, and the API appears simpler. |
|
So, I ran the benchmark provided by @geek1011 on my H2O, with a random, relatively high resolution cover from my Calibre library. Note, I canceled my first benchmark run halfway through and restarted without benching Bild, cause I didn't want to spend all night benchmarking :p |
|
And here's the sorted results:
With a bigger and/or more complex image, the performance gap between rez and nfnt/resize definitely increases. Whatever library/algorithm gets chosen, I think we will need to generate |
|
Do you want to generate each image from the previous one? |
|
I'm afraid scaling LIB_GRID from LIB_FULL risks generating mush, no matter the algorithm ;). EDIT: Or not! See below :). |
|
To answer an earlier question from @shermp, I'm clearly not opposed to settings ;). In fact, having an option to disable thumbnail generation entirely could be desirable, in cases you want to let Nickel handle it, our you're using another tool to do it for you later anyway (KoboUtilities plugin, for instance). |
|
@shermp: Okay, that insane 41s does look familiar ;p. For fun, here's how long the FULL -> LIB_FULL scaling takes with a couple other things: FBInk (which, okay, does a rendering pass instead of an encoding pass ;p):
From my earlier experiments, much of that time is actually spent in the decoding pass, not the scaling or rendering one ;p. IM (with a classic
(Triangle should be Bilinear. This jumps to roughly (Displaying that with FBInk then takes IM should pretty much be considered a worst-case, as its goal is never speed ;). But from my various experiences using it on Kindle for the ScreenSaver hack, I rarely saw it spinning for more than 4 or 5s on a heavy screen-sized rescale, and that was w/ Lanczos + error-diffusion dithering. Most of the time it's in the 2s range. |
|
I may have been a bit harsh in my earlier comment ;). Scaling GRID from LIB_FULL is indeed noticeably faster, without being noticeably worse looking. For ref., with IM, Triangle: From Full (750ms): From Lib (235ms): |
|
The only quirk is potentially additional rounding errors because of the lack of rounding in |
That should be pretty easy to implement, if @shermp agrees.
That seems pretty useful. I'm going to add this when I get back to a proper computer. It'll probably be as simple as directly editing
Hey, I'm just copying Qt's implementation. 😈 Edit: Also, note that the target sizes could be all calculated from the original, then you only have the rounding errors from one resize. I'm going to test the quality of the different libs and algorithms with multiple resizes later today (or tomorrow). |
|
@geek1011 do you still want to run your experiments? Or should I just implement rez and call it a day? |
|
I won't be able to get to it today, so I'd probably suggest just switching to rez for now, and we can make more improvements later if necessary. |
|
Sounds good. Thanks for the update. |
|
We'll need values for the Libra. If @NiLuJe doesn't find them first, I'll do them later this week. |
|
Oh, that's easy, as we now only need the screen's resolution ;). And that's |
Confirmed: |
|
I decided to check the firmware for values again, and here is the first part (I'm putting them here for now for lack of a better place): I'll sort through these in a bit to put them into a usable form, and I'll probably add them to my koboutils repo (and KoboStuff whenever I get around to it). |
|
Here's what I have so far (everything to do with codenames, classnames, familynames, deviceids, and devicenames is done, I'll be adding specs and so on as I need them): By doing this, I think I finally completely understand how the hierarchy of Kobo devices works. @NiLuJe, you might find this interesting. |
|
Wow, that's a bit of a tangled web! |
|
Here's a nicer version, which I'm working on updating: https://gist.github.com/geek1011/613b34c23f026f7c39c50ee32f5e167e Update: and it turns out there are third-level codenames too, like frost32 and superDaylight |
* A lot more information (codenames, storage, etc) * Cover sizes * Cleaner code, easier to extend in the future * Codenames now match the internal layout (class -> family + optional secondary) * Based on firmware 4.18.13737 Also see https://gist.github.com/geek1011/613b34c23f026f7c39c50ee32f5e167e and shermp/Kobo-UNCaGED#16
|
Oh, and this will probably also interest @jackiew1, as the top-level device classes I found are used for the CSS too. |
Opening this to follow on discussions from #3 & #9 regarding cover images and thumbnails.
The text was updated successfully, but these errors were encountered: