D13700: implement reading of the replaygain tags

Stefan BrĂ¼ns noreply at phabricator.kde.org
Sat Sep 1 19:01:43 BST 2018


bruns added inline comments.

INLINE COMMENTS

> taglibextractor.cpp:252
> +    if (!lstID3v2.isEmpty()) {
> +        TagLib::ID3v2::UserTextIdentificationFrame *userTextFrame;
> +        userTextFrame = TagLib::ID3v2::UserTextIdentificationFrame::find(mpegFile.ID3v2Tag(), "replaygain_track_gain");

You can use a typedef in this scope, makes the long lines easier to read
https://en.cppreference.com/w/cpp/language/namespace_alias

  typedef TagLib::ID3v2::UserTextIdentificationFrame IdFrame;
  auto trackGainFrame = IdFrame::find(mpegFile.ID3v2Tag(), "replaygain_track_gain");
  if (trackGainFrame) { ...

Using a distinct name for each instance makes it clearer these are independent frames.

> taglibextractor.cpp:254
> +        userTextFrame = TagLib::ID3v2::UserTextIdentificationFrame::find(mpegFile.ID3v2Tag(), "replaygain_track_gain");
> +        if ( userTextFrame ) {
> +            data.replayGainTrackGain = convertWCharsToQString(userTextFrame->fieldList().back().toCString(true));

Remove the extra space inside the parentheses.

> taglibextractor.cpp:255
> +        if ( userTextFrame ) {
> +            data.replayGainTrackGain = convertWCharsToQString(userTextFrame->fieldList().back().toCString(true));
> +        }

you can use TStringToQString
`data.replayGainTrackGain = TStringToQString(userTextFrame->fieldList().back());`

> taglibextractor.cpp:255
> +        if ( userTextFrame ) {
> +            data.replayGainTrackGain = convertWCharsToQString(userTextFrame->fieldList().back().toCString(true));
> +        }

`fieldList()` may return an empty list, please add a check

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/20180901/dbe47eba/attachment-0001.html>


More information about the Kde-frameworks-devel mailing list