Review Request: tag reading support for mod, s3m, it and xm files

[Mathias Stephan Panzenböck]

> On Feb. 6, 2012, 7:07 p.m., Matěj Laitl wrote:
> > The last revision looks all-fine to me. When commiting, I would also apply following last-bits patch:
> > ====================================================================
> > diff --git a/CMakeLists.txt b/CMakeLists.txt
> > index 87dc1eb..58b3732 100644
> > --- a/CMakeLists.txt
> > +++ b/CMakeLists.txt
> > @@ -224,6 +224,9 @@ if( WITH_PLAYER )
> >      add_subdirectory( src )
> >      add_subdirectory( external )
> >  
> > +    # following line is here (and not near TAGLIB_MOD_FOUND) because there may be no MacroLogFeature without kdelibs
> > +    macro_log_feature( TAGLIB_MOD_FOUND "TagLib" "Support for tags in mod, s3m, it and xm files" "http://developer.kde.org/~wheeler/taglib.html" FALSE "1.8" "" )
> > +
> >      macro_display_feature_log()
> >  
> >      #Do not remove or modify these.  The release script substitutes in for these
> > diff --git a/ChangeLog b/ChangeLog
> > index 755858a..4df073d 100644
> > --- a/ChangeLog
> > +++ b/ChangeLog
> > @@ -4,6 +4,8 @@ Amarok ChangeLog
> >  
> >  Version 2.6-Beta 1
> >    FEATURES:
> > +    * Support for reading and writing tags from/to mod, s3m, it and xm files
> > +      thanks to Mathias Stephan Panzenböck.
> >      * Support for embedded album covers in non-collection tracks and
> >        in USB Mass Storage collection.
> >      * Hold the Shift key when dragging tracks to collections to move them
> > ====================================================================
> > 
> > I wanted to test this before pushing, but I faced 2 problems:
> >  * TagLib 1.8 was not yet released and I didn't feel like compiling it by hand
> >  * I found it hard to get example files of all the formats. Do you have some? It would be ideal if you had one small file of each format that would have a license compatible with Amarok so that they can be added to Amarok sources. Then you could write a test for these to ensure tag reading works in all future versions.
> > 
> > I think this patch _should_ be merged one day, but IMO we should wait for:
> >  a) TagLib 1.8 release
> >  b) ability to easily test the feature. Ideally if example files are added to Amarok sources and test-case is written, see the tests/ Amarok subdirectory. (Don't forget to enable KDE4_BUILD_TESTS cmake var; tests are then run using `make test`)

As I patched TagLib I also added some basic tests and with them some small example files, so you can take these: https://github.com/taglib/taglib/tree/master/tests/data I created the files with the original trackers or with MilkyTracker and MODPlug Tracker (not sure which file I created with which program). They don't really contain any music, only some empty samples. If you want some real-life example files take a look at http://www.chiptune.com/ or http://keygenmusic.net/. There is no license information on the files there, but they are certainly free.


- Mathias Stephan


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/101598/#review10375
-----------------------------------------------------------


On Feb. 4, 2012, 4:32 p.m., Mathias Stephan Panzenböck wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/101598/
> -----------------------------------------------------------
> 
> (Updated Feb. 4, 2012, 4:32 p.m.)
> 
> 
> Review request for Amarok.
> 
> 
> Description
> -------
> 
> This patch adds read/write tag support for mod, s3m, it and xm files if taglib supports it. All new parts are #ifdefed so it won't break if an old taglib version is used.
> 
> I wrote a patch for taglib that enables support for these file formats and it will be included in taglib 1.8.0. Here is the pull request:
> https://github.com/taglib/taglib/pull/4
> 
> 
> This addresses bug 90524.
>     https://bugs.kde.org/show_bug.cgi?id=90524
> 
> 
> Diffs
> -----
> 
>   CMakeLists.txt d47c28b 
>   config-amarok.h.cmake 2d25cc7 
>   shared/FileType.h 5c8081f 
>   shared/FileType.cpp a6e25d5 
>   shared/FileTypeResolver.cpp e69a514 
>   shared/tag_helpers/TagHelper.cpp 4c0fb2b 
> 
> Diff: http://git.reviewboard.kde.org/r/101598/diff/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Mathias Stephan Panzenböck
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/amarok-devel/attachments/20120206/9130046e/attachment-0001.html>