Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Text color bug on subsampled formats #308

Closed
Reel-Deal opened this issue Jan 10, 2023 · 17 comments
Closed

Text color bug on subsampled formats #308

Reel-Deal opened this issue Jan 10, 2023 · 17 comments

Comments

@Reel-Deal
Copy link
Member

Hi there pinterf. Happy new years :)

I'm not sure if this is a bug or a limitation of the Text filter. But on subsampled formats the Text filter produces weird results. I know that it is a "quick and dirty" filter so maybe it was intended to be that way?

Here's how YV12/16/24 look:

text bug

And here's a little test script:

a1 = Blankclip(width=128, height=64, pixel_type="YV12", color=$FF0000).propShow().Text("TEST 1 2 3", align=2, text_color=$FFFFFF)
a2 = a1.ConvertToYV24()
b1 = Blankclip(width=128, height=64, pixel_type="YV16", color=$FF0000).propShow().Text("TEST 1 2 3", align=2, text_color=$FFFFFF)
b2 = b1.ConvertToYV24() 
c1 = Blankclip(width=128, height=64, pixel_type="YV24", color=$FF0000).propShow().Text("TEST 1 2 3", align=2, text_color=$FFFFFF)
c2 = c1.ConvertToYV12().ConvertToYV24() 
c3 = c1.ConvertToYV16().ConvertToYV24() 

Interleave(a2,b2,c1) # how it looks now (yv12, yv16, yv24)
#Interleave(a2,c2)    # how yv12 would look going from yv24->yv12
#Interleave(b2,c3)    # how yv16 would look going from yv24->yv16

PointResize(width*4, height*4)
@pinterf
Copy link

pinterf commented Jan 11, 2023

Hi! Happy New Year! I suppose, the Text filter does not try to align the chroma in any way. Thus chroma location of the Text is simply top_left (4:2:0) or left (4:2:2) I guess. I'm gonna look at it how it looks like if this placement is explicitely given to ConvertToYV24, or you can try it yourself until then.

@Reel-Deal
Copy link
Member Author

I think there's something else going with the chroma. In the script above the commented out lines show how it would look going from 444 to 42x. You can set the chroma placement to anything and it looks better than how it currently looks, although less saturated but that is expected. Here's how 420 looks:

(current | 444->420)
text bug

Looking at the chroma planes might be helpful in seeing what is happening.

@pinterf
Copy link

pinterf commented Jan 12, 2023

Thanks, just tried, I can see it now.

@Reel-Deal
Copy link
Member Author

Reel-Deal commented Jan 16, 2023

I think I found another bug. The Text filter does not render the "spleen-32x64.bdf" font correctly, but FreeSub does.

test 00

Here's the font: https://github.com/fcambus/spleen and here's the script:

c = Blankclip(width=128, height=64, pixel_type="RGB24", color=$FF0000).KillAudio()
f = "spleen-32x64.bdf"
a = Text(c, "TEST", font=f)
b = FreeSub(c, "TEST", font=f, text_color=$FFFF00, halo_color=0)
StackHorizontal(a,b)

Oddly, only the 32x64 font is not rendered correctly. Although, Text has trouble with the halo on smaller fonts:

test 00

c = Blankclip(width=64, height=32, pixel_type="RGB24", color=$FF0000).KillAudio()
f = "spleen-8x16.bdf"
a = Text(c, "TEST", font=f, align=8)
b = FreeSub(a, "TEST", font=f, text_color=$FFFF00, halo_color=0, align=8) # Freesub's align is backwards
b.PointResize(c.width*4,c.height*4)

@pinterf
Copy link

pinterf commented Jan 16, 2023

Halo is not expanded to outside of the actual font matrix area. E.g. if a the leftmost pixel is lit, then there won't be left-side halo.
Back to the original problem: the actual background is not used when rendering the chroma pixels of either the actual font or its halo. Font is not overlaid. For a 4:2:0 format a single chroma value is established based on a 2x2 pixel area. We have to find out what color should we set. Current rule: if there is any main pixel then the whole subsampled color value is set to the font's color. Otherwise: if there is any halo pixels there, then the whole subsampled color value is set to the halo's color value.

