search

po5 / thumbfast

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

Black border on the right side of 16:9 content #2

Closed christoph-heinrich closed 2 years ago

christoph-heinrich commented 2 years ago

Happens on 16:9 content, but not on 4:3. I didn't test any other aspect ratios. Screenshot_20220918_220951

christoph-heinrich commented 2 years ago

It's still there on that commit and also on the latest one. Btw that was a local file, but it also happens on streams.

Edit: does not happen on a 2:1 stream (https://www.youtube.com/watch?v=pfM-JUoaOHI)

hooke007 commented 2 years ago

It looks like a filter's crop/pad logic. For ????x1080(width should be longer) source, set thumb max height to 270 would be fine(no black pad).

Zabooby commented 2 years ago

On 1980x1080 and experiencing the same issue. Tried changing max height to 270 but it didn't fix it.

hooke007 commented 2 years ago

I'm not sure if it should be a issue because mpv handle the pad in the same logic.

Snipaste_2022-09-21_14-15-00

christoph-heinrich commented 2 years ago

Then the calculated output resolution needs to take that behavior of mpv into account to not end up with padding on the side. Maybe there is a configuration to tell mpv to not do that (I have never noticed that on any of my windows, so I'll check my config)

Edit: does happen with my config, I just never noticed.

hooke007 commented 2 years ago

Ah I saw the pad here. It looks like intended. https://github.com/po5/thumbfast/blob/335cbd0ae9a48b4255e72a6aab25d3b96d8ccbcd/thumbfast.lua#L266

edit:

            "--vf="..vf_string(filters_all).."scale=w="..effective_w..":h="..effective_h..",format=bgra",

LGTM

po5 commented 2 years ago

We add padding if necessary because media files are big liars, the resolution may change in the middle of the file (see discord resize memes).
Since the output is raw bgra, we can't just guess what resolution it really spat out from filesize, so instead we force the output to be a known expected resolution.
Something must be slightly wrong about the x=(ow-iw)/2:y=(oh-ih)/2 formula (this should do padding that keeps the image centered, if padding is needed), I think I introduced the issue when making thumbnails mod2 resolutions because some videos would break.

christoph-heinrich commented 2 years ago

Is it possible to output to something that contains resolution information instead of just bgra? I've tried mpv --of=help but it doesn't output anything for me.

tomasklaen commented 2 years ago

Afaik you can only use bgra as an overlay image: https://mpv.io/manual/master/#command-interface-overlay-add

po5 commented 2 years ago

Fat fingered the close issue button (:

mpv only accepts bgra, and we don't have much in the way of cross-process communication, if we want it to work on Windows out of the box it has to be one-way only as it is now.