Skip to content

Conversation

@ze0987
Copy link
Contributor

@ze0987 ze0987 commented Mar 31, 2025

Currently, Yazi only scales thumbnails to preview.max_height, which means that thumbnails of videos in landscape orientation are unnecessarily large.

Example:

max_width       = 600
max_height      = 900
image_quality   = 75

Before

File size: 131 kB
Image size: 1600x900
On the left is a preview in Yazi, and on the right is a generated thumbnail.
before

After

File size: 28 kB
Image size: 600x338
after

@sxyazi
Copy link
Owner

sxyazi commented Apr 1, 2025

Thanks for the PR!

Could you provide some benchmarks to see how this change affects performance — separately when the video size is under max_width/max_height and when it reaches them?

@ze0987
Copy link
Contributor Author

ze0987 commented Apr 1, 2025

Sure.

max_width       = 600
max_height      = 900
image_quality   = 75

Video resolution 320x180, software

Before

  Time (mean ± σ):     253.2 ms ±   1.7 ms    [User: 194.5 ms, System: 53.8 ms]
  Range (min … max):   250.8 ms … 255.5 ms    10 runs

After

  Time (mean ± σ):     254.1 ms ±   1.6 ms    [User: 194.9 ms, System: 54.5 ms]
  Range (min … max):   251.3 ms … 256.3 ms    10 runs

Video resolution 320x180, vaapi

Before

  Time (mean ± σ):     272.6 ms ±   2.4 ms    [User: 198.1 ms, System: 68.9 ms]
  Range (min … max):   269.8 ms … 277.5 ms    10 runs

After

  Time (mean ± σ):     278.0 ms ±   2.7 ms    [User: 202.2 ms, System: 70.4 ms]
  Range (min … max):   273.3 ms … 282.8 ms    10 runs

Video resolution 1920x1080, software

Before

  Time (mean ± σ):     884.0 ms ±   2.7 ms    [User: 778.3 ms, System: 95.8 ms]
  Range (min … max):   879.6 ms … 888.7 ms    10 runs

After

  Time (mean ± σ):     843.4 ms ±   2.9 ms    [User: 744.5 ms, System: 89.7 ms]
  Range (min … max):   838.7 ms … 846.9 ms    10 runs

Video resolution 1920x1080, vaapi

Before

  Time (mean ± σ):     448.4 ms ±   3.7 ms    [User: 314.1 ms, System: 126.7 ms]
  Range (min … max):   442.9 ms … 453.8 ms    10 runs

After

  Time (mean ± σ):     388.1 ms ±   2.9 ms    [User: 268.0 ms, System: 113.2 ms]
  Range (min … max):   383.8 ms … 392.5 ms    10 runs

With this PR, thumbnails of videos smaller than preview.max_width x preview.max_height will be upscaled to preview.max_width x preview.max_height, which may or may not be a good thing.

@sxyazi sxyazi force-pushed the main branch 5 times, most recently from a9c693e to 86f4659 Compare April 8, 2025 09:06
@maga-tama
Copy link

This one won't upscale https://superuser.com/a/794924
"-filter_complex", string.format("scale=iw*min(1\,min(%d/iw\, %d/ih)):-1:flags=fast_bilinear", rt.preview.max_width, rt.preview.max_height)

@ze0987
Copy link
Contributor Author

ze0987 commented Apr 10, 2025

Here is another variant:

"-vf", string.format("scale='min(%d,iw)':'min(%d,ih)':force_original_aspect_ratio=decrease:flags=fast_bilinear", rt.preview.max_width , rt.preview.max_height),

But the question remains, upscale or not? Both approaches have their pros and cons. Or maybe give the users a switch and let them decide. The performance difference is small.

max_width       = 600
max_height      = 900
image_quality   = 75

Single Intel E-core @ 800 MHz

2160p vaapi

Test Mean [s] Min [s] Max [s] Relative
main 3.292 ± 0.005 3.284 3.298 1.03 ± 0.00
upscale 3.197 ± 0.009 3.181 3.208 1.00 ± 0.00
scale='min(%d,iw)':'min(%d,ih) 3.195 ± 0.004 3.189 3.199 1.00
scale=iw*min(1\,min(%d/iw\, %d/ih)) 3.198 ± 0.006 3.186 3.205 1.00 ± 0.00

180p vaapi

Test Mean [ms] Min [ms] Max [ms] Relative
main 276.5 ± 2.6 272.8 280.2 1.00
upscale 283.6 ± 3.3 278.1 287.5 1.03 ± 0.02
scale='min(%d,iw)':'min(%d,ih) 276.6 ± 3.2 270.2 281.4 1.00 ± 0.01
scale=iw*min(1\,min(%d/iw\, %d/ih)) 278.6 ± 2.5 275.5 283.7 1.01 ± 0.01

@maga-tama
Copy link

Or maybe give the users a switch and let them decide.

Feels like the right call.
Even though it's just for video, I can still think of cases it'll get real old real fast -
short clips/reactions/memes (or just saving as videos instead of gifs in general)

@sxyazi
Copy link
Owner

sxyazi commented Apr 11, 2025

Thanks for the benchmarks, it's really helpful!

I decided to use scale='min(%d,iw)':'min(%d,ih)' to keep the old behavior here

@sxyazi sxyazi changed the title fix: respect preview.max_width for video thumbnails fix: respect the user's max_width setting for the built-in video previewer Apr 11, 2025
Copy link
Owner

@sxyazi sxyazi left a comment

Choose a reason for hiding this comment

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

Thank you!

@sxyazi sxyazi merged commit bef4810 into sxyazi:main Apr 11, 2025
6 checks passed
@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 12, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants