Hi Daniel,

> [Aim]
> An mp3 file contains only an ID3v2 tag whose content should be
> moved to a newly created ID3v1 tag and the ID3v2 tag should be deleted.
> 
> [Reproduce]
> 1. Fill the ID3v1 tag with information by clicking "von ID3v2" / "from ID3v2"
> 2. Click the "Entfernen" ("remove/delete") button
>    on the right side of the ID3v2 tag
> 3. Save the directory contents (Save / Strg-s)
> 
> [Expection]
> Now the ID3v2 tag should be gone and an ID3v1 tag should have been
> created which contains some of the copied information.
> 
> [Reality]
> The ID3v2 tag has been deleted (OK), the ID3v1 tag has disappeared (bug!)
> and the tag information cannot be recovered.

I just discovered your bug report. Thanks a lot, I could reproduce it.
This seems to be a bug in the id3lib. When updating a tag (V1 or V2)
and then stripping the other tag, the updated tag is stripped too. I
fixed it by first stripping any empty tags (V1 or V2) and then
updating non-empty tags (V1 or V2), see the following patch. This will
be fixed in the next release.

Regards,
Urs


--- kid3/kid3/mp3file.cpp.ba1   2005-09-09 20:07:34.000000000 +0200
+++ kid3/kid3/mp3file.cpp       2005-09-09 20:19:35.695272520 +0200
@@ -349,25 +349,28 @@
                return FALSE;
        }
 
-       if (tagV1 && (force || changedV1)) {
-               // There seems to be a bug in id3lib: The V1 genre is not
-               // removed. So we check here and strip the whole header
-               // if there are no frames.
-               if (tagV1->NumFrames() == 0) {
-                       tagV1->Strip(ID3TT_ID3V1);
-               } else {
-                       tagV1->Update(ID3TT_ID3V1);
-               }
+       // There seems to be a bug in id3lib: The V1 genre is not
+       // removed. So we check here and strip the whole header
+       // if there are no frames.
+       if (tagV1 && (force || changedV1) && (tagV1->NumFrames() == 0)) {
+               tagV1->Strip(ID3TT_ID3V1);
                changedV1 = FALSE;
        }
-       if (tagV2 && (force || changedV2)) {
-               // Even after removing all frames, HasV2Tag() still returns 
true,
-               // so we strip the whole header.
-               if (tagV2->NumFrames() == 0) {
-                       tagV2->Strip(ID3TT_ID3V2);
-               } else {
-                       tagV2->Update(ID3TT_ID3V2);
-               }
+       // Even after removing all frames, HasV2Tag() still returns true,
+       // so we strip the whole header.
+       if (tagV2 && (force || changedV2) && (tagV2->NumFrames() == 0)) {
+               tagV2->Strip(ID3TT_ID3V2);
+               changedV2 = FALSE;
+       }
+       // There seems to be a bug in id3lib: If I update an ID3v1 and then
+       // strip the ID3v2 the ID3v1 is removed too and vice versa, so I
+       // first make any stripping and then the updating.
+       if (tagV1 && (force || changedV1) && (tagV1->NumFrames() > 0)) {
+               tagV1->Update(ID3TT_ID3V1);
+               changedV1 = FALSE;
+       }
+       if (tagV2 && (force || changedV2) && (tagV2->NumFrames() > 0)) {
+               tagV2->Update(ID3TT_ID3V2);
                changedV2 = FALSE;
        }
        if (new_filename != filename) {

Reply via email to