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

Tags of type Short are read as Unsigned #204

Open
lgpasquale opened this issue May 19, 2023 · 3 comments · May be fixed by #235
Open

Tags of type Short are read as Unsigned #204

lgpasquale opened this issue May 19, 2023 · 3 comments · May be fixed by #235
Labels
help wanted Extra attention is needed

Comments

@lgpasquale
Copy link

When reading a tag that fits into the offset field and is of type SHORT (Tiff type 3) the value is returned as Value::Unsigned instead of Value::Short.

I believe the issue lies in line 407 of ifd.rs.

The right behavior is what can be seen when multiple values are handled (line 475 of ifd.rs)

@fintelia
Copy link
Contributor

fintelia commented May 29, 2023

This makes sense to fix, though the annoying part is figuring out whether anywhere else in the code is relying on the current behavior. Or whether it is likely to break any downstream users and thus might warrant additional caution

@fintelia fintelia added the help wanted Extra attention is needed label Jun 10, 2023
@Jondeen
Copy link

Jondeen commented Sep 6, 2023

It looks like a minimal fix is something like so.

This will make all the tests run successfully.

Regarding side effects I would suppose anyone interfering with the TAG data directly might have colliding types. What use case brought you to this, @lgpasquale ?

I suppose taking care of SSHORT is needed as well? This would require introducing an additional Value:

pub enum Value {
    Byte(u8),
    Short(u16),
    Signed(i32),
    SignedBig(i64),
    Unsigned(u32),
    UnsignedBig(u64),
    Float(f32),
    Double(f64),
    List(Vec<Value>),
    Rational(u32, u32),
    RationalBig(u64, u64),
    SRational(i32, i32),
    SRationalBig(i64, i64),
    Ascii(String),
    Ifd(u32),
    IfdBig(u64),
}

@weiji14
Copy link
Contributor

weiji14 commented May 17, 2024

Adding support for SSHORT (i16) and SBYTE (i8) at #234, I can try to add the patches for the casting of SHORT as u16 instead of u32 in a follow up PR if there's interest?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants