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

EXIF Support for PNG's #616

Merged
merged 45 commits into from
Aug 4, 2018
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
45 commits
Select commit Hold shift + click to select a range
cd44c22
added EXIF chunk type and setting ExifProfile, if eXIf chunk data is …
brianpopow Jun 12, 2018
39e3122
writes the eXIf chunk to the stream during encoding (#611)
brianpopow Jun 12, 2018
89f60fc
if ignoreMetadata is set, EXIF chunk will be ignored
brianpopow Jun 12, 2018
4ed1df8
added unit test which writes a png with a Exif Profile and checks aft…
brianpopow Jun 13, 2018
820f30c
- no longer exposing raw exif data byte's, using ToByteArray to write…
brianpopow Jun 14, 2018
34e2443
all Exif tests which write and load images are now testing png and jpg
brianpopow Jun 16, 2018
9e6c3e5
removed code duplication setting the byte order marker
brianpopow Jun 17, 2018
fd02990
ToByteArray skips the first 6 bytes if includeExifCode is false, adde…
brianpopow Jun 17, 2018
5594177
Merge branch 'master' into feature/pngExif
brianpopow Jun 17, 2018
e26ea49
avoiding unnecessary allocation of the exif id byte array in StartsWi…
brianpopow Jun 17, 2018
8a2c8d9
Merge branch 'master' into feature/pngExif
brianpopow Jun 18, 2018
bc9ba35
Merge branch 'master' into feature/pngExif
brianpopow Jun 20, 2018
48178b8
Merge remote-tracking branch 'upstream/master' into feature/pngExif
brianpopow Jun 23, 2018
5a7926d
Merge remote-tracking branch 'origin/feature/pngExif' into feature/pn…
brianpopow Jun 23, 2018
a0a0142
removed ExifIdCode from ExifConstants and ExifProfile, using ExifMark…
brianpopow Jun 23, 2018
88e41ea
using Span CopyTo() in the ExifProfile Constructor to copy the data f…
brianpopow Jun 23, 2018
9d81e7c
Merge remote-tracking branch 'upstream/master' into feature/pngExif
brianpopow Jun 23, 2018
53336ea
fixed submodule merge mistake, setting it to "add reference output fo…
brianpopow Jun 23, 2018
7e688ee
corrected big endian byte order marker
brianpopow Jun 23, 2018
976aad3
using defined exif constants in the TestProfileToByteArrayWorks Test
brianpopow Jun 23, 2018
4b484f9
to make ExifProfile format agnostic again: passing ExifIdCode as Read…
brianpopow Jun 24, 2018
caf7fb3
Merge remote-tracking branch 'upstream/master' into feature/pngExif
brianpopow Jun 24, 2018
565d885
Merge branch 'master' into feature/pngExif
JimBobSquarePants Jun 28, 2018
c228f94
commented out not needed references
brianpopow Jun 27, 2018
f32e68a
- fixed error message in WriteExifProfile, when Exif data exceeds the…
brianpopow Jun 27, 2018
700fde4
Merge remote-tracking branch 'upstream/master' into feature/pngExif
brianpopow Jun 28, 2018
94da0b3
added SyncProfiles() before writing the Exif Chunk
brianpopow Jun 28, 2018
f544e18
if Exif data exceeds 64K and is split over multiple App1 marker, it w…
brianpopow Jul 7, 2018
e6bcb2b
if exif data exceeds 64k, it will be written in multiple APP1 markers
brianpopow Jul 7, 2018
685b5c5
added unit test for writing large exif profiles
brianpopow Jul 7, 2018
ff160a7
Merge remote-tracking branch 'upstream/master' into feature/pngExif
brianpopow Jul 7, 2018
a5d576e
removed unnecessary comments
brianpopow Jul 9, 2018
35aaaee
removed ExifIdCode parameter from ExifProfile.ToByteArray, because th…
brianpopow Jul 9, 2018
cb6f4a0
to keep the ExifReader free from jpeg specific stuff, the Exif Id Cod…
brianpopow Jul 9, 2018
7d7e518
Merge branch 'master' into feature/pngExif
brianpopow Jul 9, 2018
c4e81de
skipping exif id code before extending the exif profile
brianpopow Jul 10, 2018
a995021
added support for large exif profiles for the old GolangJpegDecoderCore
brianpopow Jul 10, 2018
8834f15
Merge branch 'master' into feature/pngExif
brianpopow Jul 10, 2018
efd0288
moved extending the ExifProfile to the jpeg decoder
brianpopow Jul 13, 2018
7f21da4
Merge branch 'master' into feature/pngExif
brianpopow Jul 13, 2018
7e8b464
Merge branch 'master' into feature/pngExif
JimBobSquarePants Jul 16, 2018
9aba104
Merge branch 'master' into feature/pngExif
JimBobSquarePants Jul 17, 2018
21a7c78
Merge branch 'master' into feature/pngExif
JimBobSquarePants Jul 18, 2018
659bc7c
Merge remote-tracking branch 'upstream/master' into feature/pngExif
JimBobSquarePants Aug 4, 2018
6c777d4
Align ICC and EXIF API.
JimBobSquarePants Aug 4, 2018
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 7 additions & 2 deletions src/ImageSharp/Formats/Png/PngChunkType.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
namespace SixLabors.ImageSharp.Formats.Png
{
/// <summary>
/// Contains a list of of chunk types.
/// Contains a list of chunk types.
/// </summary>
internal enum PngChunkType : uint
{
Expand Down Expand Up @@ -55,6 +55,11 @@ internal enum PngChunkType : uint
/// <summary>
/// The pHYs chunk specifies the intended pixel size or aspect ratio for display of the image.
/// </summary>
Physical = 0x70485973U // pHYs
Physical = 0x70485973U, // pHYs

/// <summary>
/// The data chunk which contains the Exif profile.
/// </summary>
Exif = 0x65584966U // eXIf
}
}
10 changes: 10 additions & 0 deletions src/ImageSharp/Formats/Png/PngDecoderCore.cs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
using SixLabors.ImageSharp.Formats.Png.Zlib;
using SixLabors.ImageSharp.Memory;
using SixLabors.ImageSharp.MetaData;
using SixLabors.ImageSharp.MetaData.Profiles.Exif;
using SixLabors.ImageSharp.PixelFormats;

namespace SixLabors.ImageSharp.Formats.Png
Expand Down Expand Up @@ -249,6 +250,15 @@ public Image<TPixel> Decode<TPixel>(Stream stream)
break;
case PngChunkType.Text:
this.ReadTextChunk(metadata, chunk.Data.Array, chunk.Length);
break;
case PngChunkType.Exif:
if (!this.ignoreMetadata)
{
byte[] exifData = new byte[chunk.Length];
Buffer.BlockCopy(chunk.Data.Array, 0, exifData, 0, chunk.Length);
metadata.ExifProfile = new ExifProfile(exifData);
}

break;
case PngChunkType.End:
this.isEndChunkReached = true;
Expand Down
18 changes: 17 additions & 1 deletion src/ImageSharp/Formats/Png/PngEncoderCore.cs
Original file line number Diff line number Diff line change
Expand Up @@ -221,6 +221,7 @@ public void Encode<TPixel>(Image<TPixel> image, Stream stream)

this.WritePhysicalChunk(stream, image);
this.WriteGammaChunk(stream);
this.WriteExifChunk(stream, image);
this.WriteDataChunks(image.Frames.RootFrame, stream);
this.WriteEndChunk(stream);
stream.Flush();
Expand Down Expand Up @@ -397,7 +398,7 @@ private IManagedByteBuffer GetOptimalFilteredScanline()
/// <summary>
/// Calculates the correct number of bytes per pixel for the given color type.
/// </summary>
/// <returns>The <see cref="int"/></returns>
/// <returns>Bytes per pixel</returns>
private int CalculateBytesPerPixel()
{
switch (this.pngColorType)
Expand Down Expand Up @@ -523,6 +524,21 @@ private void WritePhysicalChunk<TPixel>(Stream stream, Image<TPixel> image)
}
}

