po5 / thumbfast

High-performance on-the-fly thumbnailer script for mpv
Mozilla Public License 2.0
760 stars 34 forks source link

Fix black borders, again #28

Closed natural-harmonia-gropius closed 1 year ago

natural-harmonia-gropius commented 1 year ago

Not sure is this will work very well. For now keep the step to 2, someone actually encounter issues and then change it to 4.

You can test this with vf=lavfi-crop=w=1920:h=802:x=0:y=0

christoph-heinrich commented 1 year ago

I get a black bar of 1px at the bottom on with max thumbnail size of 201. black_bottom

natural-harmonia-gropius commented 1 year ago

I'll look at it tomorrow.

christoph-heinrich commented 1 year ago

I think black bars are unavoidable with our current approach, the only thing we can avoid is crashes due to wrong resolution. I try something different in https://github.com/po5/thumbfast/issues/27#issuecomment-1257024128, we'll see if it holds up to testing :smile:

natural-harmonia-gropius commented 1 year ago

1920x802 to 300x126 = 300x125 + 1px border 1920x802 to 300x125 = crash

WOW

image image

natural-harmonia-gropius commented 1 year ago

I can only manage to align one of the edges. No idea what to do with the extra 1px.

Memo

For example for the pixel format "yuv422p" hsub is 2 and vsub is 1.

Other solutions I can think of:

  1. Give up on keep aspect ratio. Those meme are too rare.
  2. Replace decrease with increase and pad with crop. Probably the performance cost is high.
  3. thumbfast are exporting bgra, do something with the alpha channel. But not a real solution. I tried "color:#00000000" with "--alpha=yes", doesn't work.
christoph-heinrich commented 1 year ago

1920x802 to 300x126 = 300x125 + 1px border 1920x802 to 300x125 = crash

That is because the output resolution changes, look https://github.com/po5/thumbfast/pull/29#issuecomment-1257083021 for details

natural-harmonia-gropius commented 1 year ago

output resolution changes

Where can I observe changes? pad should ensure that the size is constant ...... image image

168x299 works, 300x125 doesn't. 🙃

christoph-heinrich commented 1 year ago

Where can I observe changes?

In the file size of the generated thumbnail. It decreases, and so the check for equality with thumb_size doesn't work anymore and nothing gets drawn because it enters the if here.

natural-harmonia-gropius commented 1 year ago

I see, why did the pad not work as expected?

christoph-heinrich commented 1 year ago

I don't know.

natural-harmonia-gropius commented 1 year ago

We can't do it perfectly right now, 1 pixel on the bottom edge is the best I can do. If nothing goes wrong after testing, then I think this is a good short-term solution. This PR makes minimal changes, waiting for someone to come up with a better solution.

christoph-heinrich commented 1 year ago

That iterative approach looks interesting, mind explaining the reasoning behind it please?

natural-harmonia-gropius commented 1 year ago

In https://github.com/po5/thumbfast/issues/27#issuecomment-1257005088 I noticed exact size (320x172.666) larger than rounded "effective" size (320x172). When vf=scale get effective size, scaler will results smaller size (318x172), 172 / 1036 (original height) * 1920 = 318.7644787644788. It matches the 318x172 I measured with the paint.

I might not have explained it well enough, but here's the general idea.

christoph-heinrich commented 1 year ago

After testing I've gotten both 1 and 2 pixel wide borders at the right side and the bottom (usually not both at the same time). Don't know how that's even possible, because pad should center it when there is more then one pixel padding shrug.

I've tested with max thumbnail size set to 50.

Example (test file is 848x464): Screenshot_20220925_175822 Note how there is 2px border at the right side, and also 1px border at the bottom.

natural-harmonia-gropius commented 1 year ago

Strange things ...... image

Use this patch to print.

diff --git a/thumbfast.lua b/thumbfast.lua
index 7162e27..cc489c9 100644
--- a/thumbfast.lua
+++ b/thumbfast.lua
@@ -197,6 +197,14 @@ local function calc_dimensions()
     effective_w = diff_w ~=0 and pass2_w - 2 or pass2_w
     effective_h = pass2_h

+    print(
+        "exact", pass1_w, pass1_h, "\n",
+        "expected", pass2_w, pass2_h, "\n",
+        "estimated", pass3_w, pass3_h, "\n",
+        "effective", effective_w, effective_h, "\n",
+        "======"
+    )
christoph-heinrich commented 1 year ago

Screenshot_20220925_181225

If we manage to make a resolution calculation that always results in 0 or 1 pixel border at the bottom, with no border anywhere else, and also have a way of predicting if there will be a border or not, we'll be able to cut off the bottom line when necessary, resulting in no borders.

I think that should wait until #31 lands, because then we don't have the problem of no thumbnail appearing when the output resolution does not match the expected one.

Edit: I just realized that we can also cut off lines from the top with the offset parameter, so as long as we can reliably prevent horizontal padding and calculate how much vertical padding there is, we should always be able to get rid of black borders.

natural-harmonia-gropius commented 1 year ago

close by https://github.com/po5/thumbfast/commit/0c9788ce33509eea939d8c11d94161f9f31b6f34