From 843976865eaa187faa042a18eac420b2a6260498 Mon Sep 17 00:00:00 2001 From: Dustin Oprea Date: Thu, 24 May 2018 01:58:58 -0400 Subject: [PATCH] ifdBuilder.dumpToStrings: Fixed usage of II. - We were ambiguously treating it as both the parent II in some places and the child II in others. It's supposed to be the parent II. --- ifd_builder.go | 17 +++--- ifd_builder_encode_test.go | 112 ------------------------------------ ifd_builder_test.go | 114 ++++++++++++++++++++++++++++++++++++- ifd_enumerate.go | 2 +- 4 files changed, 123 insertions(+), 122 deletions(-) diff --git a/ifd_builder.go b/ifd_builder.go index 29dc3fb..16490d0 100644 --- a/ifd_builder.go +++ b/ifd_builder.go @@ -62,7 +62,7 @@ func (ibtv IfdBuilderTagValue) Ib() *IfdBuilder { type builderTag struct { - // ii is non-empty if represents a child-IFD. + // ii is the IfdIdentity of the IFD that hosts this tag. ii IfdIdentity tagId uint16 @@ -446,14 +446,15 @@ func (ib *IfdBuilder) dumpToStrings(thisIb *IfdBuilder, prefix string, lines []s } for i, tag := range thisIb.tags { - line := fmt.Sprintf(" IFD-TAG-ID=(0x%02x) CHILD-IFD=[%s] INDEX=(%d) TAG=[0x%02x]", prefix, thisIb.ii.IfdName, thisIb.ifdTagId, tag.ii.IfdName, i, tag.tagId) + childIfdName := "" + if tag.value.IsIb() == true { + childIfdName = tag.value.Ib().ii.IfdName + } + + line := fmt.Sprintf(" IFD-TAG-ID=(0x%02x) CHILD-IFD=[%s] INDEX=(%d) TAG=[0x%02x]", prefix, thisIb.ii.IfdName, thisIb.ifdTagId, childIfdName, i, tag.tagId) linesOutput = append(linesOutput, line) - if tag.ii.IfdName != "" { - if tag.value.IsIb() == false { - log.Panicf("tag has IFD tag-ID (0x%02x) and is acting like a child IB but does not *look* like a child IB: %v", tag.tagId, tag) - } - + if tag.value.IsIb() == true { childPrefix := "" if prefix == "" { childPrefix = fmt.Sprintf("%s", thisIb.ii.IfdName) @@ -669,7 +670,7 @@ func (ib *IfdBuilder) AddChildIb(childIb *IfdBuilder) (err error) { value := NewIfdBuilderTagValueFromIfdBuilder(childIb) bt := NewChildIfdBuilderTag( - childIb.ii, + ib.ii, childIb.ifdTagId, value) diff --git a/ifd_builder_encode_test.go b/ifd_builder_encode_test.go index 754f7af..dd16876 100644 --- a/ifd_builder_encode_test.go +++ b/ifd_builder_encode_test.go @@ -6,8 +6,6 @@ import ( "reflect" "fmt" "strings" - "path" - "sort" "github.com/dsoprea/go-logging" ) @@ -927,113 +925,3 @@ func ExampleIfdByteEncoder_EncodeToExif() { // 4: IfdTagEntry [[{286335522 858997828}]] // 5: IfdTagEntry [[{286335522 858997828}]] } - -func TestNewIfdBuilderFromExistingChain_RealData(t *testing.T) { - filepath := path.Join(assetsPath, "NDM_8901.jpg") - - e := NewExif() - - - rawExif, err := e.SearchAndExtractExif(filepath) - log.PanicIf(err) - - // Decode from binary. - - _, index, err := e.Collect(rawExif) - log.PanicIf(err) - - originalTags := index.RootIfd.DumpTags() - - // Encode back to binary. - - ibe := NewIfdByteEncoder() - - rootIb := NewIfdBuilderFromExistingChain(index.RootIfd, rawExif) - - updatedExif, err := ibe.EncodeToExif(rootIb) - log.PanicIf(err) - - // Parse again. - - _, index, err = e.Collect(updatedExif) - log.PanicIf(err) - - recoveredTags := index.RootIfd.DumpTags() - - - // Validate that all of the same IFDs were presented. - - originalIfdTags := make([][2]interface{}, 0) - for _, ite := range originalTags { - if ite.ChildIfdName != "" { - originalIfdTags = append(originalIfdTags, [2]interface{} { ite.Ii, ite.TagId }) - } - } - - recoveredIfdTags := make([][2]interface{}, 0) - for _, ite := range recoveredTags { - if ite.ChildIfdName != "" { - recoveredIfdTags = append(recoveredIfdTags, [2]interface{} { ite.Ii, ite.TagId }) - } - } - - if reflect.DeepEqual(recoveredIfdTags, originalIfdTags) != true { - fmt.Printf("Original IFD tags:\n\n") - - for i, x := range originalIfdTags { - fmt.Printf(" %02d %v\n", i, x) - } - - fmt.Printf("\nRecovered IFD tags:\n\n") - - for i, x := range recoveredIfdTags { - fmt.Printf(" %02d %v\n", i, x) - } - - fmt.Printf("\n") - - t.Fatalf("Recovered IFD tags are not correct.") - } - - - // Validate that all of the tags owned by the IFDs were presented. The tags - // might not be in the same order since the IFD tags are allocated after - // the non-IFD ones. - - originalNonIfdTags := make([]string, 0) - for _, ite := range originalTags { - if ite.ChildIfdName == "" { - originalNonIfdTags = append(originalNonIfdTags, fmt.Sprintf("%s 0x%02x", ite.Ii, ite.TagId)) - } - } - - sort.StringSlice(originalNonIfdTags).Sort() - - recoveredNonIfdTags := make([]string, 0) - for _, ite := range recoveredTags { - if ite.ChildIfdName == "" { - recoveredNonIfdTags = append(recoveredNonIfdTags, fmt.Sprintf("%s 0x%02x", ite.Ii, ite.TagId)) - } - } - - sort.StringSlice(recoveredNonIfdTags).Sort() - - - if reflect.DeepEqual(recoveredNonIfdTags, originalNonIfdTags) != true { - fmt.Printf("Original non-IFD tags:\n\n") - - for i, x := range originalNonIfdTags { - fmt.Printf(" %02d %v\n", i, x) - } - - fmt.Printf("\nRecovered non-IFD tags:\n\n") - - for i, x := range recoveredNonIfdTags { - fmt.Printf(" %02d %v\n", i, x) - } - - fmt.Printf("\n") - - t.Fatalf("Recovered non-IFD tags are not correct.") - } -} diff --git a/ifd_builder_test.go b/ifd_builder_test.go index 3c4242c..8834c8a 100644 --- a/ifd_builder_test.go +++ b/ifd_builder_test.go @@ -5,6 +5,8 @@ import ( "reflect" "bytes" "path" + "fmt" + "sort" "github.com/dsoprea/go-logging" ) @@ -1263,7 +1265,7 @@ func TestNewIfdBuilderFromExistingChain(t *testing.T) { " IFD-TAG-ID=(0x8769) CHILD-IFD=[] INDEX=(18) TAG=[0x9290]", " IFD-TAG-ID=(0x8769) CHILD-IFD=[] INDEX=(19) TAG=[0x9291]", " IFD-TAG-ID=(0x8769) CHILD-IFD=[] INDEX=(20) TAG=[0x9292]", -- " IFD-TAG-ID=(0x8769) CHILD-IFD=[] INDEX=(21) TAG=[0xa000]", + " IFD-TAG-ID=(0x8769) CHILD-IFD=[] INDEX=(21) TAG=[0xa000]", " IFD-TAG-ID=(0x8769) CHILD-IFD=[] INDEX=(22) TAG=[0xa001]", " IFD-TAG-ID=(0x8769) CHILD-IFD=[] INDEX=(23) TAG=[0xa002]", " IFD-TAG-ID=(0x8769) CHILD-IFD=[] INDEX=(24) TAG=[0xa003]", @@ -1293,6 +1295,115 @@ func TestNewIfdBuilderFromExistingChain(t *testing.T) { // TODO(dustin): !! Test with an actual GPS-attached image. +func TestNewIfdBuilderFromExistingChain_RealData(t *testing.T) { + filepath := path.Join(assetsPath, "NDM_8901.jpg") + + e := NewExif() + + + rawExif, err := e.SearchAndExtractExif(filepath) + log.PanicIf(err) + + // Decode from binary. + + _, index, err := e.Collect(rawExif) + log.PanicIf(err) + + originalTags := index.RootIfd.DumpTags() + + // Encode back to binary. + + ibe := NewIfdByteEncoder() + + rootIb := NewIfdBuilderFromExistingChain(index.RootIfd, rawExif) + + updatedExif, err := ibe.EncodeToExif(rootIb) + log.PanicIf(err) + + // Parse again. + + _, index, err = e.Collect(updatedExif) + log.PanicIf(err) + + recoveredTags := index.RootIfd.DumpTags() + + + // Validate that all of the same IFDs were presented. + + originalIfdTags := make([][2]interface{}, 0) + for _, ite := range originalTags { + if ite.ChildIfdName != "" { + originalIfdTags = append(originalIfdTags, [2]interface{} { ite.Ii, ite.TagId }) + } + } + + recoveredIfdTags := make([][2]interface{}, 0) + for _, ite := range recoveredTags { + if ite.ChildIfdName != "" { + recoveredIfdTags = append(recoveredIfdTags, [2]interface{} { ite.Ii, ite.TagId }) + } + } + + if reflect.DeepEqual(recoveredIfdTags, originalIfdTags) != true { + fmt.Printf("Original IFD tags:\n\n") + + for i, x := range originalIfdTags { + fmt.Printf(" %02d %v\n", i, x) + } + + fmt.Printf("\nRecovered IFD tags:\n\n") + + for i, x := range recoveredIfdTags { + fmt.Printf(" %02d %v\n", i, x) + } + + fmt.Printf("\n") + + t.Fatalf("Recovered IFD tags are not correct.") + } + + + // Validate that all of the tags owned by the IFDs were presented. The tags + // might not be in the same order since the IFD tags are allocated after + // the non-IFD ones. + + originalNonIfdTags := make([]string, 0) + for _, ite := range originalTags { + if ite.ChildIfdName == "" { + originalNonIfdTags = append(originalNonIfdTags, fmt.Sprintf("%s 0x%02x", ite.Ii, ite.TagId)) + } + } + + sort.StringSlice(originalNonIfdTags).Sort() + + recoveredNonIfdTags := make([]string, 0) + for _, ite := range recoveredTags { + if ite.ChildIfdName == "" { + recoveredNonIfdTags = append(recoveredNonIfdTags, fmt.Sprintf("%s 0x%02x", ite.Ii, ite.TagId)) + } + } + + sort.StringSlice(recoveredNonIfdTags).Sort() + + + if reflect.DeepEqual(recoveredNonIfdTags, originalNonIfdTags) != true { + fmt.Printf("Original non-IFD tags:\n\n") + + for i, x := range originalNonIfdTags { + fmt.Printf(" %02d %v\n", i, x) + } + + fmt.Printf("\nRecovered non-IFD tags:\n\n") + + for i, x := range recoveredNonIfdTags { + fmt.Printf(" %02d %v\n", i, x) + } + + fmt.Printf("\n") + + t.Fatalf("Recovered non-IFD tags are not correct.") + } +} func TestNewIfdBuilderWithExistingIfd(t *testing.T) { tagId := IfdTagIdWithIdentityOrFail(GpsIi) @@ -1387,3 +1498,4 @@ func TestAddFromConfigWithName(t *testing.T) { t.Fatalf("Value not correct: (%d) [%s]", len(s), s) } } + diff --git a/ifd_enumerate.go b/ifd_enumerate.go index 02b9a02..7b08bd8 100644 --- a/ifd_enumerate.go +++ b/ifd_enumerate.go @@ -426,7 +426,7 @@ func (ifd Ifd) dumpTags(tags []*IfdTagEntry) []*IfdTagEntry { return tags } -// PrintTagTree prints the IFD hierarchy. +// DumpTags prints the IFD hierarchy. func (ifd Ifd) DumpTags() []*IfdTagEntry { return ifd.dumpTags(nil) }