Re: [Bug-gnupod] mktunes.pl creates corrupt iTunesDB ?

[Richard van den Berg]

On Wed, June 17, 2009 10:41, H. Langos wrote:
> The code and the naming of variables should express
> that this is a workaround, not the default. I.e the "keepattr" would
> better be named "low_ram_attr" or "shrink_itunes_db" or "minimal_attr".

I'll fix that. I was already thinkign you wouldn't like the keepattr name.
:-)

> Your mhod skipping code should only be active if that option is set in
> your .gnupodrc or given as command line switch.

That is the case now. If keppattr is not set, @keep is empty and $#keep =
-1 and nothing will be filtered. I wouldn't have written it any other way.

> Did you find out if shrinking the existing attributes (in contrast to
> skipping) had any effect?

It has effect on the iTunesDB size for sure, but not as drastically as
removing mhods. I can do more testing to see if it has an effect on the
iPod accepting the iTunesDB, but I am rather fed up with binary searches
at the moment. It takes a lot of time. I'd have to find the critical point
in my current configuration and then use the size of the mhods to see if I
can push it over or under this point.

> You only need to rerun tunes2pod if your GNUtunesDB.xml is lost or if you
> played around with iTunes in the meantime ... and we could warn people
> about this.

I don't plan on using iTunes, but I could see myself using Floola in
combination with gnupod.

> BTW: Does iTunes manage to get your full collection over to your ipod, or
> does it also produce an unusable iTunesDB?

I don't have my collection loaded in iTunes. I was saving that as a last
resort. Perhaps I'll give it a try over the weekend, after making sure it
can only access my collection read-only.

> Code remarks: I like the passing of optional arguments in a hash. However
> I strongly suggest using lower case keys (keep instead of Keep and item
> instead of Item.)

I though I was using the gnupod coding convention there. I don't like it
either, so I'll fix the other capitalized keys as well.

> Shouldn't that be :
>
> $mktunes->WriteItunesDB( {Keep => $opts{'keepattr'}} );

Absolutely. I already noticed that, and was quite surprised it worked
without the {}.

> Instead of @keep, I'd use %keep. and instead of creating it here for
> every mhit, I'd create it only once, and pass it along as a hashref.

Will do.

> PS: I've see the following line in the patch. So it seems my empty desc
> attributes wouldn't have made it into the iTunesDB anyway:
>>                       next unless $value; # Do not write empty values

Does $value evaluate to false when it is an empty string? I am planning to
check which mhods I am now actually filtering. I was quite surprised by
the size decrease my patch caused. Perhaps there are some mhods that
should be filtered by default.

Cheers,

Richard