/// <summary>
/// Writes the eXIf chunk to the stream, if any EXIF Profile values are present in the meta data.
/// </summary>
/// <typeparam name="TPixel">The pixel format.</typeparam>
/// <param name="stream">The <see cref="Stream"/> containing image data.</param>
/// <param name="image">The image.</param>
private void WriteExifChunk<TPixel>(Stream stream, Image<TPixel> image)
where TPixel : struct, IPixel<TPixel>
{
if (image.MetaData.ExifProfile?.Values.Count > 0)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to alter this slightly as there's extra checks and balances.

image.MetaData.SyncProfiles(); is required before writing according to the jpeg encoder and we need additional length checks as the spec dictates it's length should match jpeg limitations.

private void WriteExifProfile(ExifProfile exifProfile)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i have added the SyncProfile before writing the chunk.

For the length check: I could add a check, if the length exceeds 2^16, like the JpegEncoder does, and throw an exception, but that feels wrong to me. This restriction is Jpeg specific not PNG specific.

In the recommendations they state, when writing PNG Exif to Jpeg:

the total length of the eXIf chunk data may need to be adjusted

But how to decide what to cut off here is not clear to me. I really would like to hear @dlemstra opinion on that.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking at this it appears we should simply the split data into 64k chunks. http://dev.exiv2.org/issues/1232

That means we need an extend method, like the ICC readeron decoding.

this.MetaData.IccProfile.Extend(profile);

{
this.WriteChunk(stream, PngChunkType.Exif, image.MetaData.ExifProfile.ToByteArray(includeExifIdCode: false));
}
}

