Hello Brian,
Thank you for the comments (hopefully I wasn't too confident picking the
issue).
Yes, from the "do not harm" position it seems better to remove all the
reduction-related fragments (leaving these worries to the user), but
keep the zero denominator checks + the sign checks for the unsigned
fractions.
One more silly question - if some upper bound (2^32 - 1) check should be
added for TIFF_RATIONAL's numerator/denominator? At a 1st glance, it
seems to be needless; on the other hand, it seems that the longs should
be used to store 32-bit unsigned integers(?). Not sure if it could cause
any side effects.
Thanks,
Alexander
On 11/15/2016 12:13 AM, Brian Burkhalter wrote:
Hello Alexander,
Thanks for picking up this issue. I actually worked on it a little last week as
well but did not yet finish a patch.
The TIFF 6.0 specification does not provide any guidance on the handling of
rational values in terms of the three questions at hand:
1 - Should the rational values be stored as given or with common factors
divided out?
2 - Should the displayed form of rational values be fractions as given (if
these are not reduced before being stored), fractions with common factors
divided out, or real values obtained by floating point division?
3 - Should providing a rational value with a zero denominator be an error?
Also, the libtiff tools ‘tiffdump’ and ‘tiffinfo’ seem to handle rational
values inconsistently. For example, for the images in the libtiffpic set of
sample images, XResolution and YResolution are reported to have a value such as
“72” instead of “72/1”, the values of ReferenceBlackWhite are displayed as-is,
but those of YCbCrCoefficients are divided out to real values.
My preference right now leans towards storing and displaying the fractions in
their initial, non-reduced state and throwing an exception if a zero denominator
is encountered (which would also cover the case 0/0). Note however that the zero
denominator check is not straightforward as it is possible to populate the
rational array after the TIFFField is created and this is actually done in the
TIFF ImageWriter. Because of this the zero denominator check might be infeasible.
Also note that removing the existing fractional reduction {k*q, k} —> {q, 1}
from getValueAsString() would require changing the API documentation hence a CCC
request. The zero denominator check if implemented would also require an API
update.
Thanks,
Brian
On Nov 14, 2016, at 7:02 AM, Alexander Stepanov
<alexander.v.stepa...@oracle.com> wrote:
P.S. please let me know if for some purposes the fractions should be stored in
the initial (non-reduced) state. at a 1st glance it is not required, but maybe
I'm wrong.
If this is the case, then only some part of the changes should probably be
applied:
1. remove needless reduction in getValueAsString()
2. check the sign for the unsigned fractions
3. forbid null denominators (?)
On 11/14/2016 5:30 PM, Alexander Stepanov wrote:
Hello,
Could you please review the following fix
http://cr.openjdk.java.net/~avstepan/8152293/webrev.00/
for
https://bugs.openjdk.java.net/browse/JDK-8152293 ?
Thanks,
Alexander
