-
Notifications
You must be signed in to change notification settings - Fork 51
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
First implementation of chapter TOC #72
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||
---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,141 @@ | ||||||||
package id3v2 | ||||||||
|
||||||||
import ( | ||||||||
"encoding/binary" | ||||||||
"errors" | ||||||||
"fmt" | ||||||||
"io" | ||||||||
) | ||||||||
|
||||||||
const ( | ||||||||
maskOrdered = byte(1 << 0) | ||||||||
maskToplevel = byte(1 << 1) | ||||||||
) | ||||||||
|
||||||||
var ErrUnexpectedId = errors.New("unexpected ID") | ||||||||
|
||||||||
type ChapterTocFrame struct { | ||||||||
ElementID string | ||||||||
// This frame is the root of the Table of Contents tree and is not a child of any other "CTOC" frame. | ||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||
TopLevel bool | ||||||||
// This provides a hint as to whether the elements should be played as a continuous ordered sequence or played individually. | ||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||
Ordered bool | ||||||||
ChapterIds []string | ||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. IMO it's better to name properties the same as they named in the spec. Also good idea to provide comment for it:
Suggested change
|
||||||||
Description *TextFrame | ||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it should be named as
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think my implementation is wrong. After reading the spec again: TIT2 and TIT3 can be added, so it's a collection of text frames rather than one single text frame. |
||||||||
} | ||||||||
|
||||||||
func (ctf ChapterTocFrame) Size() int { | ||||||||
size := encodedSize(ctf.ElementID, EncodingISO) | ||||||||
size += 1 // trailing zero after ElementID | ||||||||
size += 1 // CTOC Flags | ||||||||
// The Entry count is the number of entries in the Child Element ID | ||||||||
// list that follows and must be greater than zero. | ||||||||
size += 1 // Entrycount | ||||||||
|
||||||||
// entries | ||||||||
for _, id := range ctf.ChapterIds { | ||||||||
size += encodedSize(id, EncodingISO) | ||||||||
size += 1 // trailing zero after ID | ||||||||
} | ||||||||
|
||||||||
// (optional) descriptive data | ||||||||
if ctf.Description != nil { | ||||||||
size += frameHeaderSize // Description frame header size | ||||||||
size += ctf.Description.Size() | ||||||||
} | ||||||||
|
||||||||
return size | ||||||||
} | ||||||||
|
||||||||
func (ctf ChapterTocFrame) UniqueIdentifier() string { | ||||||||
return ctf.ElementID | ||||||||
} | ||||||||
|
||||||||
func (ctf ChapterTocFrame) WriteTo(w io.Writer) (n int64, err error) { | ||||||||
return useBufWriter(w, func(bw *bufWriter) { | ||||||||
bw.EncodeAndWriteText(ctf.ElementID, EncodingISO) | ||||||||
bw.WriteByte(0) | ||||||||
|
||||||||
ctocFlags := byte(0) | ||||||||
if ctf.TopLevel { | ||||||||
ctocFlags |= maskToplevel | ||||||||
} | ||||||||
if ctf.Ordered { | ||||||||
ctocFlags |= maskOrdered | ||||||||
} | ||||||||
|
||||||||
binary.Write(bw, binary.BigEndian, ctocFlags) | ||||||||
|
||||||||
binary.Write(bw, binary.BigEndian, uint8(len(ctf.ChapterIds))) | ||||||||
|
||||||||
for _, id := range ctf.ChapterIds { | ||||||||
bw.EncodeAndWriteText(id, EncodingISO) | ||||||||
bw.WriteByte(0) | ||||||||
} | ||||||||
|
||||||||
if ctf.Description != nil { | ||||||||
writeFrame(bw, "TIT2", *ctf.Description, true) | ||||||||
} | ||||||||
}) | ||||||||
} | ||||||||
|
||||||||
func parseChapterTocFrame(br *bufReader, version byte) (Framer, error) { | ||||||||
elementID := string(br.ReadText(EncodingISO)) | ||||||||
synchSafe := version == 4 | ||||||||
var ctocFlags byte | ||||||||
if err := binary.Read(br, binary.BigEndian, &ctocFlags); err != nil { | ||||||||
return nil, err | ||||||||
} | ||||||||
|
||||||||
var elements uint8 | ||||||||
if err := binary.Read(br, binary.BigEndian, &elements); err != nil { | ||||||||
return nil, err | ||||||||
} | ||||||||
|
||||||||
chaptersIDs := make([]string, elements) | ||||||||
for i := uint8(0); i < elements; i++ { | ||||||||
chaptersIDs[i] = string(br.ReadText(EncodingISO)) | ||||||||
} | ||||||||
|
||||||||
var description TextFrame | ||||||||
|
||||||||
// borrowed from parse.go | ||||||||
buf := getByteSlice(32 * 1024) | ||||||||
defer putByteSlice(buf) | ||||||||
|
||||||||
for { | ||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As far as I understand, you're parsing DescriptiveData here. You're using for-loop, but AFAIU there can be only one DescriptiveData, so you don't need loop, right? |
||||||||
header, err := parseFrameHeader(buf, br, synchSafe) | ||||||||
if err == io.EOF || err == errBlankFrame || err == ErrInvalidSizeFormat { | ||||||||
break | ||||||||
} | ||||||||
|
||||||||
if err != nil { | ||||||||
return nil, err | ||||||||
} | ||||||||
|
||||||||
if header.ID != "TIT2" { | ||||||||
return nil, fmt.Errorf("expected: '%s', got: '%s' : %w", "TIT2", header.ID, ErrUnexpectedId) | ||||||||
} | ||||||||
|
||||||||
bodyRd := getLimitedReader(br, header.BodySize) | ||||||||
br := newBufReader(bodyRd) | ||||||||
frame, err := parseTextFrame(br) | ||||||||
if err != nil { | ||||||||
putLimitedReader(bodyRd) | ||||||||
return nil, err | ||||||||
} | ||||||||
description = frame.(TextFrame) | ||||||||
|
||||||||
putLimitedReader(bodyRd) | ||||||||
} | ||||||||
|
||||||||
tocFrame := ChapterTocFrame{ | ||||||||
ElementID: elementID, | ||||||||
TopLevel: (ctocFlags & maskToplevel) == maskToplevel, | ||||||||
Ordered: (ctocFlags & maskOrdered) == maskOrdered, | ||||||||
ChapterIds: chaptersIDs, | ||||||||
Description: &description, | ||||||||
} | ||||||||
|
||||||||
return tocFrame, nil | ||||||||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,116 @@ | ||
package id3v2 | ||
|
||
import ( | ||
"bytes" | ||
"fmt" | ||
"log" | ||
"testing" | ||
"time" | ||
) | ||
|
||
const ( | ||
testChapterTocSampleTitle = "Chapter TOC title" | ||
) | ||
|
||
func newChapterFrames(noOfChapters int) []ChapterFrame { | ||
var start time.Duration | ||
offset := time.Duration(1000 * nanosInMillis) | ||
|
||
chapters := make([]ChapterFrame, noOfChapters) | ||
|
||
for i := 0; i < noOfChapters; i++ { | ||
end := start + offset | ||
|
||
chapters[i] = ChapterFrame{ | ||
ElementID: fmt.Sprintf("ch%d", i), | ||
StartTime: start, | ||
EndTime: end, | ||
StartOffset: IgnoredOffset, | ||
EndOffset: IgnoredOffset, | ||
Title: &TextFrame{ | ||
Encoding: EncodingUTF8, | ||
Text: fmt.Sprintf("Chapter %d", i), | ||
}, | ||
} | ||
|
||
start = end | ||
} | ||
|
||
return chapters | ||
} | ||
|
||
func TestAddChapterTocFrame(t *testing.T) { | ||
const noOfChapters = 5 | ||
buf := &bytes.Buffer{} | ||
tag := NewEmptyTag() | ||
|
||
chapters := newChapterFrames(noOfChapters) | ||
|
||
chapterIds := make([]string, len(chapters)) | ||
for i, c := range chapters { | ||
tag.AddChapterFrame(c) | ||
|
||
chapterIds[i] = c.ElementID | ||
} | ||
|
||
chapterToc := ChapterTocFrame{ | ||
ElementID: "Main TOC", | ||
TopLevel: true, | ||
Ordered: true, | ||
ChapterIds: chapterIds, | ||
Description: &TextFrame{ | ||
Encoding: EncodingUTF8, | ||
Text: testChapterTocSampleTitle, | ||
}, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nice! |
||
} | ||
|
||
tag.AddChapterTocFrame(chapterToc) | ||
tag.WriteTo(buf) | ||
|
||
// Read back | ||
|
||
tagBack, err := ParseReader(buf, Options{Parse: true}) | ||
if err != nil { | ||
log.Fatal("Error parsing mp3 content: ", err) | ||
} | ||
|
||
if !tagBack.HasFrames() { | ||
log.Fatal("No tags in content in mp3 content") | ||
} | ||
|
||
chapterTocBackFrame := tag.GetLastFrame("CTOC") | ||
if chapterTocBackFrame == nil { | ||
log.Fatal("Error getting chapter TOC frame: ", err) | ||
} | ||
|
||
chapterTocBack, ok := chapterTocBackFrame.(ChapterTocFrame) | ||
if !ok { | ||
log.Fatal("Error casting chapter TOC frame") | ||
} | ||
|
||
if chapterToc.ElementID != chapterTocBack.ElementID { | ||
t.Errorf("Expected element ID: %s, but got %s", chapterToc.ElementID, chapterTocBack.ElementID) | ||
} | ||
|
||
if chapterToc.TopLevel != chapterTocBack.TopLevel { | ||
t.Errorf("Expected top level: %v, but got %v", chapterToc.TopLevel, chapterTocBack.TopLevel) | ||
} | ||
|
||
if chapterToc.Ordered != chapterTocBack.Ordered { | ||
t.Errorf("Expected ordered: %v, but got %v", chapterToc.Ordered, chapterTocBack.Ordered) | ||
} | ||
|
||
if expected, actual := len(chapterToc.ChapterIds), len(chapterTocBack.ChapterIds); expected != actual { | ||
t.Errorf("Expected ordered: %v, but got %v", expected, actual) | ||
} | ||
|
||
for i := 0; i < len(chapterToc.ChapterIds); i++ { | ||
if expected, actual := chapterToc.ChapterIds[i], chapterTocBack.ChapterIds[i]; expected != actual { | ||
t.Errorf("Expected chapter reference at index: %d: %s, but got %s", i, expected, actual) | ||
} | ||
} | ||
|
||
if chapterToc.Description != nil && chapterToc.Description.Text != chapterTocBack.Description.Text { | ||
t.Errorf("Expected description: %s, but got %s", chapterToc.Description.Text, chapterTocBack.Description.Text) | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As far as I see this change is in the v1 file, but actually all changes should go to v2 folder and you already made them in v2/tag.go, so please delete it :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
However,
AddChapterFrame
is used in V1:chapter_frame_test.go
. Either remove the test or the implementation.