/// <summary>
/// Writes the gamma information to the stream.
/// </summary>
Expand Down
7 changes: 4 additions & 3 deletions src/ImageSharp/MetaData/Profiles/Exif/ExifProfile.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@

using System;
using System.Collections.Generic;
using System.Collections.ObjectModel;
using System.IO;
using SixLabors.ImageSharp.PixelFormats;
using SixLabors.ImageSharp.Primitives;
Expand Down Expand Up @@ -233,8 +232,10 @@ public void SetValue(ExifTag tag, object value)
/// <summary>
/// Converts this instance to a byte array.
/// </summary>
/// <param name="includeExifIdCode">Indicates, if the Exif ID code should be included.
/// This Exif ID code should not be included in case of PNG's. Defaults to true.</param>
/// <returns>The <see cref="T:byte[]"/></returns>
public byte[] ToByteArray()
public byte[] ToByteArray(bool includeExifIdCode = true)
Copy link
Member

@JimBobSquarePants JimBobSquarePants Jun 21, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I want to move the Exif identifier out of the ExifProfile as that's defined as part of the App1 segment itself in section 4.7.2 of the specification.

http://www.exif.org/Exif2-2.PDF

Ideally we should pass the marker to the ToByteArray method when exporting. The ExifProfile constructor should have an overload accepting the marker.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, i will do that on the weekend.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fantastic, thanks!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pass through the code as a ReadonlySpan<bye> copying that to the output array.

{
if (this.values == null)
Copy link
Member

@dlemstra dlemstra Jun 17, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should also check if this.data starts with the exif id and skip the first 6 bytes when includeExifIdCode is false.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good point, i missed that one. I have added a unit test for that case.

{
Expand All @@ -247,7 +248,7 @@ public byte[] ToByteArray()
}

var writer = new ExifWriter(this.values, this.Parts);
return writer.GetData();
return writer.GetData(includeExifIdCode);
}

/// <summary>
Expand Down
1 change: 1 addition & 0 deletions src/ImageSharp/MetaData/Profiles/Exif/ExifReader.cs
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ public List<ExifValue> ReadValues()

