D13700: implement reading of the replaygain tags

[Matthieu Gallien]

mgallien requested changes to this revision.
mgallien added a comment.
This revision now requires changes to proceed.


  Some inline comments

INLINE COMMENTS

> taglibextractor.cpp:254-263
> +            if(!strcmp( userTextFrame->description().toCString( true ), "replaygain_track_gain" ) ) {
> +                data.replayGainTrackGain = convertWCharsToQString(userTextFrame->fieldList().back().toCString(true));
> +            }
> +            if(!strcmp( userTextFrame->description().toCString( true ), "replaygain_track_peak" ) ) {
> +                data.replayGainTrackPeak = convertWCharsToQString(userTextFrame->fieldList().back().toCString(true));
> +            }
> +            if(!strcmp( userTextFrame->description().toCString( true ), "replaygain_album_gain" ) ) {

I am not a fan of the strcmp usage here. Is it not possible to do without them ?

> taglibextractor.cpp:468
>  
> -    // Rating.
>      itMPC = lstMusepack.find("RATING");

This change is unrelated to the patch. Please remove it.

> taglibextractor.cpp:662
>  
> -        // Rating.
>          itOgg = lstOgg.find("RATING");

same comment

> taglibextractor.cpp:934
> +            }
> +            qDebug() << data.replayGainAlbumGain;
> +            result->add(Property::ReplayGainAlbumGain, data.replayGainAlbumGain.toDouble());

Can you remove it ?

> taglibextractor.cpp:944
> +            /* remove " dB" suffix */
> +            if (data.replayGainTrackGain.endsWith(QStringLiteral(" dB"),Qt::CaseInsensitive))
> +            {

Could you check if it is not faster to use QLatin1String here ?

> properties.h:327-334
> +    /**
> +      * Contains ReplayGain information for audio files
> +      * The album/track gain is given in "dB"
> +      */
> +    ReplayGainAlbumPeak,
> +    ReplayGainAlbumGain,
> +    ReplayGainTrackPeak,

Please document each entry separately otherwise only the first one will have documentation (as seen on api.kde.org).

REPOSITORY
  R286 KFileMetaData

REVISION DETAIL
  https://phabricator.kde.org/D13700

To: astippich, mgallien, bruns
Cc: kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, astippich, spoorun, ngraham, bruns, abrahams
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20180829/b594266e/attachment.html>