ifd_enumerate.go: Bugfix for ITEs not representing fully-qualified IFD paths

- This resulted in the lookup table having unique entries, and therefore
  no longer needing its values to be slices. We are going to fix this.
  It is not a backwards-compatible change, but this is a fairly internal
  internal functionality.

- Add simple accessors to determine if a thumbnail offset or size.
dustin/add_skipped_tags_tracking
Dustin Oprea 2020-05-16 16:51:54 -04:00
parent efa6e2d6c0
commit 220964731d
8 changed files with 75 additions and 41 deletions

View File

@ -179,10 +179,8 @@ func main() {
log.PanicIf(err)
var thumbnail []byte
if matches, found := index.Lookup[exif.ThumbnailFqIfdPath]; found == true {
ifd1 := matches[0]
thumbnail, err = ifd1.Thumbnail()
if ifd, found := index.Lookup[exif.ThumbnailFqIfdPath]; found == true {
thumbnail, err = ifd.Thumbnail()
if err != nil && err != exif.ErrNoThumbnail {
log.Panic(err)
}

View File

@ -99,7 +99,7 @@ IFD-PATH=[IFD] ID=(0x0128) NAME=[ResolutionUnit] COUNT=(1) TYPE=[SHORT] VALUE=[2
IFD-PATH=[IFD] ID=(0x0201) NAME=[JPEGInterchangeFormat] COUNT=(1) TYPE=[LONG] VALUE=[11444]
IFD-PATH=[IFD] ID=(0x0202) NAME=[JPEGInterchangeFormatLength] COUNT=(1) TYPE=[LONG] VALUE=[21491]
EXIF blob is approximately (11442) bytes.
EXIF blob is approximately (32935) bytes.
`
if actual != expected {

View File

@ -5,6 +5,7 @@ import (
"fmt"
"os"
"reflect"
"sort"
"testing"
"encoding/binary"
@ -98,7 +99,7 @@ func TestVisit(t *testing.T) {
_, furthestOffset, err := Visit(exifcommon.IfdStandard, im, ti, data[foundAt:], visitor)
log.PanicIf(err)
if furthestOffset != 11442 {
if furthestOffset != 32935 {
t.Fatalf("Furthest-offset is not valid: (%d)", furthestOffset)
}
@ -276,8 +277,27 @@ func TestCollect(t *testing.T) {
t.Fatalf("The IFD list is not the right size: (%d)", len(ifds))
} else if len(tree) != 5 {
t.Fatalf("The IFD tree is not the right size: (%d)", len(tree))
} else if len(lookup) != 4 {
t.Fatalf("The IFD lookup is not the right size: (%d)", len(lookup))
}
actualIfdPaths := make([]string, len(lookup))
i := 0
for ifdPath, _ := range lookup {
actualIfdPaths[i] = ifdPath
i++
}
sort.Strings(actualIfdPaths)
expectedIfdPaths := []string{
"IFD",
"IFD/Exif",
"IFD/Exif/Iop",
"IFD/GPSInfo",
"IFD1",
}
if reflect.DeepEqual(actualIfdPaths, expectedIfdPaths) != true {
t.Fatalf("The IFD lookup is not the right size: %v", actualIfdPaths)
}
if rootIfd.NextIfdOffset != 0x2c54 {
@ -302,27 +322,27 @@ func TestCollect(t *testing.T) {
t.Fatalf("Exif IFD child is not an IOP IFD: [%s]", rootIfd.Children[0].Children[0].IfdPath)
}
if lookup[exifcommon.IfdPathStandard][0].IfdPath != exifcommon.IfdPathStandard {
if lookup[exifcommon.IfdPathStandard].IfdPath != exifcommon.IfdPathStandard {
t.Fatalf("Lookup for standard IFD not correct.")
} else if lookup[exifcommon.IfdPathStandard][1].IfdPath != exifcommon.IfdPathStandard {
} else if lookup[exifcommon.IfdPathStandard+"1"].IfdPath != exifcommon.IfdPathStandard {
t.Fatalf("Lookup for standard IFD not correct.")
}
if lookup[exifcommon.IfdPathStandardExif][0].IfdPath != exifcommon.IfdPathStandardExif {
if lookup[exifcommon.IfdPathStandardExif].IfdPath != exifcommon.IfdPathStandardExif {
t.Fatalf("Lookup for EXIF IFD not correct.")
}
if lookup[exifcommon.IfdPathStandardGps][0].IfdPath != exifcommon.IfdPathStandardGps {
if lookup[exifcommon.IfdPathStandardGps].IfdPath != exifcommon.IfdPathStandardGps {
t.Fatalf("Lookup for GPS IFD not correct.")
}
if lookup[exifcommon.IfdPathStandardExifIop][0].IfdPath != exifcommon.IfdPathStandardExifIop {
if lookup[exifcommon.IfdPathStandardExifIop].IfdPath != exifcommon.IfdPathStandardExifIop {
t.Fatalf("Lookup for IOP IFD not correct.")
}
foundExif := 0
foundGps := 0
for _, ite := range lookup[exifcommon.IfdPathStandard][0].Entries {
for _, ite := range lookup[exifcommon.IfdPathStandard].Entries {
if ite.ChildIfdPath() == exifcommon.IfdPathStandardExif {
foundExif++
@ -347,7 +367,7 @@ func TestCollect(t *testing.T) {
}
foundIop := 0
for _, ite := range lookup[exifcommon.IfdPathStandardExif][0].Entries {
for _, ite := range lookup[exifcommon.IfdPathStandardExif].Entries {
if ite.ChildIfdPath() == exifcommon.IfdPathStandardExifIop {
foundIop++

View File

@ -1042,7 +1042,7 @@ func (ib *IfdBuilder) AddTagsFromExisting(ifd *Ifd, includeTagIds []uint16, excl
}
for i, ite := range ifd.Entries {
if (ite.TagId() == ThumbnailOffsetTagId || ite.TagId() == ThumbnailSizeTagId) && ifd.FqIfdPath == ThumbnailFqIfdPath {
if ite.IsThumbnailOffset() == true || ite.IsThumbnailSize() {
// These will be added on-the-fly when we encode.
continue
}

View File

@ -422,7 +422,7 @@ func Test_IfdByteEncoder_encodeTagToBytes_childIfd__withAllocate(t *testing.T) {
t.Fatalf("Child IFD is not the right size: (%d)", len(childIfdBlock))
}
iteV, err := ParseOneTag(im, ti, fmt.Sprintf("%s%d", exifcommon.IfdPathStandard, 0), exifcommon.IfdPathStandard, exifcommon.TestDefaultByteOrder, tagBytes)
iteV, err := ParseOneTag(im, ti, exifcommon.IfdPathStandard, exifcommon.IfdPathStandard, exifcommon.TestDefaultByteOrder, tagBytes)
log.PanicIf(err)
if iteV.TagId() != exifcommon.IfdExifId {
@ -443,7 +443,7 @@ func Test_IfdByteEncoder_encodeTagToBytes_childIfd__withAllocate(t *testing.T) {
// Validate the child's raw IFD bytes.
childNextIfdOffset, childEntries, err := ParseOneIfd(im, ti, "IFD0/Exif0", "IFD/Exif", exifcommon.TestDefaultByteOrder, childIfdBlock, nil)
childNextIfdOffset, childEntries, err := ParseOneIfd(im, ti, "IFD/Exif", "IFD/Exif", exifcommon.TestDefaultByteOrder, childIfdBlock, nil)
log.PanicIf(err)
if childNextIfdOffset != uint32(0) {
@ -525,7 +525,7 @@ func Test_IfdByteEncoder_encodeTagToBytes_simpleTag_allocate(t *testing.T) {
t.Fatalf("Child IFD not have been allocated.")
}
ite, err := ParseOneTag(im, ti, fmt.Sprintf("%s%d", exifcommon.IfdPathStandard, 0), exifcommon.IfdPathStandard, exifcommon.TestDefaultByteOrder, tagBytes)
ite, err := ParseOneTag(im, ti, exifcommon.IfdPathStandard, exifcommon.IfdPathStandard, exifcommon.TestDefaultByteOrder, tagBytes)
log.PanicIf(err)
if ite.TagId() != 0x000b {

View File

@ -1642,6 +1642,12 @@ func TestIfdBuilder_NewIfdBuilderFromExistingChain_RealData(t *testing.T) {
originalTagPhrases := make([]string, 0)
for _, ite := range originalTags {
// Adds a lot of noise if/when debugging, and we're already checking the
// thumbnail bytes separately.
if ite.IsThumbnailOffset() == true || ite.IsThumbnailSize() == true {
continue
}
phrase := ite.String()
// The value (the offset) of IFDs will almost never be the same after
@ -1660,6 +1666,12 @@ func TestIfdBuilder_NewIfdBuilderFromExistingChain_RealData(t *testing.T) {
recoveredTagPhrases := make([]string, 0)
for _, ite := range recoveredTags {
// Adds a lot of noise if/when debugging, and we're already checking the
// thumbnail bytes separately.
if ite.IsThumbnailOffset() == true || ite.IsThumbnailSize() == true {
continue
}
phrase := ite.String()
// The value (the offset) of IFDs will almost never be the same after

View File

@ -240,7 +240,7 @@ func (ie *IfdEnumerate) parseTag(fqIfdPath string, tagPosition int, bp *bytePars
log.PanicIf(err)
ite = newIfdTagEntry(
ifdPath,
fqIfdPath,
tagId,
tagPosition,
tagType,
@ -310,13 +310,12 @@ func (ie *IfdEnumerate) ParseIfd(fqIfdPath string, ifdIndex int, bp *byteParser,
log.PanicIf(err)
}
tagId := ite.TagId()
if tagId == ThumbnailOffsetTagId && fqIfdPath == ThumbnailFqIfdPath {
if ite.IsThumbnailOffset() == true {
enumeratorThumbnailOffset = ite
entries = append(entries, ite)
continue
} else if tagId == ThumbnailSizeTagId && fqIfdPath == ThumbnailFqIfdPath {
} else if ite.IsThumbnailSize() == true {
enumeratorThumbnailSize = ite
entries = append(entries, ite)
@ -1006,7 +1005,7 @@ type IfdIndex struct {
RootIfd *Ifd
Ifds []*Ifd
Tree map[int]*Ifd
Lookup map[string][]*Ifd
Lookup map[string]*Ifd
}
// Scan enumerates the different EXIF blocks (called IFDs).
@ -1019,7 +1018,7 @@ func (ie *IfdEnumerate) Collect(rootIfdOffset uint32) (index IfdIndex, err error
tree := make(map[int]*Ifd)
ifds := make([]*Ifd, 0)
lookup := make(map[string][]*Ifd)
lookup := make(map[string]*Ifd)
queue := []QueuedIfd{
{
@ -1122,14 +1121,7 @@ func (ie *IfdEnumerate) Collect(rootIfdOffset uint32) (index IfdIndex, err error
tree[id] = ifd
// Install into by-name buckets.
if list_, found := lookup[fqIfdPath]; found == true {
lookup[fqIfdPath] = append(list_, ifd)
} else {
lookup[fqIfdPath] = []*Ifd{
ifd,
}
}
lookup[fqIfdPath] = ifd
// Add a link from the previous IFD in the chain to us.
if previousIfd, found := edges[offset]; found == true {

View File

@ -38,8 +38,8 @@ type IfdTagEntry struct {
// TODO(dustin): !! IB's host the child-IBs directly in the tag, but that's not the case here. Refactor to accomodate it for a consistent experience.
// ifdPath is the IFD that this tag belongs to.
ifdPath string
// fqIfdPath is the IFD that this tag belongs to.
fqIfdPath string
isUnhandledUnknown bool
@ -47,9 +47,9 @@ type IfdTagEntry struct {
byteOrder binary.ByteOrder
}
func newIfdTagEntry(ifdPath string, tagId uint16, tagIndex int, tagType exifcommon.TagTypePrimitive, unitCount uint32, valueOffset uint32, rawValueOffset []byte, addressableData []byte, byteOrder binary.ByteOrder) *IfdTagEntry {
func newIfdTagEntry(fqIfdPath string, tagId uint16, tagIndex int, tagType exifcommon.TagTypePrimitive, unitCount uint32, valueOffset uint32, rawValueOffset []byte, addressableData []byte, byteOrder binary.ByteOrder) *IfdTagEntry {
return &IfdTagEntry{
ifdPath: ifdPath,
fqIfdPath: fqIfdPath,
tagId: tagId,
tagIndex: tagIndex,
tagType: tagType,
@ -63,12 +63,12 @@ func newIfdTagEntry(ifdPath string, tagId uint16, tagIndex int, tagType exifcomm
// String returns a stringified representation of the struct.
func (ite *IfdTagEntry) String() string {
return fmt.Sprintf("IfdTagEntry<TAG-IFD-PATH=[%s] TAG-ID=(0x%04x) TAG-TYPE=[%s] UNIT-COUNT=(%d)>", ite.ifdPath, ite.tagId, ite.tagType.String(), ite.unitCount)
return fmt.Sprintf("IfdTagEntry<TAG-IFD-PATH=[%s] TAG-ID=(0x%04x) TAG-TYPE=[%s] UNIT-COUNT=(%d)>", ite.fqIfdPath, ite.tagId, ite.tagType.String(), ite.unitCount)
}
// IfdPath returns the path of the IFD that owns this tag.
// IfdPath returns the fully-qualified path of the IFD that owns this tag.
func (ite *IfdTagEntry) IfdPath() string {
return ite.ifdPath
return ite.fqIfdPath
}
// TagId returns the ID of the tag that we represent. The combination of
@ -77,6 +77,18 @@ func (ite *IfdTagEntry) TagId() uint16 {
return ite.tagId
}
// IsThumbnailOffset returns true if the tag has the IFD and tag-ID of a
// thumbnail offset.
func (ite *IfdTagEntry) IsThumbnailOffset() bool {
return ite.tagId == ThumbnailOffsetTagId && ite.fqIfdPath == ThumbnailFqIfdPath
}
// IsThumbnailSize returns true if the tag has the IFD and tag-ID of a thumbnail
// size.
func (ite *IfdTagEntry) IsThumbnailSize() bool {
return ite.tagId == ThumbnailSizeTagId && ite.fqIfdPath == ThumbnailFqIfdPath
}
// TagType is the type of value for this tag.
func (ite *IfdTagEntry) TagType() exifcommon.TagTypePrimitive {
return ite.tagType
@ -244,7 +256,7 @@ func (ite *IfdTagEntry) ChildFqIfdPath() string {
func (ite *IfdTagEntry) getValueContext() *exifcommon.ValueContext {
return exifcommon.NewValueContext(
ite.ifdPath,
ite.fqIfdPath,
ite.tagId,
ite.unitCount,
ite.valueOffset,