if (this.ReadString(4) == "Exif")
{
// two zeros are expected to follow the Exif Id code
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we really need this comment?

if (this.ReadUInt16() != 0)
{
return values;
Expand Down
76 changes: 51 additions & 25 deletions src/ImageSharp/MetaData/Profiles/Exif/ExifWriter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,6 @@ namespace SixLabors.ImageSharp.MetaData.Profiles.Exif
/// </summary>
internal sealed class ExifWriter
{
/// <summary>
/// The start index.
/// </summary>
private const int StartIndex = 6;

/// <summary>
/// Which parts will be written.
/// </summary>
Expand Down Expand Up @@ -46,11 +41,14 @@ public ExifWriter(IList<ExifValue> values, ExifParts allowedParts)
/// <summary>
/// Returns the EXIF data.
/// </summary>
/// <param name="includeExifIdCode">Indicates, if the Exif ID code should be included.
/// This Exif ID code should not be included in case of PNG's. Defaults to true.</param>
/// <returns>
/// The <see cref="T:byte[]"/>.
/// </returns>
public byte[] GetData()
public byte[] GetData(bool includeExifIdCode = true)
{
uint startIndex = 6;
uint length;
int exifIndex = -1;
int gpsIndex = -1;
Expand Down Expand Up @@ -86,23 +84,51 @@ public byte[] GetData()
return null;
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

im not sure about this one, i think this was supposed to return null, if only the Exif Id code is present. Is that correct?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dlemstra What do you think?

}

length += 10 + 4 + 2;
if (includeExifIdCode)
{
// Exif Code (6 bytes) + byte order marker (4 bytes)
length += 10;
}
else
{
// special case for PNG eXIf Chunk:
// two bytes for the byte Order marker 'II', followed by the number 42 (0x2A) and a 0, making 4 bytes total
length += 4;

// if the Exif Code ("Exif00") is not included, the start index is 0 instead of 6
startIndex = 0;
}

length += 4 + 2;

byte[] result = new byte[length];

result[0] = (byte)'E';
result[1] = (byte)'x';
result[2] = (byte)'i';
result[3] = (byte)'f';
result[4] = 0x00;
result[5] = 0x00;
result[6] = (byte)'I';
result[7] = (byte)'I';
result[8] = 0x2A;
result[9] = 0x00;

int i = 10;
uint ifdOffset = ((uint)i - StartIndex) + 4;
int i = 0;
if (includeExifIdCode)
{
result[0] = (byte)'E';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could also increment i when you set the values so you don't need the duplicate code block in the else part.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i have changed that

result[1] = (byte)'x';
result[2] = (byte)'i';
result[3] = (byte)'f';
result[4] = 0x00;
result[5] = 0x00;
result[6] = (byte)'I';
result[7] = (byte)'I';
result[8] = 0x2A;
result[9] = 0x00;
i = 10;
}
else
{
// the byte order marker followed by the number 42 and a 0
result[0] = (byte)'I';
result[1] = (byte)'I';
result[2] = 0x2A;
result[3] = 0x00;
i = 4;
}

uint ifdOffset = ((uint)i - startIndex) + 4;
uint thumbnailOffset = ifdOffset + ifdLength + exifLength + gpsLength;

if (exifLength > 0)
Expand All @@ -118,18 +144,18 @@ public byte[] GetData()
i = WriteUInt32(ifdOffset, result, i);
i = this.WriteHeaders(this.ifdIndexes, result, i);
i = WriteUInt32(thumbnailOffset, result, i);
i = this.WriteData(this.ifdIndexes, result, i);
i = this.WriteData(startIndex, this.ifdIndexes, result, i);

if (exifLength > 0)
{
i = this.WriteHeaders(this.exifIndexes, result, i);
i = this.WriteData(this.exifIndexes, result, i);
i = this.WriteData(startIndex, this.exifIndexes, result, i);
}

if (gpsLength > 0)
{
i = this.WriteHeaders(this.gpsIndexes, result, i);
i = this.WriteData(this.gpsIndexes, result, i);
i = this.WriteData(startIndex, this.gpsIndexes, result, i);
}

WriteUInt16((ushort)0, result, i);
Expand Down Expand Up @@ -266,7 +292,7 @@ private int WriteArray(ExifValue value, byte[] destination, int offset)
return newOffset;
}

private int WriteData(List<int> indexes, byte[] destination, int offset)
private int WriteData(uint startIndex, List<int> indexes, byte[] destination, int offset)
{
if (this.dataOffsets.Count == 0)
{
Expand All @@ -281,7 +307,7 @@ private int WriteData(List<int> indexes, byte[] destination, int offset)
ExifValue value = this.values[index];
if (value.Length > 4)
{
WriteUInt32((uint)(newOffset - StartIndex), destination, this.dataOffsets[i++]);
WriteUInt32((uint)(newOffset - startIndex), destination, this.dataOffsets[i++]);
newOffset = this.WriteValue(value, destination, newOffset);
}
}
Expand Down
Loading