pinterf / masktools

MaskTools v2 fork
Other
48 stars 11 forks source link

new yfmin, yfmax, cfmin and cfmax keywords #19

Closed realfinder closed 3 years ago

realfinder commented 3 years ago

same as https://github.com/pinterf/masktools/pull/18 but real changes only

realfinder commented 3 years ago

@pinterf ok now?

pinterf commented 3 years ago

Yeah, much better :), now I see. I could check only on mobile, I'm not near my PC this weekend.

pinterf commented 3 years ago

I do not understand how these constants were generated? Are they just experimental workaround constants that - under specific circumstances and script - will give expected results for a single use case? They are very ugly. If so, I think these must not appear in the core, there has to be a generic and reasonable solution instead. These are neither intuitive, no one will use it even if it is documented. In your code I can see 16/64/257/1028/4112 as a minimum value for 8/10/12/14/16 bits, And for max 235/1175/3995/15275/60395.

realfinder commented 3 years ago

they are from https://forum.doom9.org/showpost.php?p=1945318&postcount=1096

pinterf commented 3 years ago

they are from https://forum.doom9.org/showpost.php?p=1945318&postcount=1096

Not really. Min values are different. They must keep the rule of 162^(N-8) where N is the bit depth. As for the others I can imagine that they are (235+1)2^(N-8) - 1 but it must be checked again.

pinterf commented 3 years ago

My followups in https://forum.doom9.org/showthread.php?p=1945613#post1945613 and in https://forum.doom9.org/showthread.php?p=1945617#post1945617

Adjusting with a different yminf to get a specific result in a specific script is surely wrong decision I think.

realfinder commented 3 years ago

lets see what Dogway said then, I did these changes for his case

pinterf commented 3 years ago

Yes I know, anyway, thanks for being so quick.

realfinder commented 3 years ago

ok, I think will close this then