VSFilter: misc colour changes #23
Conversation
TheOneric
force-pushed
the
vsfilter_fix_colours
branch
from
6ee2bd3
to
945757c
Compare
|
New version:
|
|
Copying Cyberbeing’s comment from another PR for future reference:
|
TheOneric
force-pushed
the
vsfilter_fix_colours
branch
2 times, most recently
from
a32bf09
to
d2fb683
Compare
|
One thing to mention when making changing to the color handling code, is to be careful to ensure BT.601 continues to always be used on |
|
btw, how about merging the first two or only the first commit(s) early? The first one only changes whitespaces to make line endings consistent in each file and should thus be totally safe. Since it touches many files, leaving it unmerged makes future merge conflicts likely. The second commit seems like a strict improvement to me, since before |
Some source files had mixed CRLF and LF line endings which can be
bothersome. To fix this convert all files with at least one preexisting
CRLF line ending to full CRLF:
find src/ -type f \( -name "*.h" -o -name "*.c" -o -name "*.cpp" \) -execdir \
bash -c 'cmp <(dos2unix < "$1" 2>/dev/null) "$1" 1>/dev/null || unix2dos "$1"' _ '{}' \;
To confirm nothing else changed use
git diff --ignore-space-change HEAD~
or
git diff --ignore-cr-at-eol HEAD~
An application is supposed to first configure a supported format via csri_request_fmt(..) for an instance and then pass frames in the previously requested format to csri_render(..). This setup logic was broken, when support for non BGR_ formats was dropped in 50d3b3b and applications can no longer detect whether a format is supported ahead of time and will instead crash when trying to render using any non BGR_ frame. Supposedly the non-BGR formats already didn't work properly before their removal. And since xy-VSFilter ungracefully bailed out on those for 10½ years now, it is highly unlikely anyone is actually (still) using them anyway. So just properly signal them as unsupported, so format probing is possible again. (Aegisub exclusively uses BGR_ since 2010, predating the removal in xy-VSFilter. See Aegisub SVN r5079 or 824294078f23cb77e4a46d9b927421f7888002e2)
Commit e928f83 allowed colorimetry information to be received from the DirectShow filter chain, so we don’t need to rely on resolution-based guesses as much. However, since it only checked for BT709 (and assigned a suboptimal fallback for SMPTE240) but not BT601, a known-to-be BT601 matrix was still overriden by the fallback guess for larger video resolutions.
xy-VSFilter already works with an alpha channel for BGR_ since 6a6f7cb, so we just need to accept an additional format. Treating the padding byte of BGR_ as alpha may of course create its own problem. But given - the change to RGBA supposedly fixed some SSF bug - this behaviour existed since 2014 - I don’t know who besides Aegisub uses the csri interface I’m wary of changing this now. Regarding the naming mismatch BGR* and RGB*: csri has two distinct types for RGB* and BGR* with the name signifying the order of colour channels. Of those, guliverkli2 and xy-VSFilter always only supported the BGR* variants and mapped them to their internal RGB* types. This is most likely just a naming mismatch and *VSFilter’s RGB* types actually use BGR order. A look at the YUV to "RGB" conversion from the Aegisub commit mentioned in the previous commit corrobarates this. Even if not, this already dating back to guliverkli2 probably means every modern csri application already expects and relies on this.
TheOneric
force-pushed
the
vsfilter_fix_colours
branch
from
d2fb683
to
c844868
Compare
Not actually tested yet, but putting this up so it’s less likely to be forgotten about.
I won't have time to work on and test this for a while; if someone else wants they’re welcome to pick this up.
@astiob: is the change in the 3rd commit what you had in mind here?
The idea behind officially supporting BGRA is that it could be used for colour-accurate rendering with CSRI by allowing the CSRI user to perform colour mangling like they’d do with libass. Without an alpha channel this isn't possible without degrading the video’s colours. SInce users can probe supported formats before rendering starts a fallback can be used and a warning emitted when no alpha channel and thus no colour accurate rendering is supported.
I think aegisub already uses/expects the alpha channel to have ASS semantics where 0xFF is fully transparent and 0x00 fully opaque, but this should be rechecked and it might also be good to check what alpha semantics asa’s CSRI interface used/expected.