CHAP and CTOC frames support (solves https://github.com/taglib/taglib/issues/81) #173

dckface · 2013-04-27T14:25:32Z

I've added support for ID3v2 CHAP and CTOC frames (according to http://id3.org/id3v2-chapters-1.0).

Every field is supported, except optional embedded frames, which are currently ignored, because I wasn't sure, how to handle them properly. If I shouldn't ignore them, just let me know and I will change it.

Also, I added some basic tests of CHAP and CTOC frames parsing.

Fixed minor bugs in ChapterFrame and TableOfContentsFrame headers.

…rame and TableOfContentsFrame classes. Methods setElementID of ChapterFrame and TableOfContentsFrame classes now automatically terminates new element ID with null.

Fixed errors in ChapterFrame method renderFields. Fixed errors in TableOfContentsFrame method parseFields. Added ChapterFrame and TableOfContentsFrame headers and sources to CMakeLists.txt. Added some basic testing of CHAP and CTOC frames parsing.

dckface · 2013-05-04T19:30:42Z

I removed duplicated function for CHAP frame parsing test (I just noticed it). Other parts of code should be ok, so I hope this will be useful somehow. And if I shouldn't ignore optional embedded frames in CHAP and CTOC frames, just let me know and I will change it.

lalinsky · 2013-05-06T19:29:20Z

I'll try to review this later this week. It would be great to get the embedded frames in as well. I'm not familiar withe CHAP frame specification, but I think it would work similarly to the loop when reading the main ID3v2 frames in ID3v2::Tag::parse().

dckface · 2013-05-07T08:33:53Z

Ok, I'll do it later today. I think the best approach is to read embedded frames as you say and store them in frame list, because CHAP and CTOC frames specification doesn't say anything about which types of frames can be embedded in CHAP and CTOC frames.

sbooth · 2013-09-29T01:00:15Z

@krejclu6 is there any chance you have time to take a look at implementing embedded frames? Its been a few months and I'd like to merge this pull request.

dckface · 2013-09-29T15:18:25Z

I didn't have much time during last few months, but I'll try to find some time in following week.

Added embedded frames parsing. Added embedded frames rendering. Modified constructor of CHAP and CTOC frame, so it can accept list of embedded frames. Added unit tests for CHAP and CTOC frames parsing and rendering (with support of embedded frames). Fixed bugs in rendering of CTOC frames. Added functions for adding and removing child elements in CTOC frames.

dckface · 2013-10-13T16:42:03Z

I was finally able to find some time to complete embedded frames support (including modification of existing chapter and table of contents frames unit tests). It should work as expected.
However, when I tried to use TIT2 frame as CHAP frame's embedded frame, rendering function of TIT2 frame actually never appended terminating null of contained string. I wasn't sure, if this is bug or not, so i modified unit to ignore this.
And sorry about last commit (uncommenting unit test). I had to comment out testCompressedFrameWithBrokenLength, because it was causing SEGFAULT, so this commit just uncomments this unit test.
Also, I'm not sure, if my changes still apply to current taglib's master. Should I rebase?

sbooth · 2013-10-15T12:00:25Z

If you're able to rebase to get your changes to apply cleanly it would make things much easier. Thank you for taking the time to write this code!

gholzmann · 2014-01-08T08:37:54Z

That's great, thanks a lot!

If you need some files for testing, you can use:

https://auphonic.com/media/blog/auphonic_chapters_demo.mp3 (with chapter title, url and image)

Or some real-world podcasts (bigger files):

Removed a wrong note from a comment in tstring.h.

Fix #162: Xiph and APE generic getters return space-concatenated values

attilagyorffy · 2014-05-13T15:40:25Z

Any update on this one? I'd love to be able to read and write chapter information using TagLib. Great effort, thank you!

Fixed minor bugs in ChapterFrame and TableOfContentsFrame headers.

…rame and TableOfContentsFrame classes. Methods setElementID of ChapterFrame and TableOfContentsFrame classes now automatically terminates new element ID with null.

Fixed errors in ChapterFrame method renderFields. Fixed errors in TableOfContentsFrame method parseFields. Added ChapterFrame and TableOfContentsFrame headers and sources to CMakeLists.txt. Added some basic testing of CHAP and CTOC frames parsing.

Added embedded frames parsing. Added embedded frames rendering. Modified constructor of CHAP and CTOC frame, so it can accept list of embedded frames. Added unit tests for CHAP and CTOC frames parsing and rendering (with support of embedded frames). Fixed bugs in rendering of CTOC frames. Added functions for adding and removing child elements in CTOC frames.

…into krejclu6_chapters Conflicts: taglib/mpeg/id3v2/id3v2framefactory.cpp tests/test_id3v2.cpp

dckface · 2014-05-18T14:37:44Z

After several months, I was finally able to find some free time to do the rebase. Howere, I'm not sure, if i did it the right way, so let me know, if there is anything I could fix (maybe squashing the commits or so).

sbooth · 2014-06-07T02:29:37Z

This looks good to me. @lalinsky what do you think?

lalinsky · 2014-06-10T15:30:40Z

taglib/ape/apetag.cpp

@@ -89,35 +89,35 @@ String APE::Tag::title() const
 {
  if(d->itemListMap["TITLE"].isEmpty())
    return String::null;
-  return d->itemListMap["TITLE"].toString();
+  return d->itemListMap["TITLE"].values().toString();


What is the reason for this change?

Hm, you can probably ignore this and the other comment. I see it was from a commit that was merged to master, but I'm not sure why it's showing up as a change.

Yeah, I think it appeared in changes after I merged current master with my fork's master.

lalinsky · 2014-06-10T16:03:16Z

I have not done a very thorough review, but does look good to me. Thanks for working on this!

dckface · 2014-06-18T18:11:54Z

Ok, so adressed issues are fixed, except that two changes from some old commit.

lalinsky · 2014-06-18T20:44:34Z

Thanks, I'll merge it tomorrow.

lalinsky · 2014-07-16T13:53:12Z

Sorry this took so long. It's merged now. Thank you for the patch!

Lukáš Krejčí added 7 commits April 20, 2013 15:52

Created CPP and H files for CTOC and CHAP frames.

08ae0e8

Added basic members of ChapterFrame and TableOfContentsFrame classes.

4be1279

Fixed minor bugs in ChapterFrame and TableOfContentsFrame headers.

Finished parseFields, renderFields and asProperty methods of ChapterF…

fcfd9f5

…rame and TableOfContentsFrame classes. Methods setElementID of ChapterFrame and TableOfContentsFrame classes now automatically terminates new element ID with null.

Fixed error in childElements function.

bcad792

Changed copyright and e-mail in modified files.

6500695

Removed duplicated CHAP frame testing funtion.

cbd6f73

Lukas Krejci added 2 commits October 13, 2013 18:27

Uncommenting unit test.

1ce5385

supermihi and others added 3 commits April 3, 2014 21:07

Fix #162: Xiph and APE generic getters return space-concatenated values

1723e9b

Merge pull request #380 from TsudaKageyu/fix-comment

ecc768d

Removed a wrong note from a comment in tstring.h.

Merge pull request #370 from supermihi/fix162v2

f27e0aa

Fix #162: Xiph and APE generic getters return space-concatenated values

Lukáš Krejčí added 9 commits May 18, 2014 16:11

Created CPP and H files for CTOC and CHAP frames.

0864634

Added basic members of ChapterFrame and TableOfContentsFrame classes.

3a1040d

Fixed minor bugs in ChapterFrame and TableOfContentsFrame headers.

Finished parseFields, renderFields and asProperty methods of ChapterF…

c5f9258

…rame and TableOfContentsFrame classes. Methods setElementID of ChapterFrame and TableOfContentsFrame classes now automatically terminates new element ID with null.

Fixed error in childElements function.

17841e8

Changed copyright and e-mail in modified files.

98ed58f

Removed duplicated CHAP frame testing funtion.

140fb2b

Uncommenting unit test.

500b3e6

Merge branch 'krejclu6_chapters' of https://github.com/krejclu6/taglib …

459adc6

…into krejclu6_chapters Conflicts: taglib/mpeg/id3v2/id3v2framefactory.cpp tests/test_id3v2.cpp

lalinsky reviewed Jun 10, 2014
View reviewed changes

Code clean-up.

a192db0

lalinsky merged commit a192db0 into taglib:master Jul 16, 2014

This was referenced Dec 4, 2014

MP3 chapter support #81

Closed

CHAP and CTOC support robinst/taglib-ruby#63

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

CHAP and CTOC frames support (solves https://github.com/taglib/taglib/issues/81) #173

CHAP and CTOC frames support (solves https://github.com/taglib/taglib/issues/81) #173

dckface commented Apr 27, 2013

dckface commented May 4, 2013

lalinsky commented May 6, 2013

dckface commented May 7, 2013

sbooth commented Sep 29, 2013

dckface commented Sep 29, 2013

dckface commented Oct 13, 2013

sbooth commented Oct 15, 2013

gholzmann commented Jan 8, 2014

attilagyorffy commented May 13, 2014

dckface commented May 18, 2014

sbooth commented Jun 7, 2014

lalinsky Jun 10, 2014

lalinsky Jun 10, 2014

dckface Jun 11, 2014

lalinsky commented Jun 10, 2014

dckface commented Jun 18, 2014

lalinsky commented Jun 18, 2014

lalinsky commented Jul 16, 2014

CHAP and CTOC frames support (solves https://github.com/taglib/taglib/issues/81) #173

CHAP and CTOC frames support (solves https://github.com/taglib/taglib/issues/81) #173

Conversation

dckface commented Apr 27, 2013

dckface commented May 4, 2013

lalinsky commented May 6, 2013

dckface commented May 7, 2013

sbooth commented Sep 29, 2013

dckface commented Sep 29, 2013

dckface commented Oct 13, 2013

sbooth commented Oct 15, 2013

gholzmann commented Jan 8, 2014

attilagyorffy commented May 13, 2014

dckface commented May 18, 2014

sbooth commented Jun 7, 2014

lalinsky Jun 10, 2014

Choose a reason for hiding this comment

lalinsky Jun 10, 2014

Choose a reason for hiding this comment

dckface Jun 11, 2014

Choose a reason for hiding this comment

lalinsky commented Jun 10, 2014

dckface commented Jun 18, 2014

lalinsky commented Jun 18, 2014

lalinsky commented Jul 16, 2014