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/#review25977
-----------------------------------------------------------


Hi, thanks for the patch, Opus codec support is (and will be even more in future) a requested feature.

Apart from comments below, there's one big thing: it doesn't build with TagLib 1.7 or 1.8, right? Please add cmake test to detect opus support and #ifdef the code - very similarly how this is done with mod/s3m support. A note in README file would be good too. Otherwise this looks good.

If you were an Amaork developer, you'd also add ChangeLog entry and search bugs.kde.org for any requests closed by this and add commit tags per https://projects.kde.org/projects/kde/kdelibs/repository/revisions/master/raw/.commit-template


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

    Hmm, http://en.wikipedia.org/wiki/Opus_%28audio_format%29 mentions just audio/opus (and audio/ogg, but let's not assign opus to plain audio/ogg) - have you seen audio/x-opus+ogg being used somewhere? I have no mention of opus under my /usr/share/mime - do you have some? Also note nice tool called kmimetypefinder.



shared/tag_helpers/TagHelper.cpp
<http://git.reviewboard.kde.org/r/108542/#comment19825>

    Amarok::Opus instead of Amarok::Ogg here?


- Matěj Laitl


On Jan. 22, 2013, 11:49 a.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, 11:49 a.m.)
> 
> 
> Review request for Amarok.
> 
> 
> Description
> -------
> 
> Support for .opus file tags added. Not sure about the mime type. Recent version of taglib necessary.
> 
> 
> Diffs
> -----
> 
>   shared/FileType.h b3d4470f11e48027e4442f8de8b305fc0df07e59 
>   shared/FileType.cpp 43d8777c3b2010f40147dad24138aa513b8ec481 
>   shared/FileTypeResolver.cpp 63a83e120f7197c44b86dbdcbcf663c8d42a8d2f 
>   shared/tag_helpers/TagHelper.cpp 45340180139b9ac0604ad93483dd9fd725a4cb02 
> 
> 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/20130122/9cea8549/attachment.html>