Review Request 108542: Support for .opus file tags added

[Matěj Laitl]

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


Hi, this starts to look good now, just a few more nitpicks. I think it is also worth to add a macro_log_feature line to CMakeLists.txt similar to what it is done with mod support. Also adding a note to README would be nice.


shared/FileTypeResolver.cpp
<http://git.reviewboard.kde.org/r/108542/#comment20373>

    Why is including config.h needed for TAGLIB_OPUS_FOUND while it isn't needed for TAGLIB_MOD_FOUND?



shared/FileTypeResolver.cpp
<http://git.reviewboard.kde.org/r/108542/#comment20374>

    Please re-add the "audio/x-opus+ogg" - after discussion with Edward we decided it would be more safe to mention it too. Sorry about the fuss.


- Matěj Laitl


On Jan. 22, 2013, 3:30 p.m., Martin Brodbeck wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/108542/
> -----------------------------------------------------------
> 
> (Updated Jan. 22, 2013, 3:30 p.m.)
> 
> 
> Review request for Amarok.
> 
> 
> Description
> -------
> 
> Support for .opus file tags added. Not sure about the mime type. Recent version of taglib necessary.
> 
> 
> Diffs
> -----
> 
>   CMakeLists.txt f87e05e 
>   shared/FileType.h b3d4470 
>   shared/FileType.cpp 43d8777 
>   shared/FileTypeResolver.cpp 63a83e1 
>   shared/config.h.cmake 004dd57 
>   shared/tag_helpers/TagHelper.cpp 4534018 
> 
> Diff: http://git.reviewboard.kde.org/r/108542/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Martin Brodbeck
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/amarok-devel/attachments/20130209/c2813d6d/attachment.html>