After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 540394 - Importing multiple CDs where metadata wasn't found causes tracks to be overwritten
Importing multiple CDs where metadata wasn't found causes tracks to be overwr...
Status: RESOLVED FIXED
Product: banshee
Classification: Other
Component: Importing
1.0.0
Other All
: Normal critical
: 1.x
Assigned To: Banshee Maintainers
Banshee Maintainers
: 562041 575915 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2008-06-26 23:02 UTC by Ryan Zeigler
Modified: 2010-11-29 20:43 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
rename with an index imported file when exist (1.14 KB, patch)
2009-07-19 13:32 UTC, olivier dufour
needs-work Details | Review
patch 1 (1.36 KB, patch)
2010-11-05 17:20 UTC, olivier dufour
needs-work Details | Review
patch 2 ripper (2.05 KB, patch)
2010-11-28 21:00 UTC, olivier dufour
committed Details | Review

Description Ryan Zeigler 2008-06-26 23:02:57 UTC
Please describe the problem:
Pretty much as above. If metadata cannot be found for a CD it is imported into Unknown Artist/Unknown Album/... The next time a CD is imported, Banshee appears to write over the previous.

Steps to reproduce:
1. Import CD #1 with no metadata
2. Import CD #2 with no metadata
3. Observe that 1 set of files is created. 


Actual results:
Overlapping files are corrupted and will not play in totem.

Expected results:
Some kind of namespacing would occur

Does this happen every time?
Yes

Other information:
I tested this with an import profile of flac. Probably unrelated, but Sound Juicer is able to detect metadata for both of the CDs that I use, probably through cd text or w/e that file is called.
Comment 1 John Douthat 2008-08-24 01:22:56 UTC
I'm using Banshee 1.2.1, and I've also encountered this bug.

The first CD is ripped to 
/home/john/Music/Unknown Artist - Unknown Album/01. Track 1.flac
etc..

After ripping the first CD, I tagged the album and ripped another CD. Both CDs were missing metadata. After ripping the second album, Track 1 of both the newly ripped album and the previously ripped album played the same song. They pointed to the same file.

It seems that Banshee's ripper doesn't check for file existence before ripping.

This was particularly frustrating because the first CD was heavily scratched and took a long time to rip.

(revision 4424)
banshee/libbanshee/banshee-ripper.c:320
banshee/src/Backends/Banshee.GStreamer/Banshee.GStreamer/AudioCdRipper.cs:121-130
banshee/src/Extensions/Banshee.AudioCd/Banshee.AudioCd/AudioCdRipper.cs:184-186
  * This file should probably check to make sure it's not overwriting an existing file
Comment 2 Wiennat Mongkulmann 2008-09-28 13:12:41 UTC
I also encountered this bug.

I use Ubuntu 8.04.1 with Banshee 1.2.1. I tried to import my 2 CDs into MP3 before sync it with my MP3 player. I had no internet access on that machine so Banshee could not download the disc information from the internet and left the metadata to Unknown Album/Unknown Artist. 

At first, when I ripped my first CD. The ripped tracks were saved to my HOMEDIR/Unknown Artist/Unknown Album/Track #.mp3. And when I ripped the second CD, the ripped tracks from disc 1 were overwritten by ripped track from disc 2.


May I propose a solution?
Enabling user to edit the CD meta before importing the tracks and use CD disc ID to recognize the disc. Though it is not the best solution I hope this help.
Comment 3 Chris Lees 2008-11-16 09:53:24 UTC
Wiennat: The solution you propose is the most intuitive one. Banshee used to allow this but does not anymore!
Comment 4 Alexander Kojevnikov 2009-05-23 01:22:15 UTC
*** Bug 575915 has been marked as a duplicate of this bug. ***
Comment 5 Alexander Kojevnikov 2009-05-23 01:26:03 UTC
*** Bug 562041 has been marked as a duplicate of this bug. ***
Comment 6 era+gnome 2009-06-24 13:11:57 UTC
(From duplicate #562041) An alternative solution in the interim might be to use a unique temporary directory name for each CD rather than a static "Unwnown Artist".
Comment 7 olivier dufour 2009-07-19 13:32:06 UTC
Created attachment 138738 [details] [review]
rename with an index imported file when exist

It seems that I have commit a patch at the wrong bug (bug 540597). So hope it is attach to the good one ;)
Comment 8 Gabriel Burt 2009-08-12 21:54:06 UTC
Thanks, Olivier.

1. Please make your patches from the toplevel checkout directory
2. Can you rework this to use String.Format ("{0} ({1})", uri, i++); instead of StringBuilder
3. as implicitly mentioned in 2, don't forget to increment i
Comment 9 olivier dufour 2010-11-05 17:20:07 UTC
Created attachment 173897 [details] [review]
patch 1
Comment 10 Alex Launi 2010-11-17 02:24:18 UTC
It seems like you should initialize i to 1, or use ++i so that you get (1) instead of (0).

Also the while() block should be enclosed in { } braces.
Comment 11 Bertrand Lorentz 2010-11-27 12:24:34 UTC
Review of attachment 173897 [details] [review]:

The patch doesn't fix the problem, because the URI built here doesn't have any extension. It's something like '/path/to/Track O1'.
So File.Exists(uri2) always returns false.

The extension will only be set by the actual ripper, depending on which format the file is encoded to : .mp3 or .ogg, etc.

Oh, and Alex should really use the "review" feature of Bugzilla, or at least change the status of patches he comments on ;)
Comment 12 Bertrand Lorentz 2010-11-27 12:24:37 UTC
Review of attachment 173897 [details] [review]:

The patch doesn't fix the problem, because the URI built here doesn't have any extension. It's something like '/path/to/Track O1'.
So File.Exists(uri2) always returns false.

The extension will only be set by the actual ripper, depending on which format the file is encoded to : .mp3 or .ogg, etc.

Oh, and Alex should really use the "review" feature of Bugzilla, or at least change the status of patches he comments on ;)
Comment 13 olivier dufour 2010-11-28 21:00:33 UTC
Created attachment 175428 [details] [review]
patch 2 ripper 

here is a new patch: hope this one is good one.
Comment 14 Bertrand Lorentz 2010-11-29 20:41:06 UTC
Comment on attachment 175428 [details] [review]
patch 2 ripper 

Thanks for the follow-up !

The patch didn't compile as-is : File.Exists take a SafeUri.

I fixed that, and the code formatting and committed it :
http://git.gnome.org/browse/banshee/commit/?id=fa577357
Comment 15 Bertrand Lorentz 2010-11-29 20:43:28 UTC
This problem has been fixed in our software repository. The fix will go into the next software release. Thank you for your bug report.

(I also committed the fix to the stable-1.8 branch)