In an ideal world where the rendering would use overlay, the final color is computed by the weighted values of the 1.) existing background color 2.) count of font pixels 3.) count of halo pixels. When a 2x2 pixel area (covering one chroma pixel) has 1 font pixel, 2 halo pixels, then the resulting color must be computed as (1FontColor + 2HaloColor + 1*ExistingBackgroundColor) / 4.

Present method (no weight, use font color or use halo color or use nothing) can have further visual problems when the text is positioned on an odd X or Y coordinate, which makes it a bit more difficult to properly find out the chroma values on the boundaries of the characters.

I'm not sure I can program it easily :)

@pinterf
Copy link

pinterf commented Jan 16, 2023

Anyway, I've done that for curiosity and YV12 now looks good, even better than the test sequence YV24->YV12, probably needs some optimizations before going live. And there is still the cosmetic problem when the character is positioned on odd coordinate vertically (bottom-aligned "TEST" works like that probably).

Note: the present handling of text chroma is "center", whatever did I write ("top_left") earlier. For YV24 conversion tests "center" must be given, because "mpeg2" ("left") is the default for that.

@pinterf
Copy link

pinterf commented Jan 16, 2023

I think I found another bug. The Text filter does not render the "spleen-32x64.bdf" font correctly, but FreeSub does.

Width of 32? I'm quite sure that the maximum supported font pixel width is 16, because internally I'm using a 32bit wide variable to hold two neighboring character layout and this 32 bits can hold only up to two 16 bits wide characters. As for internal storage, I don't know it either if the horizontal 4 bytes (32 pixels) size option is supported or not.
EDIT: each character matrix row can contain only 16 pixels; technically the font is an array of uint16_t elements.

@Reel-Deal
Copy link
Member Author

Anyway, I've done that for curiosity and YV12 now looks good, even better than the test sequence YV24->YV12...

I'm sure it will look better than a dark blob around each character. And you were doubting yourself :)

Width of 32?

I was not aware that there was a limit. I assumed since FreeSub renders large fonts correctly there was something wrong with Text. I'll make a note in the documentation that widths >16 are not supported.

Halo is not expanded to outside of the actual font matrix area.

Thanks for the info. I'll see if I can somehow add the missing halo in a script. I was trying to modify the ng_bighalo.avs script to use Text but I don't think I can do it easily since FreeSub has other features like font_width, halo_width, halo_height and can also scale the font size with PointResize.

@pinterf
Copy link

pinterf commented Jan 17, 2023

Aside from working a bit like overlay, there are still two issues that have to be done:

Correct chroma rendering for 4:2:0 (vertically subsampled), when starting Y coordinate is odd. In this case the character matrix is unfortunately not properly aligned vertically to the chroma boundaries so we have to trick to realign it.
(I've already done this one today).

The other cosmetic issue is the same phenomenon as above, but for odd X coordinates (4:2:0, 4:2:2). (and similarly for the 4:1:1 when I have still motivation for that one :) ) This is under construction.

@Reel-Deal
Copy link
Member Author

Thanks for the info. I'm sure 4:1:1 is not very thrilling, considering it is probably the least popular colorspace :)

@pinterf
Copy link

pinterf commented Jan 18, 2023

Anyway, I'm just fighting with that format :) The other tasks are already done. I'm gonna edit the documentation as well because now it is available to have halo and background fading at the same time.

pinterf added a commit that referenced this issue Jan 18, 2023
@pinterf
Copy link

pinterf commented Jan 18, 2023

@Reel-Deal
Copy link
Member Author

Nice, it definitely looks much better! Thank you. I'll test it out this evening and will let you know if I find something odd.

@Reel-Deal
Copy link
Member Author

All seems to be good.

@pinterf
Copy link

pinterf commented Jan 19, 2023

Cuold you check my rst modification? The result is not what I expected (to tell the truth, I did not succeed to build it with Sphynx bacause it is missing on my new PC and forgot what to do to have it)
I thought it matched with the sorrounding lines but both the "Text" halo_color section and the Changelog at the end went wrong.
Thanks

@Reel-Deal
Copy link
Member Author

I did already, and I fixed it also :) ... I'll do a PR soon, I have other cosmetics and typos to fix on a few pages.

@pinterf
Copy link

pinterf commented Jan 19, 2023

You can open an issue for supporting up to 32 pixel wide bdf fonts, if you'd like to separate this new (and almost working :) ) feature from this one...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants