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

support CHAP frames #62

Merged
merged 24 commits into from Nov 23, 2021
Merged

support CHAP frames #62

merged 24 commits into from Nov 23, 2021

Conversation

takaishi
Copy link
Contributor

@takaishi takaishi commented Jan 19, 2021

Support to add and parse CHAP frames according to https://id3.org/id3v2-chapters-1.0 .
Similar PullRequest already opened (#40), but it looks like doesn't proceed.
I respect it's commit and update in this PullRequest.

@takaishi
Copy link
Contributor Author

takaishi commented Feb 7, 2021

@BoGeM Hello, could you review this PR?

@beautifulentropy
Copy link

@BoGeM Hello, could you review this PR?

Just passing by; excellent work on this!

Copy link
Owner

@n10v n10v left a comment

Choose a reason for hiding this comment

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

Hello @takaishi,
thank you for the PR and sorry for so big delay. It's awesome functionality you implemented! Please take a look at my comments :)

chapter_frame.go Outdated
buf := getByteSlice(32 * 1024)
defer putByteSlice(buf)
for {
// no way to determine whether this should be true or not
Copy link
Owner

@n10v n10v Mar 23, 2021

Choose a reason for hiding this comment

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

synchSafe is true when tag version is 4. You have to add an argument synchSafe bool to all parser functions and provide tag.Version() === 4 to them like here

chapter_frame.go Show resolved Hide resolved
chapter_frame.go Outdated Show resolved Hide resolved
chapter_frame.go Outdated Show resolved Hide resolved
chapter_frame_test.go Outdated Show resolved Hide resolved
chapter_frame_test.go Outdated Show resolved Hide resolved
chapter_frame_test.go Outdated Show resolved Hide resolved
chapter_frame_test.go Outdated Show resolved Hide resolved
chapter_frame_test.go Show resolved Hide resolved
@takaishi
Copy link
Contributor Author

@BoGeM Hello, I added commit. Could you review again?

Copy link
Owner

@n10v n10v left a comment

Choose a reason for hiding this comment

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

Hi @takaishi,
I'm super sorry for so huge delay. I left some simple and minor suggestions. Please resolve them before merging :)

chapter_frame.go Outdated Show resolved Hide resolved
chapter_frame.go Outdated Show resolved Hide resolved
chapter_frame.go Outdated Show resolved Hide resolved
chapter_frame.go Show resolved Hide resolved
chapter_frame_test.go Outdated Show resolved Hide resolved
chapter_frame_test.go Outdated Show resolved Hide resolved
chapter_frame_test.go Outdated Show resolved Hide resolved
chapter_frame_test.go Outdated Show resolved Hide resolved
chapter_frame_test.go Outdated Show resolved Hide resolved
chapter_frame_test.go Outdated Show resolved Hide resolved
@n10v
Copy link
Owner

n10v commented Aug 24, 2021

Also one important thing: did you test the MP3 file with written CHAPs via id3v2 in a major media player (e.g. Apple Music, WinAmp)? Does a player recognise such CHAPs? If yes, please send me a screenshot 🙏

takaishi and others added 9 commits November 21, 2021 10:26
More clarity message

Co-authored-by: Albert Nigmatzianov <albertnigma@gmail.com>
More clarity message

Co-authored-by: Albert Nigmatzianov <albertnigma@gmail.com>
More clarity message

Co-authored-by: Albert Nigmatzianov <albertnigma@gmail.com>
More clarity message

Co-authored-by: Albert Nigmatzianov <albertnigma@gmail.com>
More clarity message

Co-authored-by: Albert Nigmatzianov <albertnigma@gmail.com>
More clarity message

Co-authored-by: Albert Nigmatzianov <albertnigma@gmail.com>
takaishi and others added 2 commits November 21, 2021 10:34
More clarity message

Co-authored-by: Albert Nigmatzianov <albertnigma@gmail.com>
@takaishi
Copy link
Contributor Author

@BoGeM I'm so sorry for late, I added commits for your comment. And I added results of test chapters written by id3v2 package:

Also one important thing: did you test the MP3 file with written CHAPs via id3v2 in a major media player (e.g. Apple Music, WinAmp)? Does a player recognise such CHAPs? If yes, please send me a screenshot 🙏

I tried test in a Apple Music or QuickTime Player, but they may not support chapters. I don't know major player supports chapter, so I tested by 2 software ( Forecast and IINA ).

First, I wrote chapters to test.mp3 by following code:

Code to write 3 chapters to copied file from testdata/test.mp3
package main

import (
	"fmt"
	"github.com/bogem/id3v2"
	"os"
	"time"
)

func main() {
	if err := _main(); err != nil {
		fmt.Errorf(err.Error())
		os.Exit(1)
	}
}

func _main() error {
	fmt.Println("AAA")

	f, err := os.Open("./test_copied.mp3")
	if err != nil {
		return err
	}
	defer f.Close()

	tag, err := id3v2.Open(f.Name(), id3v2.Options{Parse: true})
	if tag == nil || err != nil {
		return fmt.Errorf("error while opening mp3 file: ", err)
	}
	defer tag.Close()

	cfs := []id3v2.ChapterFrame{
		{
			ElementID:   "0",
			StartTime:   0,
			EndTime:     time.Duration(60000 * 1000000),
			StartOffset: 0,
			EndOffset:   0,
			Title: &id3v2.TextFrame{
				Text:     "chap0",
				Encoding: id3v2.EncodingUTF8,
			},
			Description: &id3v2.TextFrame{
				Text:     "description0",
				Encoding: id3v2.EncodingUTF8,
			},
		},
		{
			ElementID:   "1",
			StartTime:   time.Duration(60000 * 1000000),
			EndTime:     time.Duration(120000 * 1000000),
			StartOffset: 0,
			EndOffset:   0,
			Title: &id3v2.TextFrame{
				Text:     "chap1",
				Encoding: id3v2.EncodingUTF8,
			},
			Description: &id3v2.TextFrame{
				Text:     "description1",
				Encoding: id3v2.EncodingUTF8,
			},
		},
		{
			ElementID:   "2",
			StartTime:   time.Duration(120000 * 1000000),
			EndTime:     time.Duration(180000 * 1000000),
			StartOffset: 0,
			EndOffset:   0,
			Title: &id3v2.TextFrame{
				Text:     "chap2",
				Encoding: id3v2.EncodingUTF8,
			},
			Description: &id3v2.TextFrame{
				Text:     "description2",
				Encoding: id3v2.EncodingUTF8,
			},
		},
	}


	for _, cf := range cfs {
		tag.AddChapterFrame(cf)
	}
	if err := tag.Save(); err != nil {
		return err
	}

	return nil
}

Second, I opened mp3 file by Forecast that is chapter editor of mp3. It has 3 chapters with same start time, duration and title.

image

Finally, I played mp3 file by IINA because IINA supports chapters. In following gif movie, I select chapter 1 and 2, player jumps selected point (Sorry menu text is japanese)

Nov-21-2021 11-43-03

@takaishi takaishi requested a review from n10v November 21, 2021 02:55
Copy link
Owner

@n10v n10v left a comment

Choose a reason for hiding this comment

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

Awesome feature! Huge thanks to you for making this 👍

@n10v n10v merged commit 033cd25 into n10v:master Nov 23, 2021
@beautifulentropy
Copy link

Super excited for this. Thanks for all the hard work!

@n10v
Copy link
Owner

n10v commented Nov 24, 2021

Released in v2.1.2

@takaishi takaishi deleted the chapter-frame branch November 25, 2021 09:31
@umputun
Copy link

umputun commented Jan 1, 2024

@takaishi - I have tried to add chapters, and it worked the same way as you show in Forecast screenshot above. The issue (well, at least I think it is an issue) is that players don't show chapters. For example, Quick View doesn't show any. I think the reason can be seen on your screenshot - as you can see, the third column ("Include in chapters list") is unselected for each chapter. I have the same mp3 tagged with Python's eyed3, and all the chapters are marked as included and showed just fine by any player I have tried.

I'm not an expert in mp3 tags, but this may be due to a missing CTOC frame. I have tried to add it manually, like this, but it didn't change the result. Any bright ideas or suggestions on how to fix it?

	// create a CTOC frame manually
	ctocFrame := u.createCTOCFrame(chapters)
	tag.AddFrame(tag.CommonID("CTOC"), ctocFrame)

....

func (u *Upload) createCTOCFrame(chapters []chapter) *id3v2.UnknownFrame {
	var frameBody bytes.Buffer
	frameBody.WriteByte(0x03)                // write flags (e.g., 0x03 for top-level and ordered chapters)
	frameBody.WriteByte(byte(len(chapters))) // write the number of child elements

	// append child element IDs (chapter IDs)
	for i, _ := range chapters {
		elementID := fmt.Sprintf("%d", i+1)
		frameBody.WriteString(elementID)
		frameBody.WriteByte(0x00) // Null separator for IDs
	}

	// create and return an UnknownFrame with the constructed body
	return &id3v2.UnknownFrame{Body: frameBody.Bytes()}
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants