From 5cdc82a1932ad892a46e5d9aa9bebcf6a8946f42 Mon Sep 17 00:00:00 2001 From: cnt0 <4948391+cnt0@users.noreply.github.com> Date: Fri, 8 Feb 2019 20:04:46 +0500 Subject: [PATCH 01/23] add ChapterFrame type for CHAP frames --- chapter_frame.go | 116 +++++++++++++++++++++++++++++++++++++++++++++++ common_ids.go | 5 +- sequence.go | 2 + tag.go | 5 ++ 4 files changed, 127 insertions(+), 1 deletion(-) create mode 100644 chapter_frame.go diff --git a/chapter_frame.go b/chapter_frame.go new file mode 100644 index 0000000..5181b55 --- /dev/null +++ b/chapter_frame.go @@ -0,0 +1,116 @@ +package id3v2 + +import ( + "encoding/binary" + "io" + "time" +) + +const ( + nanosInMillis = 1000000 + IgnoredOffset = 0xFFFFFFFF +) + +// ChapterFrame is used to work with CHAP frames +// according to spec from http://id3.org/id3v2-chapters-1.0 +// This implementation only supports single TIT2 subframe (Title field). +// All other subframes are ignored. +// If StartOffset or EndOffset == id3v2.IgnoredOffset, then it should be ignored +// and StartTime or EndTime should be utilized +type ChapterFrame struct { + ElementID string + StartTime time.Duration + EndTime time.Duration + StartOffset uint32 + EndOffset uint32 + Title string +} + +func (cf ChapterFrame) Size() int { + titleFrame := TextFrame{ + Encoding: EncodingUTF8, + Text: cf.Title, + } + return encodedSize(cf.ElementID, EncodingISO) + + 1 + // trailing zero after ElementID + 4 + 4 + 4 + 4 + // (Start, End) (Time, Offset) + frameHeaderSize + // Title frame header size + titleFrame.Size() +} + +func (cf ChapterFrame) WriteTo(w io.Writer) (n int64, err error) { + return useBufWriter(w, func(bw *bufWriter) { + bw.EncodeAndWriteText(cf.ElementID, EncodingISO) + bw.WriteByte(0) + // nanoseconds => milliseconds + binary.Write(bw, binary.BigEndian, int32(cf.StartTime/nanosInMillis)) + binary.Write(bw, binary.BigEndian, int32(cf.EndTime/nanosInMillis)) + + binary.Write(bw, binary.BigEndian, cf.StartOffset) + binary.Write(bw, binary.BigEndian, cf.EndOffset) + + titleFrame := TextFrame{ + Encoding: EncodingUTF8, + Text: cf.Title, + } + writeFrame(bw, "TIT2", titleFrame, true) + }) +} + +func parseChapterFrame(br *bufReader) (Framer, error) { + ElementID := br.ReadText(EncodingISO) + chapterTime := make([]int32, 2) + for i := range chapterTime { + if err := binary.Read(br, binary.BigEndian, &chapterTime[i]); err != nil { + return nil, err + } + } + chapterOffset := make([]uint32, 2) + for i := range chapterOffset { + if err := binary.Read(br, binary.BigEndian, &chapterOffset[i]); err != nil { + return nil, err + } + } + var title string + + // borrowed from parse.go + buf := getByteSlice(32 * 1024) + defer putByteSlice(buf) + for { + // no way to determine whether this should be true or not + // this is likely should be fixed + header, err := parseFrameHeader(buf, br, true) + if err == io.EOF || err == errBlankFrame || err == ErrInvalidSizeFormat { + break + } + if err != nil { + return nil, err + } + id, bodySize := header.ID, header.BodySize + if id == "TIT2" { + bodyRd := getLimitedReader(br, bodySize) + br2 := newBufReader(bodyRd) + frame, err := parseTextFrame(br2) + if err != nil { + putLimitedReader(bodyRd) + return nil, err + } + title = frame.(TextFrame).Text + + putLimitedReader(bodyRd) + break + } + } + + cf := ChapterFrame{ + ElementID: string(ElementID), + // StartTime is given in milliseconds, so we should convert it to nanoseconds + // for time.Duration + StartTime: time.Duration(chapterTime[0] * nanosInMillis), + EndTime: time.Duration(chapterTime[1] * nanosInMillis), + StartOffset: chapterOffset[0], + EndOffset: chapterOffset[1], + Title: title, + } + return cf, nil +} diff --git a/common_ids.go b/common_ids.go index 1677b7a..cfa0a60 100644 --- a/common_ids.go +++ b/common_ids.go @@ -8,6 +8,7 @@ package id3v2 var ( V23CommonIDs = map[string]string{ "Attached picture": "APIC", + "Chapters": "CHAP", "Comments": "COMM", "Album/Movie/Show title": "TALB", "BPM": "TBPM", @@ -59,6 +60,7 @@ var ( V24CommonIDs = map[string]string{ "Attached picture": "APIC", + "Chapters": "CHAP", "Comments": "COMM", "Album/Movie/Show title": "TALB", "BPM": "TBPM", @@ -133,6 +135,7 @@ var ( // } var parsers = map[string]func(*bufReader) (Framer, error){ "APIC": parsePictureFrame, + "CHAP": parseChapterFrame, "COMM": parseCommentFrame, "TXXX": parseUserDefinedTextFrame, "UFID": parseUFIDFrame, @@ -143,7 +146,7 @@ var parsers = map[string]func(*bufReader) (Framer, error){ // be added to sequence. func mustFrameBeInSequence(id string) bool { switch id { - case "APIC", "COMM", "TXXX", "USLT": + case "APIC", "CHAP", "COMM", "TXXX", "USLT": return true } return false diff --git a/sequence.go b/sequence.go index 2abc942..26794c7 100644 --- a/sequence.go +++ b/sequence.go @@ -27,6 +27,8 @@ func (s *sequence) AddFrame(f Framer) { id = uslf.Language + uslf.ContentDescriptor } else if udtf, ok := f.(UserDefinedTextFrame); ok { id = udtf.Description + } else if cf, ok := f.(ChapterFrame); ok { + id = cf.ElementID } else { panic("sequence: unknown type of Framer") } diff --git a/tag.go b/tag.go index de0418c..a15a590 100644 --- a/tag.go +++ b/tag.go @@ -51,6 +51,11 @@ func (tag *Tag) AddAttachedPicture(pf PictureFrame) { tag.AddFrame(tag.CommonID("Attached picture"), pf) } +// AddChapterFrame adds the chapter frame to tag. +func (tag *Tag) AddChapterFrame(cf ChapterFrame) { + tag.AddFrame(tag.CommonID("Chapters"), cf) +} + // AddCommentFrame adds the comment frame to tag. func (tag *Tag) AddCommentFrame(cf CommentFrame) { tag.AddFrame(tag.CommonID("Comments"), cf) From c41e764146c7e767c177389201d3bf7b248a72dd Mon Sep 17 00:00:00 2001 From: r_takaishi Date: Tue, 19 Jan 2021 09:19:20 +0900 Subject: [PATCH 02/23] fix to parse CHAP frame --- chapter_frame.go | 79 ++++++++++++++++++++++++++++--------------- chapter_frame_test.go | 63 ++++++++++++++++++++++++++++++++++ test/test.go | 30 ++++++++++++++++ 3 files changed, 145 insertions(+), 27 deletions(-) create mode 100644 chapter_frame_test.go create mode 100644 test/test.go diff --git a/chapter_frame.go b/chapter_frame.go index 5181b55..b7e8a17 100644 --- a/chapter_frame.go +++ b/chapter_frame.go @@ -23,19 +23,21 @@ type ChapterFrame struct { EndTime time.Duration StartOffset uint32 EndOffset uint32 - Title string + Title *TextFrame + SubTitle *TextFrame } func (cf ChapterFrame) Size() int { - titleFrame := TextFrame{ - Encoding: EncodingUTF8, - Text: cf.Title, - } return encodedSize(cf.ElementID, EncodingISO) + 1 + // trailing zero after ElementID 4 + 4 + 4 + 4 + // (Start, End) (Time, Offset) frameHeaderSize + // Title frame header size - titleFrame.Size() + cf.Title.Size() + + cf.SubTitle.Size() +} + +func (cf ChapterFrame) UniqueIdentifier() string { + return cf.ElementID } func (cf ChapterFrame) WriteTo(w io.Writer) (n int64, err error) { @@ -49,29 +51,38 @@ func (cf ChapterFrame) WriteTo(w io.Writer) (n int64, err error) { binary.Write(bw, binary.BigEndian, cf.StartOffset) binary.Write(bw, binary.BigEndian, cf.EndOffset) - titleFrame := TextFrame{ - Encoding: EncodingUTF8, - Text: cf.Title, + if cf.Title != nil { + writeFrame(bw, "TIT2", *cf.Title, true) + } + + if cf.SubTitle != nil { + writeFrame(bw, "TIT3", cf.SubTitle, true) } - writeFrame(bw, "TIT2", titleFrame, true) }) } func parseChapterFrame(br *bufReader) (Framer, error) { ElementID := br.ReadText(EncodingISO) - chapterTime := make([]int32, 2) - for i := range chapterTime { - if err := binary.Read(br, binary.BigEndian, &chapterTime[i]); err != nil { - return nil, err - } + var startTime uint32 + var startOffset uint32 + var endTime uint32 + var endOffset uint32 + + if err := binary.Read(br, binary.BigEndian, &startTime); err != nil { + return nil, err } - chapterOffset := make([]uint32, 2) - for i := range chapterOffset { - if err := binary.Read(br, binary.BigEndian, &chapterOffset[i]); err != nil { - return nil, err - } + if err := binary.Read(br, binary.BigEndian, &endTime); err != nil { + return nil, err + } + if err := binary.Read(br, binary.BigEndian, &startOffset); err != nil { + return nil, err } - var title string + if err := binary.Read(br, binary.BigEndian, &endOffset); err != nil { + return nil, err + } + + var title TextFrame + var subTitle TextFrame // borrowed from parse.go buf := getByteSlice(32 * 1024) @@ -95,7 +106,20 @@ func parseChapterFrame(br *bufReader) (Framer, error) { putLimitedReader(bodyRd) return nil, err } - title = frame.(TextFrame).Text + title = frame.(TextFrame) + + putLimitedReader(bodyRd) + break + } + if id == "TIT3" { + bodyRd := getLimitedReader(br, bodySize) + br3 := newBufReader(bodyRd) + frame, err := parseTextFrame(br3) + if err != nil { + putLimitedReader(bodyRd) + return nil, err + } + subTitle = frame.(TextFrame) putLimitedReader(bodyRd) break @@ -106,11 +130,12 @@ func parseChapterFrame(br *bufReader) (Framer, error) { ElementID: string(ElementID), // StartTime is given in milliseconds, so we should convert it to nanoseconds // for time.Duration - StartTime: time.Duration(chapterTime[0] * nanosInMillis), - EndTime: time.Duration(chapterTime[1] * nanosInMillis), - StartOffset: chapterOffset[0], - EndOffset: chapterOffset[1], - Title: title, + StartTime: time.Duration(int64(startTime) * nanosInMillis), + EndTime: time.Duration(int64(endTime) * nanosInMillis), + StartOffset: startOffset, + EndOffset: endOffset, + Title: &title, + SubTitle: &subTitle, } return cf, nil } diff --git a/chapter_frame_test.go b/chapter_frame_test.go new file mode 100644 index 0000000..ab7eb3b --- /dev/null +++ b/chapter_frame_test.go @@ -0,0 +1,63 @@ +package id3v2 + +import ( + "io" + "io/ioutil" + "log" + "os" + "testing" +) + +func TestAddChapterFrame(t *testing.T) { + src, err := os.Open("./testdata/test.mp3") + if err != nil { + t.Error(err) + } + defer src.Close() + + tmpFile, err := ioutil.TempFile("", "chapter_test") + if err != nil { + t.Error(err) + } + defer os.Remove(tmpFile.Name()) + + _, err = io.Copy(tmpFile, src) + if err != nil { + t.Error(err) + } + + tag, err := Open(tmpFile.Name(), Options{Parse: true}) + if tag == nil || err != nil { + log.Fatal("Error while opening mp3 file: ", err) + } + + chap := ChapterFrame{ + ElementID: "chap0", + StartTime: 0, + EndTime: 0, + StartOffset: 0, + EndOffset: 0, + TIT2SubFrame: &TextFrame{ + Encoding: EncodingUTF8, + Text: "chapter 0", + }, + } + tag.AddChapterFrame(chap) + + if err := tag.Save(); err != nil { + t.Error(err) + } + tag.Close() + + tag, err = Open(tmpFile.Name(), Options{Parse: true}) + if tag == nil || err != nil { + log.Fatal("Error while opening mp3 file: ", err) + } + frame := tag.GetLastFrame("CHAP").(ChapterFrame) + if frame.ElementID != "chap0" { + t.Error(err) + } + if frame.Title != "chapter 0" { + t.Errorf("expected: %s, but got %s", "chapter 0", frame.Title) + } +} diff --git a/test/test.go b/test/test.go new file mode 100644 index 0000000..1aecb5e --- /dev/null +++ b/test/test.go @@ -0,0 +1,30 @@ +package main + +import ( + "fmt" + "github.com/bogem/id3v2" + "log" + "time" +) + +func main() { + tag, err := id3v2.Open("/Users/r_takaishi/Projects/kimagurefm/kimagurefm-047/kimagurefm-047.mp3", id3v2.Options{Parse: true}) + if err != nil { + log.Fatal(err) + } + defer tag.Close() + + for _, frame := range tag.GetFrames("CHAP") { + cf := frame.(id3v2.ChapterFrame) + st := cf.StartTime + st = st.Round(time.Second) + h := st / time.Hour + st -= h * time.Hour + m := st / time.Minute + st -= m * time.Minute + s := st / time.Second + + fmt.Printf("%02d:%02d:%02d %s\n", h, m, s, cf.Title.Text) + + } +} From 47bf0f875db6c44f677dd7da3f127c9cf550a209 Mon Sep 17 00:00:00 2001 From: r_takaishi Date: Tue, 19 Jan 2021 18:38:58 +0900 Subject: [PATCH 03/23] update logic --- chapter_frame.go | 18 ++++++-- chapter_frame_test.go | 104 ++++++++++++++++++++++++++++++++++++++++-- test/test.go | 30 ------------ 3 files changed, 112 insertions(+), 40 deletions(-) delete mode 100644 test/test.go diff --git a/chapter_frame.go b/chapter_frame.go index b7e8a17..e675fc8 100644 --- a/chapter_frame.go +++ b/chapter_frame.go @@ -28,12 +28,20 @@ type ChapterFrame struct { } func (cf ChapterFrame) Size() int { - return encodedSize(cf.ElementID, EncodingISO) + + size := encodedSize(cf.ElementID, EncodingISO) + 1 + // trailing zero after ElementID - 4 + 4 + 4 + 4 + // (Start, End) (Time, Offset) - frameHeaderSize + // Title frame header size - cf.Title.Size() + - cf.SubTitle.Size() + 4 + 4 + 4 + 4 // (Start, End) (Time, Offset) + if cf.Title != nil { + size = size + + frameHeaderSize + // Title frame header size + cf.Title.Size() + } + if cf.SubTitle != nil { + size = size + + frameHeaderSize + // SubTitle frame header size + cf.SubTitle.Size() + } + return size } func (cf ChapterFrame) UniqueIdentifier() string { diff --git a/chapter_frame_test.go b/chapter_frame_test.go index ab7eb3b..345a234 100644 --- a/chapter_frame_test.go +++ b/chapter_frame_test.go @@ -6,25 +6,119 @@ import ( "log" "os" "testing" + "time" ) -func TestAddChapterFrame(t *testing.T) { +func prepareTestFile() (*os.File, error) { src, err := os.Open("./testdata/test.mp3") if err != nil { - t.Error(err) + return nil, err } defer src.Close() tmpFile, err := ioutil.TempFile("", "chapter_test") + if err != nil { + return nil, err + } + + _, err = io.Copy(tmpFile, src) + if err != nil { + return nil, err + } + return tmpFile, nil +} + +func TestAddChapterFrame(t *testing.T) { + tmpFile, err := prepareTestFile() if err != nil { t.Error(err) } defer os.Remove(tmpFile.Name()) - _, err = io.Copy(tmpFile, src) + tag, err := Open(tmpFile.Name(), Options{Parse: true}) + if tag == nil || err != nil { + log.Fatal("Error while opening mp3 file: ", err) + } + + chap := ChapterFrame{ + ElementID: "chap0", + StartTime: 0, + EndTime: time.Duration(1000 * nanosInMillis), + StartOffset: 0, + EndOffset: 0, + } + tag.AddChapterFrame(chap) + + if err := tag.Save(); err != nil { + t.Error(err) + } + tag.Close() + + tag, err = Open(tmpFile.Name(), Options{Parse: true}) + if tag == nil || err != nil { + log.Fatal("Error while opening mp3 file: ", err) + } + frame := tag.GetLastFrame("CHAP").(ChapterFrame) + if frame.ElementID != "chap0" { + t.Error(err) + } + if frame.StartTime != 0 { + t.Errorf("expected: %d, but got %s", 0, frame.StartTime) + } + if frame.EndTime.Milliseconds() != 1000 { + t.Errorf("expected: %d, but got %d", 1000, frame.EndTime.Milliseconds()) + } +} + +func TestAddChapterFrameWithTitle(t *testing.T) { + tmpFile, err := prepareTestFile() if err != nil { t.Error(err) } + defer os.Remove(tmpFile.Name()) + + tag, err := Open(tmpFile.Name(), Options{Parse: true}) + if tag == nil || err != nil { + log.Fatal("Error while opening mp3 file: ", err) + } + + chap := ChapterFrame{ + ElementID: "chap0", + StartTime: 0, + EndTime: 0, + StartOffset: 0, + EndOffset: 0, + Title: &TextFrame{ + Encoding: EncodingUTF8, + Text: "chapter 0", + }, + } + tag.AddChapterFrame(chap) + + if err := tag.Save(); err != nil { + t.Error(err) + } + tag.Close() + + tag, err = Open(tmpFile.Name(), Options{Parse: true}) + if tag == nil || err != nil { + log.Fatal("Error while opening mp3 file: ", err) + } + frame := tag.GetLastFrame("CHAP").(ChapterFrame) + if frame.ElementID != "chap0" { + t.Error(err) + } + if frame.Title.Text != "chapter 0" { + t.Errorf("expected: %s, but got %s", "chapter 0", frame.Title) + } +} + +func TestAddChapterFrameWithSubTitle(t *testing.T) { + tmpFile, err := prepareTestFile() + if err != nil { + t.Error(err) + } + defer os.Remove(tmpFile.Name()) tag, err := Open(tmpFile.Name(), Options{Parse: true}) if tag == nil || err != nil { @@ -37,7 +131,7 @@ func TestAddChapterFrame(t *testing.T) { EndTime: 0, StartOffset: 0, EndOffset: 0, - TIT2SubFrame: &TextFrame{ + SubTitle: &TextFrame{ Encoding: EncodingUTF8, Text: "chapter 0", }, @@ -57,7 +151,7 @@ func TestAddChapterFrame(t *testing.T) { if frame.ElementID != "chap0" { t.Error(err) } - if frame.Title != "chapter 0" { + if frame.SubTitle.Text != "chapter 0" { t.Errorf("expected: %s, but got %s", "chapter 0", frame.Title) } } diff --git a/test/test.go b/test/test.go deleted file mode 100644 index 1aecb5e..0000000 --- a/test/test.go +++ /dev/null @@ -1,30 +0,0 @@ -package main - -import ( - "fmt" - "github.com/bogem/id3v2" - "log" - "time" -) - -func main() { - tag, err := id3v2.Open("/Users/r_takaishi/Projects/kimagurefm/kimagurefm-047/kimagurefm-047.mp3", id3v2.Options{Parse: true}) - if err != nil { - log.Fatal(err) - } - defer tag.Close() - - for _, frame := range tag.GetFrames("CHAP") { - cf := frame.(id3v2.ChapterFrame) - st := cf.StartTime - st = st.Round(time.Second) - h := st / time.Hour - st -= h * time.Hour - m := st / time.Minute - st -= m * time.Minute - s := st / time.Second - - fmt.Printf("%02d:%02d:%02d %s\n", h, m, s, cf.Title.Text) - - } -} From 1bd15a35a9948089964161c1b9870720974537a3 Mon Sep 17 00:00:00 2001 From: r_takaishi Date: Tue, 19 Jan 2021 20:06:09 +0900 Subject: [PATCH 04/23] support old golang --- chapter_frame_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/chapter_frame_test.go b/chapter_frame_test.go index 345a234..c3a3ede 100644 --- a/chapter_frame_test.go +++ b/chapter_frame_test.go @@ -65,8 +65,8 @@ func TestAddChapterFrame(t *testing.T) { if frame.StartTime != 0 { t.Errorf("expected: %d, but got %s", 0, frame.StartTime) } - if frame.EndTime.Milliseconds() != 1000 { - t.Errorf("expected: %d, but got %d", 1000, frame.EndTime.Milliseconds()) + if frame.EndTime.Seconds()*1000 != 1000 { + t.Errorf("expected: %d, but got %d", 1000, int(frame.EndTime.Seconds()*1000)) } } From 478dc818c9be94d838dfe2da061f6891d36db02c Mon Sep 17 00:00:00 2001 From: r_takaishi Date: Thu, 25 Mar 2021 19:39:16 +0900 Subject: [PATCH 05/23] use `Description` called in spec --- chapter_frame.go | 18 +++++++++--------- chapter_frame_test.go | 11 +++++++---- 2 files changed, 16 insertions(+), 13 deletions(-) diff --git a/chapter_frame.go b/chapter_frame.go index e675fc8..c980a1b 100644 --- a/chapter_frame.go +++ b/chapter_frame.go @@ -24,7 +24,7 @@ type ChapterFrame struct { StartOffset uint32 EndOffset uint32 Title *TextFrame - SubTitle *TextFrame + Description *TextFrame } func (cf ChapterFrame) Size() int { @@ -36,10 +36,10 @@ func (cf ChapterFrame) Size() int { frameHeaderSize + // Title frame header size cf.Title.Size() } - if cf.SubTitle != nil { + if cf.Description != nil { size = size + - frameHeaderSize + // SubTitle frame header size - cf.SubTitle.Size() + frameHeaderSize + // Description frame header size + cf.Description.Size() } return size } @@ -63,8 +63,8 @@ func (cf ChapterFrame) WriteTo(w io.Writer) (n int64, err error) { writeFrame(bw, "TIT2", *cf.Title, true) } - if cf.SubTitle != nil { - writeFrame(bw, "TIT3", cf.SubTitle, true) + if cf.Description != nil { + writeFrame(bw, "TIT3", *cf.Description, true) } }) } @@ -90,7 +90,7 @@ func parseChapterFrame(br *bufReader) (Framer, error) { } var title TextFrame - var subTitle TextFrame + var description TextFrame // borrowed from parse.go buf := getByteSlice(32 * 1024) @@ -127,7 +127,7 @@ func parseChapterFrame(br *bufReader) (Framer, error) { putLimitedReader(bodyRd) return nil, err } - subTitle = frame.(TextFrame) + description = frame.(TextFrame) putLimitedReader(bodyRd) break @@ -143,7 +143,7 @@ func parseChapterFrame(br *bufReader) (Framer, error) { StartOffset: startOffset, EndOffset: endOffset, Title: &title, - SubTitle: &subTitle, + Description: &description, } return cf, nil } diff --git a/chapter_frame_test.go b/chapter_frame_test.go index c3a3ede..f6ed2bb 100644 --- a/chapter_frame_test.go +++ b/chapter_frame_test.go @@ -1,6 +1,7 @@ package id3v2 import ( + "fmt" "io" "io/ioutil" "log" @@ -113,7 +114,7 @@ func TestAddChapterFrameWithTitle(t *testing.T) { } } -func TestAddChapterFrameWithSubTitle(t *testing.T) { +func TestAddChapterFrameWithDescription(t *testing.T) { tmpFile, err := prepareTestFile() if err != nil { t.Error(err) @@ -131,7 +132,7 @@ func TestAddChapterFrameWithSubTitle(t *testing.T) { EndTime: 0, StartOffset: 0, EndOffset: 0, - SubTitle: &TextFrame{ + Description: &TextFrame{ Encoding: EncodingUTF8, Text: "chapter 0", }, @@ -151,7 +152,9 @@ func TestAddChapterFrameWithSubTitle(t *testing.T) { if frame.ElementID != "chap0" { t.Error(err) } - if frame.SubTitle.Text != "chapter 0" { - t.Errorf("expected: %s, but got %s", "chapter 0", frame.Title) + if frame.Description.Text != "chapter 0" { + fmt.Printf("%+v\n", frame) + fmt.Printf("%+v\n", frame.Description) + t.Errorf("expected: %s, but got %s", "chapter 0", frame.Description.Text) } } From cc723a79c7a9716eec94b5b85853090b3a5bfd00 Mon Sep 17 00:00:00 2001 From: r_takaishi Date: Thu, 25 Mar 2021 19:44:55 +0900 Subject: [PATCH 06/23] refactor: put together identical code --- chapter_frame.go | 24 ++++++++---------------- 1 file changed, 8 insertions(+), 16 deletions(-) diff --git a/chapter_frame.go b/chapter_frame.go index c980a1b..9915d82 100644 --- a/chapter_frame.go +++ b/chapter_frame.go @@ -106,28 +106,20 @@ func parseChapterFrame(br *bufReader) (Framer, error) { return nil, err } id, bodySize := header.ID, header.BodySize - if id == "TIT2" { + if id == "TIT2" || id == "TIT3" { bodyRd := getLimitedReader(br, bodySize) - br2 := newBufReader(bodyRd) - frame, err := parseTextFrame(br2) + br := newBufReader(bodyRd) + frame, err := parseTextFrame(br) if err != nil { putLimitedReader(bodyRd) return nil, err } - title = frame.(TextFrame) - - putLimitedReader(bodyRd) - break - } - if id == "TIT3" { - bodyRd := getLimitedReader(br, bodySize) - br3 := newBufReader(bodyRd) - frame, err := parseTextFrame(br3) - if err != nil { - putLimitedReader(bodyRd) - return nil, err + if id == "TIT2" { + title = frame.(TextFrame) + } + if id == "TIT3" { + description = frame.(TextFrame) } - description = frame.(TextFrame) putLimitedReader(bodyRd) break From 046dfba34a0ae9c699ba04ab7a3f1d16ff8ed7c9 Mon Sep 17 00:00:00 2001 From: r_takaishi Date: Thu, 25 Mar 2021 19:49:00 +0900 Subject: [PATCH 07/23] fix: should write title and description --- chapter_frame.go | 1 - chapter_frame_test.go | 54 ++++++++++++++++++++++++++++++++++++++++--- 2 files changed, 51 insertions(+), 4 deletions(-) diff --git a/chapter_frame.go b/chapter_frame.go index 9915d82..8075230 100644 --- a/chapter_frame.go +++ b/chapter_frame.go @@ -122,7 +122,6 @@ func parseChapterFrame(br *bufReader) (Framer, error) { } putLimitedReader(bodyRd) - break } } diff --git a/chapter_frame_test.go b/chapter_frame_test.go index f6ed2bb..81c597c 100644 --- a/chapter_frame_test.go +++ b/chapter_frame_test.go @@ -1,7 +1,6 @@ package id3v2 import ( - "fmt" "io" "io/ioutil" "log" @@ -29,6 +28,57 @@ func prepareTestFile() (*os.File, error) { return tmpFile, nil } +func TestAddChapterFrameWithTitleAndDescription(t *testing.T) { + tmpFile, err := prepareTestFile() + if err != nil { + t.Error(err) + } + defer os.Remove(tmpFile.Name()) + + tag, err := Open(tmpFile.Name(), Options{Parse: true}) + if tag == nil || err != nil { + log.Fatal("Error while opening mp3 file: ", err) + } + + chap := ChapterFrame{ + ElementID: "chap0", + StartTime: 0, + EndTime: time.Duration(1000 * nanosInMillis), + StartOffset: 0, + EndOffset: 0, + Title: &TextFrame{ + Encoding: EncodingUTF8, + Text: "chapter 0 title", + }, + Description: &TextFrame{ + Encoding: EncodingUTF8, + Text: "chapter 0 description", + }, + } + tag.AddChapterFrame(chap) + + if err := tag.Save(); err != nil { + t.Error(err) + } + tag.Close() + + tag, err = Open(tmpFile.Name(), Options{Parse: true}) + if tag == nil || err != nil { + log.Fatal("Error while opening mp3 file: ", err) + } + frame := tag.GetLastFrame("CHAP").(ChapterFrame) + if frame.ElementID != "chap0" { + t.Error(err) + } + if frame.Title.Text != "chapter 0 title" { + t.Errorf("expected: %s, but got %s", "chapter 0", frame.Title) + } + if frame.Description.Text != "chapter 0 description" { + t.Errorf("expected: %s, but got %s", "chapter 0", frame.Description.Text) + } + +} + func TestAddChapterFrame(t *testing.T) { tmpFile, err := prepareTestFile() if err != nil { @@ -153,8 +203,6 @@ func TestAddChapterFrameWithDescription(t *testing.T) { t.Error(err) } if frame.Description.Text != "chapter 0" { - fmt.Printf("%+v\n", frame) - fmt.Printf("%+v\n", frame.Description) t.Errorf("expected: %s, but got %s", "chapter 0", frame.Description.Text) } } From 8dad35890dafde5a3f59f2b417bf8480958dcf2c Mon Sep 17 00:00:00 2001 From: r_takaishi Date: Fri, 26 Mar 2021 17:28:52 +0900 Subject: [PATCH 08/23] refine test - test with TIT2 and TIT3 pattern - improve error message --- chapter_frame_test.go | 296 ++++++++++++++++++------------------------ 1 file changed, 125 insertions(+), 171 deletions(-) diff --git a/chapter_frame_test.go b/chapter_frame_test.go index 81c597c..e8b2fda 100644 --- a/chapter_frame_test.go +++ b/chapter_frame_test.go @@ -28,181 +28,135 @@ func prepareTestFile() (*os.File, error) { return tmpFile, nil } -func TestAddChapterFrameWithTitleAndDescription(t *testing.T) { - tmpFile, err := prepareTestFile() - if err != nil { - t.Error(err) - } - defer os.Remove(tmpFile.Name()) - - tag, err := Open(tmpFile.Name(), Options{Parse: true}) - if tag == nil || err != nil { - log.Fatal("Error while opening mp3 file: ", err) - } - - chap := ChapterFrame{ - ElementID: "chap0", - StartTime: 0, - EndTime: time.Duration(1000 * nanosInMillis), - StartOffset: 0, - EndOffset: 0, - Title: &TextFrame{ - Encoding: EncodingUTF8, - Text: "chapter 0 title", +func TestAddChapterFrame(t *testing.T) { + type fields struct { + ElementID string + StartTime time.Duration + EndTime time.Duration + StartOffset uint32 + EndOffset uint32 + Title *TextFrame + Description *TextFrame + } + tests := []struct { + name string + fields fields + wantElementId string + wantTitle string + wantDescription string + }{ + { + name: "", + fields: fields{ + ElementID: "chap0", + StartTime: 0, + EndTime: time.Duration(1000 * nanosInMillis), + StartOffset: 0, + EndOffset: 0, + }, + wantElementId: "chap0", + wantTitle: "", + wantDescription: "", }, - Description: &TextFrame{ - Encoding: EncodingUTF8, - Text: "chapter 0 description", + { + name: "", + fields: fields{ + ElementID: "chap0", + StartTime: 0, + EndTime: time.Duration(1000 * nanosInMillis), + StartOffset: 0, + EndOffset: 0, + Title: &TextFrame{ + Encoding: EncodingUTF8, + Text: "chapter 0", + }, + }, + wantElementId: "chap0", + wantTitle: "chapter 0", + wantDescription: "", }, - } - tag.AddChapterFrame(chap) - - if err := tag.Save(); err != nil { - t.Error(err) - } - tag.Close() - - tag, err = Open(tmpFile.Name(), Options{Parse: true}) - if tag == nil || err != nil { - log.Fatal("Error while opening mp3 file: ", err) - } - frame := tag.GetLastFrame("CHAP").(ChapterFrame) - if frame.ElementID != "chap0" { - t.Error(err) - } - if frame.Title.Text != "chapter 0 title" { - t.Errorf("expected: %s, but got %s", "chapter 0", frame.Title) - } - if frame.Description.Text != "chapter 0 description" { - t.Errorf("expected: %s, but got %s", "chapter 0", frame.Description.Text) - } - -} - -func TestAddChapterFrame(t *testing.T) { - tmpFile, err := prepareTestFile() - if err != nil { - t.Error(err) - } - defer os.Remove(tmpFile.Name()) - - tag, err := Open(tmpFile.Name(), Options{Parse: true}) - if tag == nil || err != nil { - log.Fatal("Error while opening mp3 file: ", err) - } - - chap := ChapterFrame{ - ElementID: "chap0", - StartTime: 0, - EndTime: time.Duration(1000 * nanosInMillis), - StartOffset: 0, - EndOffset: 0, - } - tag.AddChapterFrame(chap) - - if err := tag.Save(); err != nil { - t.Error(err) - } - tag.Close() - - tag, err = Open(tmpFile.Name(), Options{Parse: true}) - if tag == nil || err != nil { - log.Fatal("Error while opening mp3 file: ", err) - } - frame := tag.GetLastFrame("CHAP").(ChapterFrame) - if frame.ElementID != "chap0" { - t.Error(err) - } - if frame.StartTime != 0 { - t.Errorf("expected: %d, but got %s", 0, frame.StartTime) - } - if frame.EndTime.Seconds()*1000 != 1000 { - t.Errorf("expected: %d, but got %d", 1000, int(frame.EndTime.Seconds()*1000)) - } -} - -func TestAddChapterFrameWithTitle(t *testing.T) { - tmpFile, err := prepareTestFile() - if err != nil { - t.Error(err) - } - defer os.Remove(tmpFile.Name()) - - tag, err := Open(tmpFile.Name(), Options{Parse: true}) - if tag == nil || err != nil { - log.Fatal("Error while opening mp3 file: ", err) - } - - chap := ChapterFrame{ - ElementID: "chap0", - StartTime: 0, - EndTime: 0, - StartOffset: 0, - EndOffset: 0, - Title: &TextFrame{ - Encoding: EncodingUTF8, - Text: "chapter 0", + { + name: "", + fields: fields{ + ElementID: "chap0", + StartTime: 0, + EndTime: time.Duration(1000 * nanosInMillis), + StartOffset: 0, + EndOffset: 0, + Description: &TextFrame{ + Encoding: EncodingUTF8, + Text: "chapter 0", + }, + }, + wantElementId: "chap0", + wantTitle: "", + wantDescription: "chapter 0", }, - } - tag.AddChapterFrame(chap) - - if err := tag.Save(); err != nil { - t.Error(err) - } - tag.Close() - - tag, err = Open(tmpFile.Name(), Options{Parse: true}) - if tag == nil || err != nil { - log.Fatal("Error while opening mp3 file: ", err) - } - frame := tag.GetLastFrame("CHAP").(ChapterFrame) - if frame.ElementID != "chap0" { - t.Error(err) - } - if frame.Title.Text != "chapter 0" { - t.Errorf("expected: %s, but got %s", "chapter 0", frame.Title) - } -} - -func TestAddChapterFrameWithDescription(t *testing.T) { - tmpFile, err := prepareTestFile() - if err != nil { - t.Error(err) - } - defer os.Remove(tmpFile.Name()) - - tag, err := Open(tmpFile.Name(), Options{Parse: true}) - if tag == nil || err != nil { - log.Fatal("Error while opening mp3 file: ", err) - } - - chap := ChapterFrame{ - ElementID: "chap0", - StartTime: 0, - EndTime: 0, - StartOffset: 0, - EndOffset: 0, - Description: &TextFrame{ - Encoding: EncodingUTF8, - Text: "chapter 0", + { + name: "", + fields: fields{ + ElementID: "chap0", + StartTime: 0, + EndTime: time.Duration(1000 * nanosInMillis), + StartOffset: 0, + EndOffset: 0, + Title: &TextFrame{ + Encoding: EncodingUTF8, + Text: "chapter 0 title", + }, + Description: &TextFrame{ + Encoding: EncodingUTF8, + Text: "chapter 0 description", + }, + }, + wantElementId: "chap0", + wantTitle: "chapter 0 title", + wantDescription: "chapter 0 description", }, } - tag.AddChapterFrame(chap) - - if err := tag.Save(); err != nil { - t.Error(err) - } - tag.Close() - - tag, err = Open(tmpFile.Name(), Options{Parse: true}) - if tag == nil || err != nil { - log.Fatal("Error while opening mp3 file: ", err) - } - frame := tag.GetLastFrame("CHAP").(ChapterFrame) - if frame.ElementID != "chap0" { - t.Error(err) - } - if frame.Description.Text != "chapter 0" { - t.Errorf("expected: %s, but got %s", "chapter 0", frame.Description.Text) + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + tmpFile, err := prepareTestFile() + if err != nil { + t.Error(err) + } + defer os.Remove(tmpFile.Name()) + + tag, err := Open(tmpFile.Name(), Options{Parse: true}) + if tag == nil || err != nil { + log.Fatal("Error while opening mp3 file: ", err) + } + + cf := ChapterFrame{ + ElementID: tt.fields.ElementID, + StartTime: tt.fields.StartTime, + EndTime: tt.fields.EndTime, + StartOffset: tt.fields.StartOffset, + EndOffset: tt.fields.EndOffset, + Title: tt.fields.Title, + Description: tt.fields.Description, + } + tag.AddChapterFrame(cf) + + if err := tag.Save(); err != nil { + t.Error(err) + } + tag.Close() + + tag, err = Open(tmpFile.Name(), Options{Parse: true}) + if tag == nil || err != nil { + log.Fatal("Error while opening mp3 file: ", err) + } + frame := tag.GetLastFrame("CHAP").(ChapterFrame) + if frame.ElementID != tt.wantElementId { + t.Errorf("expected: %s, but got %s", tt.wantElementId, frame.ElementID) + } + if frame.Title.Text != tt.wantTitle { + t.Errorf("expected: %s, but got %s", tt.wantTitle, frame.Title) + } + if frame.Description.Text != tt.wantDescription { + t.Errorf("expected: %s, but got %s", tt.wantDescription, frame.Description.Text) + } + }) } } From edc860f6917e04cad8882ce4ccf1483e59a90f14 Mon Sep 17 00:00:00 2001 From: r_takaishi Date: Fri, 26 Mar 2021 17:55:29 +0900 Subject: [PATCH 09/23] if tag version is 4, syncSafe is enable --- chapter_frame.go | 12 ++++++++---- comment_frame.go | 2 +- common_ids.go | 2 +- encoding_test.go | 2 +- parse.go | 6 +++--- picture_frame.go | 2 +- popularimeter_frame.go | 2 +- ufid_frame.go | 2 +- unsynchronised_lyrics_frame.go | 2 +- user_defined_text_frame.go | 2 +- 10 files changed, 19 insertions(+), 15 deletions(-) diff --git a/chapter_frame.go b/chapter_frame.go index 8075230..4b7dda9 100644 --- a/chapter_frame.go +++ b/chapter_frame.go @@ -69,8 +69,9 @@ func (cf ChapterFrame) WriteTo(w io.Writer) (n int64, err error) { }) } -func parseChapterFrame(br *bufReader) (Framer, error) { +func parseChapterFrame(br *bufReader, version byte) (Framer, error) { ElementID := br.ReadText(EncodingISO) + var synchSafe bool var startTime uint32 var startOffset uint32 var endTime uint32 @@ -95,10 +96,13 @@ func parseChapterFrame(br *bufReader) (Framer, error) { // borrowed from parse.go buf := getByteSlice(32 * 1024) defer putByteSlice(buf) + if version == 4 { + synchSafe = true + } else { + synchSafe = false + } for { - // no way to determine whether this should be true or not - // this is likely should be fixed - header, err := parseFrameHeader(buf, br, true) + header, err := parseFrameHeader(buf, br, synchSafe) if err == io.EOF || err == errBlankFrame || err == ErrInvalidSizeFormat { break } diff --git a/comment_frame.go b/comment_frame.go index f78c1c2..2e399ad 100644 --- a/comment_frame.go +++ b/comment_frame.go @@ -42,7 +42,7 @@ func (cf CommentFrame) WriteTo(w io.Writer) (n int64, err error) { }) } -func parseCommentFrame(br *bufReader) (Framer, error) { +func parseCommentFrame(br *bufReader, version byte) (Framer, error) { encoding := getEncoding(br.ReadByte()) language := br.Next(3) description := br.ReadText(encoding) diff --git a/common_ids.go b/common_ids.go index 63cadb7..aeac14d 100644 --- a/common_ids.go +++ b/common_ids.go @@ -137,7 +137,7 @@ var ( // if strings.HasPrefix(id, "T") { // ... // } -var parsers = map[string]func(*bufReader) (Framer, error){ +var parsers = map[string]func(*bufReader, byte) (Framer, error){ "APIC": parsePictureFrame, "CHAP": parseChapterFrame, "COMM": parseCommentFrame, diff --git a/encoding_test.go b/encoding_test.go index f6ab722..6954b8d 100644 --- a/encoding_test.go +++ b/encoding_test.go @@ -75,7 +75,7 @@ func TestUnsynchronisedLyricsFrameWithUTF16(t *testing.T) { t.Fatal(err) } - parsed, err := parseUnsynchronisedLyricsFrame(newBufReader(buf)) + parsed, err := parseUnsynchronisedLyricsFrame(newBufReader(buf), 4) if err != nil { t.Fatal(err) } diff --git a/parse.go b/parse.go index 45d240d..2344f24 100644 --- a/parse.go +++ b/parse.go @@ -98,7 +98,7 @@ func (tag *Tag) parseFrames(opts Options) error { } br.Reset(bodyRd) - frame, err := parseFrameBody(id, br) + frame, err := parseFrameBody(id, br, tag.version) if err != nil && err != io.EOF { return err } @@ -174,13 +174,13 @@ func skipReaderBuf(rd io.Reader, buf []byte) error { return nil } -func parseFrameBody(id string, br *bufReader) (Framer, error) { +func parseFrameBody(id string, br *bufReader, version byte) (Framer, error) { if id[0] == 'T' && id != "TXXX" { return parseTextFrame(br) } if parseFunc, exists := parsers[id]; exists { - return parseFunc(br) + return parseFunc(br, version) } return parseUnknownFrame(br) diff --git a/picture_frame.go b/picture_frame.go index 4c9059e..57b0a45 100644 --- a/picture_frame.go +++ b/picture_frame.go @@ -42,7 +42,7 @@ func (pf PictureFrame) WriteTo(w io.Writer) (n int64, err error) { }) } -func parsePictureFrame(br *bufReader) (Framer, error) { +func parsePictureFrame(br *bufReader, version byte) (Framer, error) { encoding := getEncoding(br.ReadByte()) mimeType := br.ReadText(EncodingISO) pictureType := br.ReadByte() diff --git a/popularimeter_frame.go b/popularimeter_frame.go index c61d1a7..ee67385 100644 --- a/popularimeter_frame.go +++ b/popularimeter_frame.go @@ -50,7 +50,7 @@ func (pf PopularimeterFrame) WriteTo(w io.Writer) (n int64, err error) { }) } -func parsePopularimeterFrame(br *bufReader) (Framer, error) { +func parsePopularimeterFrame(br *bufReader, version byte) (Framer, error) { email := br.ReadText(EncodingISO) rating := br.ReadByte() diff --git a/ufid_frame.go b/ufid_frame.go index 9f83dc8..55530bb 100644 --- a/ufid_frame.go +++ b/ufid_frame.go @@ -24,7 +24,7 @@ func (ufid UFIDFrame) WriteTo(w io.Writer) (n int64, err error) { }) } -func parseUFIDFrame(br *bufReader) (Framer, error) { +func parseUFIDFrame(br *bufReader, version byte) (Framer, error) { owner := br.ReadText(EncodingISO) ident := br.ReadAll() diff --git a/unsynchronised_lyrics_frame.go b/unsynchronised_lyrics_frame.go index 09b86a0..ee48cb3 100644 --- a/unsynchronised_lyrics_frame.go +++ b/unsynchronised_lyrics_frame.go @@ -42,7 +42,7 @@ func (uslf UnsynchronisedLyricsFrame) WriteTo(w io.Writer) (n int64, err error) }) } -func parseUnsynchronisedLyricsFrame(br *bufReader) (Framer, error) { +func parseUnsynchronisedLyricsFrame(br *bufReader, version byte) (Framer, error) { encoding := getEncoding(br.ReadByte()) language := br.Next(3) contentDescriptor := br.ReadText(encoding) diff --git a/user_defined_text_frame.go b/user_defined_text_frame.go index f6cd917..a08ca6b 100644 --- a/user_defined_text_frame.go +++ b/user_defined_text_frame.go @@ -27,7 +27,7 @@ func (udtf UserDefinedTextFrame) WriteTo(w io.Writer) (n int64, err error) { }) } -func parseUserDefinedTextFrame(br *bufReader) (Framer, error) { +func parseUserDefinedTextFrame(br *bufReader, version byte) (Framer, error) { encoding := getEncoding(br.ReadByte()) description := br.ReadText(encoding) From a72682d4a939b9d1ad060b9615860afdfb0cb1bc Mon Sep 17 00:00:00 2001 From: r_takaishi Date: Wed, 21 Apr 2021 19:07:01 +0900 Subject: [PATCH 10/23] need test that has non-zero time and offset --- chapter_frame_test.go | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/chapter_frame_test.go b/chapter_frame_test.go index e8b2fda..984180a 100644 --- a/chapter_frame_test.go +++ b/chapter_frame_test.go @@ -113,6 +113,19 @@ func TestAddChapterFrame(t *testing.T) { wantTitle: "chapter 0 title", wantDescription: "chapter 0 description", }, + { + name: "", + fields: fields{ + ElementID: "chap0", + StartTime: time.Duration(1000 * nanosInMillis), + EndTime: time.Duration(1000 * nanosInMillis), + StartOffset: 10, + EndOffset: 10, + }, + wantElementId: "chap0", + wantTitle: "", + wantDescription: "", + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -157,6 +170,18 @@ func TestAddChapterFrame(t *testing.T) { if frame.Description.Text != tt.wantDescription { t.Errorf("expected: %s, but got %s", tt.wantDescription, frame.Description.Text) } + if frame.StartTime != tt.fields.StartTime { + t.Errorf("expected: %s, but got %s", tt.fields.StartTime, frame.StartTime) + } + if frame.EndTime != tt.fields.EndTime { + t.Errorf("expected: %s, but got %s", tt.fields.EndTime, frame.EndTime) + } + if frame.StartOffset != tt.fields.StartOffset { + t.Errorf("expected: %d, but got %d", tt.fields.StartOffset, frame.StartOffset) + } + if frame.EndOffset != tt.fields.EndOffset { + t.Errorf("expected: %d, but got %d", tt.fields.EndOffset, frame.EndOffset) + } }) } } From 10c8d21c5fffb95edf29992022f48567d79d399a Mon Sep 17 00:00:00 2001 From: r_takaishi Date: Wed, 21 Apr 2021 19:24:42 +0900 Subject: [PATCH 11/23] use `else if` --- chapter_frame.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/chapter_frame.go b/chapter_frame.go index 4b7dda9..1e05cb2 100644 --- a/chapter_frame.go +++ b/chapter_frame.go @@ -120,8 +120,7 @@ func parseChapterFrame(br *bufReader, version byte) (Framer, error) { } if id == "TIT2" { title = frame.(TextFrame) - } - if id == "TIT3" { + } else if id == "TIT3" { description = frame.(TextFrame) } From c1ed23b3e4281bdbbe233be60efaf1e5f3395b1a Mon Sep 17 00:00:00 2001 From: r_takaishi Date: Wed, 21 Apr 2021 19:25:37 +0900 Subject: [PATCH 12/23] add name --- chapter_frame_test.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/chapter_frame_test.go b/chapter_frame_test.go index 984180a..d2242f4 100644 --- a/chapter_frame_test.go +++ b/chapter_frame_test.go @@ -46,7 +46,7 @@ func TestAddChapterFrame(t *testing.T) { wantDescription string }{ { - name: "", + name: "element id only", fields: fields{ ElementID: "chap0", StartTime: 0, @@ -59,7 +59,7 @@ func TestAddChapterFrame(t *testing.T) { wantDescription: "", }, { - name: "", + name: "with title", fields: fields{ ElementID: "chap0", StartTime: 0, @@ -76,7 +76,7 @@ func TestAddChapterFrame(t *testing.T) { wantDescription: "", }, { - name: "", + name: "with description", fields: fields{ ElementID: "chap0", StartTime: 0, @@ -93,7 +93,7 @@ func TestAddChapterFrame(t *testing.T) { wantDescription: "chapter 0", }, { - name: "", + name: "with title and description", fields: fields{ ElementID: "chap0", StartTime: 0, @@ -114,7 +114,7 @@ func TestAddChapterFrame(t *testing.T) { wantDescription: "chapter 0 description", }, { - name: "", + name: "non-zero time and offset", fields: fields{ ElementID: "chap0", StartTime: time.Duration(1000 * nanosInMillis), From a9af7578948f4cc65253e4197b4b9e3e8d41fa1a Mon Sep 17 00:00:00 2001 From: r_takaishi Date: Sun, 21 Nov 2021 10:26:11 +0900 Subject: [PATCH 13/23] make more simple --- chapter_frame.go | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/chapter_frame.go b/chapter_frame.go index 1e05cb2..5d6785c 100644 --- a/chapter_frame.go +++ b/chapter_frame.go @@ -71,7 +71,7 @@ func (cf ChapterFrame) WriteTo(w io.Writer) (n int64, err error) { func parseChapterFrame(br *bufReader, version byte) (Framer, error) { ElementID := br.ReadText(EncodingISO) - var synchSafe bool + synchSafe := version == 4 var startTime uint32 var startOffset uint32 var endTime uint32 @@ -96,11 +96,7 @@ func parseChapterFrame(br *bufReader, version byte) (Framer, error) { // borrowed from parse.go buf := getByteSlice(32 * 1024) defer putByteSlice(buf) - if version == 4 { - synchSafe = true - } else { - synchSafe = false - } + for { header, err := parseFrameHeader(buf, br, synchSafe) if err == io.EOF || err == errBlankFrame || err == ErrInvalidSizeFormat { From 4930cb2c9bbc861d958c1b294b6ba5e0a35399d5 Mon Sep 17 00:00:00 2001 From: r_takaishi Date: Sun, 21 Nov 2021 10:28:10 +0900 Subject: [PATCH 14/23] better to use lowercase letter --- chapter_frame.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/chapter_frame.go b/chapter_frame.go index 5d6785c..65481a0 100644 --- a/chapter_frame.go +++ b/chapter_frame.go @@ -70,7 +70,7 @@ func (cf ChapterFrame) WriteTo(w io.Writer) (n int64, err error) { } func parseChapterFrame(br *bufReader, version byte) (Framer, error) { - ElementID := br.ReadText(EncodingISO) + elementID := br.ReadText(EncodingISO) synchSafe := version == 4 var startTime uint32 var startOffset uint32 @@ -125,7 +125,7 @@ func parseChapterFrame(br *bufReader, version byte) (Framer, error) { } cf := ChapterFrame{ - ElementID: string(ElementID), + ElementID: string(elementID), // StartTime is given in milliseconds, so we should convert it to nanoseconds // for time.Duration StartTime: time.Duration(int64(startTime) * nanosInMillis), From 3805151483b2048681cef8c26bd55c1aaa253f8a Mon Sep 17 00:00:00 2001 From: r_takaishi Date: Sun, 21 Nov 2021 10:29:13 +0900 Subject: [PATCH 15/23] remove comment --- chapter_frame.go | 1 - 1 file changed, 1 deletion(-) diff --git a/chapter_frame.go b/chapter_frame.go index 65481a0..801e584 100644 --- a/chapter_frame.go +++ b/chapter_frame.go @@ -52,7 +52,6 @@ func (cf ChapterFrame) WriteTo(w io.Writer) (n int64, err error) { return useBufWriter(w, func(bw *bufWriter) { bw.EncodeAndWriteText(cf.ElementID, EncodingISO) bw.WriteByte(0) - // nanoseconds => milliseconds binary.Write(bw, binary.BigEndian, int32(cf.StartTime/nanosInMillis)) binary.Write(bw, binary.BigEndian, int32(cf.EndTime/nanosInMillis)) From 0867a7f8bb65bf4b74961efa8e337bf4be4059ca Mon Sep 17 00:00:00 2001 From: Ryo Takaishi Date: Sun, 21 Nov 2021 10:30:28 +0900 Subject: [PATCH 16/23] Update chapter_frame_test.go More clarity message Co-authored-by: Albert Nigmatzianov --- chapter_frame_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/chapter_frame_test.go b/chapter_frame_test.go index d2242f4..dafb4c8 100644 --- a/chapter_frame_test.go +++ b/chapter_frame_test.go @@ -162,7 +162,7 @@ func TestAddChapterFrame(t *testing.T) { } frame := tag.GetLastFrame("CHAP").(ChapterFrame) if frame.ElementID != tt.wantElementId { - t.Errorf("expected: %s, but got %s", tt.wantElementId, frame.ElementID) + t.Errorf("Expected element ID: %s, but got %s", tt.wantElementId, frame.ElementID) } if frame.Title.Text != tt.wantTitle { t.Errorf("expected: %s, but got %s", tt.wantTitle, frame.Title) From 8ba656f3bddfc492fcd6794fe50fefeb2d6c93ac Mon Sep 17 00:00:00 2001 From: Ryo Takaishi Date: Sun, 21 Nov 2021 10:30:39 +0900 Subject: [PATCH 17/23] Update chapter_frame_test.go More clarity message Co-authored-by: Albert Nigmatzianov --- chapter_frame_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/chapter_frame_test.go b/chapter_frame_test.go index dafb4c8..8a8739a 100644 --- a/chapter_frame_test.go +++ b/chapter_frame_test.go @@ -165,7 +165,7 @@ func TestAddChapterFrame(t *testing.T) { t.Errorf("Expected element ID: %s, but got %s", tt.wantElementId, frame.ElementID) } if frame.Title.Text != tt.wantTitle { - t.Errorf("expected: %s, but got %s", tt.wantTitle, frame.Title) + t.Errorf("Expected title: %s, but got %s", tt.wantTitle, frame.Title) } if frame.Description.Text != tt.wantDescription { t.Errorf("expected: %s, but got %s", tt.wantDescription, frame.Description.Text) From f06deed13bf0ffa20d070545db531c659bcc0ef3 Mon Sep 17 00:00:00 2001 From: Ryo Takaishi Date: Sun, 21 Nov 2021 10:30:47 +0900 Subject: [PATCH 18/23] Update chapter_frame_test.go More clarity message Co-authored-by: Albert Nigmatzianov --- chapter_frame_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/chapter_frame_test.go b/chapter_frame_test.go index 8a8739a..cbc9328 100644 --- a/chapter_frame_test.go +++ b/chapter_frame_test.go @@ -177,7 +177,7 @@ func TestAddChapterFrame(t *testing.T) { t.Errorf("expected: %s, but got %s", tt.fields.EndTime, frame.EndTime) } if frame.StartOffset != tt.fields.StartOffset { - t.Errorf("expected: %d, but got %d", tt.fields.StartOffset, frame.StartOffset) + t.Errorf("Expected start offset: %d, but got %d", tt.fields.StartOffset, frame.StartOffset) } if frame.EndOffset != tt.fields.EndOffset { t.Errorf("expected: %d, but got %d", tt.fields.EndOffset, frame.EndOffset) From 1cddd5881745d229df4ea2406712783eff839543 Mon Sep 17 00:00:00 2001 From: Ryo Takaishi Date: Sun, 21 Nov 2021 10:31:02 +0900 Subject: [PATCH 19/23] Update chapter_frame_test.go More clarity message Co-authored-by: Albert Nigmatzianov --- chapter_frame_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/chapter_frame_test.go b/chapter_frame_test.go index cbc9328..abcb65b 100644 --- a/chapter_frame_test.go +++ b/chapter_frame_test.go @@ -180,7 +180,7 @@ func TestAddChapterFrame(t *testing.T) { t.Errorf("Expected start offset: %d, but got %d", tt.fields.StartOffset, frame.StartOffset) } if frame.EndOffset != tt.fields.EndOffset { - t.Errorf("expected: %d, but got %d", tt.fields.EndOffset, frame.EndOffset) + t.Errorf("Expected end offset: %d, but got %d", tt.fields.EndOffset, frame.EndOffset) } }) } From 8449f138cb64a43549c6afe04d0028cb23f1596b Mon Sep 17 00:00:00 2001 From: Ryo Takaishi Date: Sun, 21 Nov 2021 10:33:51 +0900 Subject: [PATCH 20/23] Update chapter_frame_test.go More clarity message Co-authored-by: Albert Nigmatzianov --- chapter_frame_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/chapter_frame_test.go b/chapter_frame_test.go index abcb65b..35e966d 100644 --- a/chapter_frame_test.go +++ b/chapter_frame_test.go @@ -168,7 +168,7 @@ func TestAddChapterFrame(t *testing.T) { t.Errorf("Expected title: %s, but got %s", tt.wantTitle, frame.Title) } if frame.Description.Text != tt.wantDescription { - t.Errorf("expected: %s, but got %s", tt.wantDescription, frame.Description.Text) + t.Errorf("Expected description: %s, but got %s", tt.wantDescription, frame.Description.Text) } if frame.StartTime != tt.fields.StartTime { t.Errorf("expected: %s, but got %s", tt.fields.StartTime, frame.StartTime) From ac6723302fe4a9bf7832c63ef08e9bc875e4c7dd Mon Sep 17 00:00:00 2001 From: Ryo Takaishi Date: Sun, 21 Nov 2021 10:34:30 +0900 Subject: [PATCH 21/23] Update chapter_frame_test.go More clarity message Co-authored-by: Albert Nigmatzianov --- chapter_frame_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/chapter_frame_test.go b/chapter_frame_test.go index 35e966d..e7128b9 100644 --- a/chapter_frame_test.go +++ b/chapter_frame_test.go @@ -171,7 +171,7 @@ func TestAddChapterFrame(t *testing.T) { t.Errorf("Expected description: %s, but got %s", tt.wantDescription, frame.Description.Text) } if frame.StartTime != tt.fields.StartTime { - t.Errorf("expected: %s, but got %s", tt.fields.StartTime, frame.StartTime) + t.Errorf("Expected start time: %s, but got %s", tt.fields.StartTime, frame.StartTime) } if frame.EndTime != tt.fields.EndTime { t.Errorf("expected: %s, but got %s", tt.fields.EndTime, frame.EndTime) From 531ce04e80112eff38eb71c4f64cf81fccc254f2 Mon Sep 17 00:00:00 2001 From: Ryo Takaishi Date: Sun, 21 Nov 2021 10:34:47 +0900 Subject: [PATCH 22/23] Update chapter_frame_test.go More clarity message Co-authored-by: Albert Nigmatzianov --- chapter_frame_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/chapter_frame_test.go b/chapter_frame_test.go index e7128b9..7a5e1de 100644 --- a/chapter_frame_test.go +++ b/chapter_frame_test.go @@ -174,7 +174,7 @@ func TestAddChapterFrame(t *testing.T) { t.Errorf("Expected start time: %s, but got %s", tt.fields.StartTime, frame.StartTime) } if frame.EndTime != tt.fields.EndTime { - t.Errorf("expected: %s, but got %s", tt.fields.EndTime, frame.EndTime) + t.Errorf("Expected end time: %s, but got %s", tt.fields.EndTime, frame.EndTime) } if frame.StartOffset != tt.fields.StartOffset { t.Errorf("Expected start offset: %d, but got %d", tt.fields.StartOffset, frame.StartOffset) From bbcbdfef76efd444781385a1961d6b916d586d14 Mon Sep 17 00:00:00 2001 From: r_takaishi Date: Sun, 21 Nov 2021 10:41:50 +0900 Subject: [PATCH 23/23] use tt.fields.* as StartTime or EndTime --- chapter_frame_test.go | 34 ++++++++-------------------------- 1 file changed, 8 insertions(+), 26 deletions(-) diff --git a/chapter_frame_test.go b/chapter_frame_test.go index 7a5e1de..8068b38 100644 --- a/chapter_frame_test.go +++ b/chapter_frame_test.go @@ -39,11 +39,8 @@ func TestAddChapterFrame(t *testing.T) { Description *TextFrame } tests := []struct { - name string - fields fields - wantElementId string - wantTitle string - wantDescription string + name string + fields fields }{ { name: "element id only", @@ -54,9 +51,6 @@ func TestAddChapterFrame(t *testing.T) { StartOffset: 0, EndOffset: 0, }, - wantElementId: "chap0", - wantTitle: "", - wantDescription: "", }, { name: "with title", @@ -71,9 +65,6 @@ func TestAddChapterFrame(t *testing.T) { Text: "chapter 0", }, }, - wantElementId: "chap0", - wantTitle: "chapter 0", - wantDescription: "", }, { name: "with description", @@ -88,9 +79,6 @@ func TestAddChapterFrame(t *testing.T) { Text: "chapter 0", }, }, - wantElementId: "chap0", - wantTitle: "", - wantDescription: "chapter 0", }, { name: "with title and description", @@ -109,9 +97,6 @@ func TestAddChapterFrame(t *testing.T) { Text: "chapter 0 description", }, }, - wantElementId: "chap0", - wantTitle: "chapter 0 title", - wantDescription: "chapter 0 description", }, { name: "non-zero time and offset", @@ -122,9 +107,6 @@ func TestAddChapterFrame(t *testing.T) { StartOffset: 10, EndOffset: 10, }, - wantElementId: "chap0", - wantTitle: "", - wantDescription: "", }, } for _, tt := range tests { @@ -161,14 +143,14 @@ func TestAddChapterFrame(t *testing.T) { log.Fatal("Error while opening mp3 file: ", err) } frame := tag.GetLastFrame("CHAP").(ChapterFrame) - if frame.ElementID != tt.wantElementId { - t.Errorf("Expected element ID: %s, but got %s", tt.wantElementId, frame.ElementID) + if frame.ElementID != tt.fields.ElementID { + t.Errorf("Expected element ID: %s, but got %s", tt.fields.ElementID, frame.ElementID) } - if frame.Title.Text != tt.wantTitle { - t.Errorf("Expected title: %s, but got %s", tt.wantTitle, frame.Title) + if tt.fields.Title != nil && frame.Title.Text != tt.fields.Title.Text { + t.Errorf("Expected title: %s, but got %s", tt.fields.Title.Text, frame.Title) } - if frame.Description.Text != tt.wantDescription { - t.Errorf("Expected description: %s, but got %s", tt.wantDescription, frame.Description.Text) + if tt.fields.Description != nil && frame.Description.Text != tt.fields.Description.Text { + t.Errorf("Expected description: %s, but got %s", tt.fields.Description.Text, frame.Description.Text) } if frame.StartTime != tt.fields.StartTime { t.Errorf("Expected start time: %s, but got %s", tt.fields.StartTime, frame.StartTime)