mid3v2: setting multiple values/frame; deleting and setting frames at the same time (with PATCH)

[GoogleCodeExporter]

Hi there-

I'm using mid3v2 as a command line MP3 tag editor. There are a couple things
it does not do, which I have implemented and attach a patch. Could you
please review and (if okay) apply? This patch is against SVN as of today.

====

1.) "mid3v2 --delete-frames=...." and "mid3v2 --frame value" can not be
called at the same time

The parser structure checks whether we are setting/adding frames or whether
we are deleting frames, and it then executes delete_frames() or
write_files(). It does not call both, if both option categories are given.

The attached patch fixes it so that if we are both deleting and setting
frames, we are executing both operations serially. This is not optimal
(better would be to parse and write the file only once) but it _works_.

2.) "mid3v2 -g genre1 -g genre2" does not work

Inside write_file(), the variable edits will be a list of (frame,value)
pairs. If a frame recurs (i.e. I'm trying to set multiple genres), the last
pair will override all earlier choices.

I added a preprocessing stage for edits, where it will collapse all pairs
with identical frame name to have value lists instead of scalars.

===

This functionality has been tested and seems to work fine. However I'm
neither a python expert, nor did I want to change the overall structure of
mid3v2 to more organically support my use case. Feel free to rewrite my
patch. But some way - any way - to support my usecase would be great in
upstream mutagen.

Thank you!

Hans

Original issue reported on code.google.com by hanse...@gmail.com on 6 Nov 2009 at 6:53

[GoogleCodeExporter]

Sorry, got a mistake when I refreshed my diff against SVN, here is the corrected
version

Original comment by hanse...@gmail.com on 6 Nov 2009 at 6:59

[GoogleCodeExporter]

Okay, here is (as far as I can see) the final version of this patch.

I reversed the order of applying the "--delete-frame" and "--FRAME=value" 
operations.
It makes more sense to first apply the deletes, then the writes. This way a 
script can
first summarily delete all occurrences of certain frames, and then add the 
correct
values back in.

Original comment by hanse...@gmail.com on 17 Nov 2009 at 5:51

Attachments:

• mid3v2.diff

[GoogleCodeExporter]

Original comment by joe.wreschnig@gmail.com on 25 Nov 2009 at 12:02

• Changed state: Accepted

[GoogleCodeExporter]

I am unclear where the atomic_types variable is supposed to come from. Can you 
change
that whole bit so the edits dict just always holds the right type, and we don't 
need
to postprocess the preprocess?

Original comment by joe.wreschnig@gmail.com on 13 Dec 2009 at 4:22

[GoogleCodeExporter]

Good point. How's the attached?

Note that the biggest part of the attached diff is just an indentation change: 
the block
between 

   values = value.split(":")

and

   encoding=3, text=value, lang=lang, desc=desc)

is now within a loop over vlist.

Original comment by hanse...@gmail.com on 28 Dec 2009 at 5:52

Attachments:

• mid3v2.diff

[GoogleCodeExporter]

This issue was closed by revision r69.

Original comment by joe.wreschnig@gmail.com on 28 Dec 2009 at 6:35

• Changed state: Fixed

[GoogleCodeExporter]

There was a problem with my patch. I'm sorry! I've never coded python before. 
Here is a fix:

Original comment by hanse...@gmail.com on 29 Dec 2009 at 10:10

Attachments:

• mid3v2.diff

[GoogleCodeExporter]

Note that the bug I introduced with r69 only appeared when actually using the 
new
functionality, for example assigning multiple genres. In that case the bug is 
fatal, but at
least it is not a regression.

Original comment by hanse...@gmail.com on 29 Dec 2009 at 10:15

[GoogleCodeExporter]

Don't know if Joe is looking at "Fixed" issues, so I made a new issue#50 out of 
it.

Original comment by hanse...@gmail.com on 5 Jan 2010 at 7:29