Exceptions when trying to set value of type UFraction32

[victornor]

Hey, thank you so much for the work on this library! I'm having some issues trying to write exif values with UFraction32 type.

I have loaded two images var sourceImageFile = ImageFile.FromFile(sourceImage); var sourceExifImageFile = ImageFile.FromFile(sourceExifImage);

And i'm trying to copy properties from one to the other. String and ushort types are working great, but when i get to properties of type MathEx.UFraction32, i get an ArgumentException.

This is the code i'm trying to execute: sourceImageFile.Properties.Set(ExifTag.ExposureTime, (MathEx.UFraction32) sourceExifImageFile.Properties.Get(ExifTag.ExposureTime).Value);

And it throws the following exception: System.ArgumentException: GenericArguments[0], 'ExifLibrary.MathEx+UFraction32', on 'ExifLibrary.ExifEnumProperty1[T]' violates the constraint of type 'T'. ---> System.TypeLoadException: GenericArguments[0], 'ExifLibrary.MathEx+UFraction32', on 'ExifLibrary.ExifEnumProperty1[T]' violates the constraint of type parameter 'T'. at System.RuntimeTypeHandle.Instantiate(Type[] inst) at System.RuntimeType.MakeGenericType(Type[] instantiation) --- End of inner exception stack trace --- at System.RuntimeType.ValidateGenericArguments(MemberInfo definition, RuntimeType[] genericArguments, Exception e) at System.RuntimeType.MakeGenericType(Type[] instantiation) at ExifLibrary.ExifPropertyCollection`1.Set(ExifTag key, Object value) in ExifLibNet/ExifLibrary/ExifPropertyCollection.cs:line 333

Any thoughts on this would be greatly appreciated!

[ziriax]

That likely is caused by suggestion #73 of mine...

Could you try using https://www.nuget.org/packages/ExifLibNet/2.1.1 and see if the problem still occurs?

[ziriax]

Actually, it seems the Set overload that accepts an object type, assumes the value must be an enum...

https://github.com/oozcitak/exiflibrary/blob/4e076ffcd9e8acb2448c388efb30ff32ddf9d22f/ExifLibrary/ExifPropertyCollection.cs#L333

And no overload seems to exist that accepts UFraction32 as value, so the object overload is picked...

I think that is a bug, but the author @oozcitak should confirm...

As a workaround, you could try

sourceImageFile.Properties.Set(ExifTag.ExposureTime, (double) sourceExifImageFile.Properties.Get(ExifTag.ExposureTime).Value);

but that is not ideal IMHO

Or maybe just

var et = sourceExifImageFile.Properties.Get(ExifTag.ExposureTime);
if (et != null) 
    sourceImageFile.Properties.Add(et);

[victornor]

That likely is caused by suggestion #73 of mine...

Could you try using https://www.nuget.org/packages/ExifLibNet/2.1.1 and see if the problem still occurs?

Thanks for the quick response. The exception i referred to in the issue was thrown when using the master branch. When i use the nuget package, there's no exceptions when setting the value, but instead there's an exception when attempting to save the modified image.

System.ArgumentException: Type provided must be an Enum. (Parameter 'enumType') at at System.RuntimeType.GetEnumUnderlyingType() at at System.Enum.GetUnderlyingType(Type enumType) at at ExifLibrary.ExifEnumProperty1.get_Interoperability() at at ExifLibrary.JPEGFile.WriteIFD(MemoryStream stream, Dictionary2 ifd, IFD ifdtype, Int64 tiffoffset, Boolean preserveMakerNote) at at ExifLibrary.JPEGFile.WriteExifApp1(Boolean preserveMakerNote) at at ExifLibrary.JPEGFile.SaveInternal(MemoryStream stream) at at ExifLibrary.ImageFile.Save(Stream stream) at at ExifLibrary.ImageFile.Save(String filename)

This is using the 2.1.1 version of the nuget package

[victornor]

Actually, it seems the Set overload that accepts an object type, assumes the value must be an enum...

https://github.com/oozcitak/exiflibrary/blob/4e076ffcd9e8acb2448c388efb30ff32ddf9d22f/ExifLibrary/ExifPropertyCollection.cs#L333

And no overload seems to exist that accepts UFraction32 as value, so the object overload is picked...

I think that is a bug, but the author @oozcitak should confirm...

As a workaround, you could try

sourceImageFile.Properties.Set(ExifTag.ExposureTime, (double) sourceExifImageFile.Properties.Get(ExifTag.ExposureTime).Value);

but that is not ideal IMHO

Or maybe just

var et = sourceExifImageFile.Properties.Get(ExifTag.ExposureTime);
if (et != null) 
    sourceImageFile.Properties.Add(et);

It's not possible to cast the UFraction32 to a double

[victornor]

Actually, it seems the Set overload that accepts an object type, assumes the value must be an enum...

https://github.com/oozcitak/exiflibrary/blob/4e076ffcd9e8acb2448c388efb30ff32ddf9d22f/ExifLibrary/ExifPropertyCollection.cs#L333

And no overload seems to exist that accepts UFraction32 as value, so the object overload is picked...

I think that is a bug, but the author @oozcitak should confirm...

As a workaround, you could try

sourceImageFile.Properties.Set(ExifTag.ExposureTime, (double) sourceExifImageFile.Properties.Get(ExifTag.ExposureTime).Value);

but that is not ideal IMHO

Or maybe just

var et = sourceExifImageFile.Properties.Get(ExifTag.ExposureTime);
if (et != null) 
    sourceImageFile.Properties.Add(et);

I was however able to add the Exif property directly, but i might have images which already has the exif property and i'm assuming adding would leave the image with two properties with the same key?

[oozcitak]

@victornor The Set method has some overloads for common types (byte, int, string, etc.) but MathEx.Ufraction32 is not one of these, this is why it falls down to the object overload, which was meant for enum types. I can fix the case for UFraction32 but sooner or later it will fail for some other type which is unaccounted for.

The safest workaround is to use the property object instead of its value as @Ziriax metioned. If you don't want duplicate values use Set instead of Add:

sourceImageFile.Properties.Set(sourceExifImageFile.Properties.Get(ExifTag.ExposureTime));

[ziriax]

Maybe something like the following code would make this more bullet proof, and provide a better error message:

In ExifPropertyFactory.cs:

        public static ExifProperty FromTaggedValue(ExifTag tag, object value)
        {
            Type type = value?.GetType() ?? throw new ArgumentNullException(nameof(value));
            TypeInfo typeInfo = type.GetTypeInfo();

            ExifProperty property = value switch
            {
                float v => new ExifURational(tag, new MathEx.UFraction32(v)),
                double v => new ExifURational(tag, new MathEx.UFraction32(v)),
                byte v => new ExifByte(tag, v),
                // etc...
                _ when typeInfo.IsEnum => (ExifProperty)Activator.CreateInstance(typeof(ExifEnumProperty<>).MakeGenericType(type), tag, value),
                _ => throw new ArgumentException("Unsupported value type", nameof(value))
            };

            return property;
        }

In ExifPropertyCollection.cs:

        public void Add(ExifTag key, object value)
        {
           AddItem(ExifPropertyFactory.FromTaggedValue(key, value));
        }

        public void Set(ExifTag key, object value)
        {
            SetItem(ExifPropertyFactory.FromTaggedValue(key, value));
        }

Another approach would be to have a mapping between the ExifTag and the required type, but maybe some tags accept multiple types, I'm not that familiar with the EXIF spec...

?

[oozcitak]

@Ziriax This is still not bulletproof. Some tags require UFraction32 where some require Fraction32. It is not possible to decide which one based on just a float value.

Another approach would be to have a mapping between the ExifTag and the required type, but maybe some tags accept multiple types, I'm not that familiar with the EXIF spec...

This would be the way to go, but as you suspected some tags accept multiple values (e.g. SubjectArea).

[ziriax]

Yes, indeed.

However, currently your code maps all float and double to UFraction32

Maybe these methods should get an (optional) 3rd parameter, bool isSigned = false?

[oozcitak]

Maybe these methods should get an (optional) 3rd parameter, bool isSigned = false?

I think this would be the best solution. Too bad it cannot be applied to the index property though. Wanna send a PR?

[ziriax]

Wanna send a PR?

Sure! But my last PR failed the CI here... Hopefully it works now.

[ziriax]

Too bad it cannot be applied to the index property though

Then maybe the indexer should be marked deprecated, and should only accept explicit fractions?