Writing rating to file causes file corruption

Bug #1740684 reported by Mirian Margiani
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Yarock
Fix Released
Undecided
SebastienAmar

Bug Description

If the option "Write rating to file" is activated, music files are destroyed when you
- change tracks in the playlist
- change the rating anywhere
- stop playback

I could reproduce it using OGG files, I'm not sure if it affects other file types as well. I didn't want to crap my music collection...

Steps to reproduce:
- add two OGG files to the playlist
- start the first one, wait a few seconds
- change rating of one file
- click on the next one, wait...
- change back to the first track -> it shouldn't start

EDIT: even easier steps:
- first way
    - add single Ogg file, don't play
    - change rating
    - try -> it shouldn't start
- second way
    - add single Ogg file, DO play, stop
    - change rating
    - try -> it shouldn't start

Trying to play the corrupted file with mpv results in "Failed to recognize file format". Metadata can be read, though.

It might be a concurrency problem because "writeTrackRatingToFile(...)" in core/mediaitem/tag.cpp is called very often. This even if nothing changed at all.

Interestingly, FLAC and OGG both use XiphComment but they are implemented differently:

...
    else if (TagLib::FLAC::File* file = dynamic_cast<TagLib::FLAC::File*>(fileref.file()))
    {
      TagLib::Ogg::XiphComment* vorbis_comments = file->xiphComment(true);

      vorbis_comments->addField("FMPS_RATING", QStringToTaglibString(QString::number( rating )));
    }
    else if (TagLib::Ogg::XiphComment* tag = dynamic_cast<TagLib::Ogg::XiphComment*>(fileref.file()->tag()))
    {
      tag->addField("FMPS_RATING", QStringToTaglibString(QString::number( rating )));
    }
...

I don't know what it does but looks like it might be the culprit...

Revision history for this message
SebastienAmar (sebastien-amardeilh) wrote :

Hi thanks for reporting and sorry for this issue.

I will try to fix it as soon as possible. Regards

Changed in yarock:
assignee: nobody → SebastienAmar (sebastien-amardeilh)
Revision history for this message
SebastienAmar (sebastien-amardeilh) wrote :

Hi,I tried to reproduce the problem with the same steps you described but everything works fine for me.

Can you run yarock with debug activated (./yarock --debug &) and sand me the log file.

Thanks for your contribution. Regards.

Changed in yarock:
milestone: none → 1.4.0
description: updated
Revision history for this message
Mirian Margiani (millimarg) wrote :

Well, I forgot the important part... you have to change the rating.

steps to reproduce:
- add single Ogg file, don't play
- change rating
- try -> it shouldn't start

or:
- add single Ogg file
- play, stop
- change rating
- try -> it shouldn't start

When you didn't start the track, there's an error message: "[Tag] taglib access failed : ==="

I experimented with flac and mp3 as well. The problem occurred only with Ogg files.

description: updated
Revision history for this message
SebastienAmar (sebastien-amardeilh) wrote :

Hi. Thanks for your help.

1) Bug reproduction
-------------------
  I have done many tests and I have still no errors.

2) Remarks
-----------
** When you didn't start the track, there's an error message: "[Tag] taglib access failed : ==="

  This message means your file cannot be read (at least by tag reading part).
  Maybe it appears after the file is corrupted ??

** It might be a concurrency problem because "writeTrackRatingToFile(...)"
  in core/mediaitem/tag.cpp is called very often. This even if nothing
  changed at all.

  This action shall be done only when you click on the stars from the playlist. Only once if one file is selected. I never see multiples calls, maybe you're right it's a good track.

3) Analysis
-----------
  Log file : I have checked all your log file and no specific error appears and the actions sequence seems good.

  Code analysis : I have checked all the code and nothing seems wrong for now

  Ogg corruption : Can you send me a corrupted ogg file you have in order to see if I can find what is wrong. Thanks.
  => we can use "ogginfo" (ogginfo -v file.ogg) to have information (before and after rating with yarock)

Regards,
Sebastien

Revision history for this message
Mirian Margiani (millimarg) wrote :

Well, now it's becoming weird. I did a clean recompile, and the rating is now being save only once (not "all the time" anymore). Then I tested with a very small Ogg file, which was not corrupted... The TagLib error did not occur anymore, too.

Revision history for this message
Mirian Margiani (millimarg) wrote :
Revision history for this message
Mirian Margiani (millimarg) wrote :
Revision history for this message
Mirian Margiani (millimarg) wrote :
Changed in yarock:
status: New → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.