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

[Richard van den Berg]

On 7/2/09 3:43 PM, H. Langos wrote:
 Could you check if it still does what it was is supposed to do? :-)
   

It looks fine, and my iPod still works, and didn't even have to reboot
it for it to show my 30640 files. :-)

 I merged the low_ram branch into master yesterday evening but I wanted 
 to sleep over it before commiting.

 Here are the changes that actually happend to master:

 git diff 94462eb 25a4d8b
   

That diff looks good to me.


 It may be better to avoid merges among branches.
   

Yeah, sorry about that. I won't be doing that again (and I'll be less
eager to push commits I did).


Cheers,

Richard


___
Bug-gnupod mailing list
Bug-gnupod@nongnu.org
http://lists.nongnu.org/mailman/listinfo/bug-gnupod

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

[H. Langos]

G'day J,

Thank you very much for the examples and the insights.
It's good to know that I'm not completely off with my 
novice's views of code style.

On Thu, Jul 02, 2009 at 10:30:04AM +1000, Jacinta Richardson wrote:
 This practice is called named arguments and is a great way of calling
 subroutines because it removes the confusion of which order the arguments need
 to go in.  It also allows you to set and use defaults, for example:

I hadn't looked at it from the angle of providing defaults. But it looks
very intuitive and robust. Until now I had defaults assigned one by one 
to my variables in sub routines and then optionally overwritten by the 
arguments passed in the $args_ref. Assigning them in one go is way more 
elegant.

   state %defaults = ( # 5.10+ only, else my %defaults
   foo = 'a', # or a closure
   bar = 'b',
   );

state seems more or less equivalent to static in C. Nice feature.

   $args_ref = { %defaults, %{$args_ref} };

I hadn't tought of that way to merge hashes. I wonder if it is usable for
subroutines that are called very often or if unrolling both hashes and
merging them this way is slower than conditionally assigning one by one.

my $foo = 'a';
my $bar = 'b';
$foo = $args_ref-{foo} if defined($args_ref-{foo});

My first guess would be that merging the hashes is faster for bigger numbers
of arguments but slower if you are dealing with only a couple of arguments.
Anyway, I'll try both when the occasion arises and see what the profiler
has to say.

 It's not discussed in perlstyle, and perlsub because the maintainers of those
 files haven't really decided on a one-true way, or because they've been too 
 lazy
 to write it...  Damian Conway wrote bunch of good ideas about good Perl style 
 in
 his book called  Perl Best Practices, which is what most people point to when
 they talk about Perl style now.  These practices have been turned into
 requirements by the Perl::Critic system too, so you can get automated feedback
 on your code quality by using the Perl::Critic programs.

Thats a very good pointer. I'll try and see what Perl::Critic has to say about
the code in gnupod.

 
 Hope this helps.
 

A lot! Thank you very much. 

cheers
-henrik



___
Bug-gnupod mailing list
Bug-gnupod@nongnu.org
http://lists.nongnu.org/mailman/listinfo/bug-gnupod

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

[H. Langos]

Hi Richard,


On Wed, Jul 01, 2009 at 04:57:14PM +0200, Richard van den Berg wrote:
 On Wed, July 1, 2009 16:13, Jacinta Richardson wrote:
  I suspect it's a matter of laziness.
 
 I claim ignorance. I didn't know there was a fundamental difference
 between the two ways of calling a subroutine. Thanks for the explanation.
 
  I'd suggest applying the patch with this tidied up as
  appropriate.

Done.

Could you check if it still does what it was is supposed to do? :-)

I merged the low_ram branch into master yesterday evening but I wanted 
to sleep over it before commiting.

Here are the changes that actually happend to master:

git diff 94462eb 25a4d8b

or

http://git.savannah.gnu.org/cgit/gnupod.git/commit/?id=25a4d8ba6a080589577abc20db7587a18b1cade9

It was a bit tricky as the previous merges on low_ram made 
parts of the changes in master look outdated eventhough they 
were more current than than in low_ram.
I hope I didn't mess it up completely. :-)

It may be better to avoid merges among branches.

rather

 -o---o---o---o---o---o---o  master
   \   \   \ /   /
o   o---o---o   / topic a
 \ /
  o---o---o---o---o topic b

than

 -o---o---o---o---o---o---o  master
   \   \   \ /   /
o   o---o---o   / topic a
 \   \ /
  o---o---o---o---o topic b

Also, if you need to merge several branches into one,
I suggest the usage of octopus merges instead of merging
step by step.

But I'm no git guru. So don't take my word for it.

cheers
-henrik



___
Bug-gnupod mailing list
Bug-gnupod@nongnu.org
http://lists.nongnu.org/mailman/listinfo/bug-gnupod

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

[H. Langos]

Hi Richard,

On Fri, Jun 19, 2009 at 08:26:30PM +0200, Richard van den Berg wrote:
 On 6/19/09 11:07 AM, H. Langos wrote:
 Maybe the merge code should only be active if your memory saving feature is
 active?
   

 Here is the new patch that does just that. I love git, it's super fast. :-)


One more question:

Why did you use
 resetxml; 
instead of 
 resetxml();
?

I know the former doesn't pass an empty @_ array for the called sub but 
passes the existing argument list. But you don't do anything with @_ in 
resetxml(). So why bother passing the current arguments list on to it?

cheers
-henrik

 diff --git a/src/ext/XMLhelper.pm b/src/ext/XMLhelper.pm
 index 748ce22..bb7c88b 100755
 --- a/src/ext/XMLhelper.pm
 +++ b/src/ext/XMLhelper.pm
 @@ -301,19 +301,23 @@ sub mkh {
  }
  
  
 -
  #
 -# Parses the XML File and do events
 -sub doxml {
 - my($xmlin, %opts) = @_;
 - return undef unless (-r $xmlin);
 - ### reset some stuff if we do a second run
 +# Reset some stuff if we do a second run
 +sub resetxml {
   $cpn = undef; #Current PlaylistName
   @idpub = ();
   @plorder = ();
   $xid = 1;
   $XDAT = undef;
 - ###
 +}
 +
 +
 +#
 +# Parses the XML File and do events
 +sub doxml {
 + my($xmlin, %opts) = @_;
 + return undef unless (-r $xmlin);
 + resetxml;
   my $p;
   my $ref = eval {
   $p = new XML::Parser(ErrorContext = 0, 
 Handlers={Start=\eventer});



___
Bug-gnupod mailing list
Bug-gnupod@nongnu.org
http://lists.nongnu.org/mailman/listinfo/bug-gnupod

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

[Jacinta Richardson]

H. Langos wrote:


One more question:

Why did you use
 resetxml; 
instead of 
 resetxml();

?

I know the former doesn't pass an empty @_ array for the called sub but 
passes the existing argument list. But you don't do anything with @_ in 
resetxml(). So why bother passing the current arguments list on to it?


I imagine that the author didn't intend that effect of calling the 
subroutine that way.  I suspect it's a matter of laziness.  FWIW,


resetxml();

or

resetxml(@_);   # if necessary

are both many,  many times preferable to:

resetxml;

and the latter is generally shunned by experienced, community-minded 
Perl folk.  I'd suggest applying the patch with this tidied up as 
appropriate.


J


___
Bug-gnupod mailing list
Bug-gnupod@nongnu.org
http://lists.nongnu.org/mailman/listinfo/bug-gnupod

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

[Richard van den Berg]

On Wed, July 1, 2009 16:13, Jacinta Richardson wrote:
 I imagine that the author didn't intend that effect of calling the
 subroutine that way.

Absolutely right.

 I suspect it's a matter of laziness.

I claim ignorance. I didn't know there was a fundamental difference
between the two ways of calling a subroutine. Thanks for the explanation.

 I'd suggest applying the patch with this tidied up as
 appropriate.

I agree.

Cheers,

Richard (learning all the time)



___
Bug-gnupod mailing list
Bug-gnupod@nongnu.org
http://lists.nongnu.org/mailman/listinfo/bug-gnupod

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

[Jacinta Richardson]

H. Langos wrote:

 Speaking of readability and code style. Is there a consensus among the
 experienced and community-minded folks in regard to the passing of 
 parameters to subs? Especially when writing subs in modules?
 
 I nowadays prefer to pass parameters as one hashref
   subname({foo = x, bar = y});
 instead of as a naked list:
   subname(x, y);
 or (heaven help)
   subname(foo = x, bar = y);
 which looks like a hash but is only a list.
 
 I took a look at perlstyle(1) and perlsub(1) but I didn't see any hints in
 regard to this. 

This practice is called named arguments and is a great way of calling
subroutines because it removes the confusion of which order the arguments need
to go in.  It also allows you to set and use defaults, for example:

subname({foo = x, bar = y});
subname({foo = x});
subname({bar = y});
subname();

...

my %defaults = (
foo = 'a',
bar = 'b',
};

sub subname {
my ($args_ref) = @_;

state %defaults = ( # 5.10+ only, else my %defaults
foo = 'a', # or a closure
bar = 'b',
);

$args_ref = { %defaults, %{$args_ref} };

# Now you know for sure that foo and bar have values
# for all the above calls.

}

It's not discussed in perlstyle, and perlsub because the maintainers of those
files haven't really decided on a one-true way, or because they've been too lazy
to write it...  Damian Conway wrote bunch of good ideas about good Perl style in
his book called  Perl Best Practices, which is what most people point to when
they talk about Perl style now.  These practices have been turned into
requirements by the Perl::Critic system too, so you can get automated feedback
on your code quality by using the Perl::Critic programs.

Anyway, Damian advises:

Use a hash of named arguments for any subroutine that has more than 3   
parameters

and agrees entirely with your preference:

subname({foo = x, bar = y});

rather than the alternatives you give.

This has the advantage that mistakes in how you call the subroutine may be
spotted at compile time rather than run time:

subname({foo = x, bar = y, cols = 1..4});

Odd number of elements in anonymous hash at demo.pl line 2

He also says it's okay to mix positional and named arguments if you don't have
many but some are always mandatory.  For example:

padded($string, {cols = 20, centered =1, filler=$SPACE});

sub padded {
my ($text, $args_ref) = @_;

# pad the string.
}

Hope this helps.

J


-- 
   (`-''-/).___..--''`-._  |  Jacinta Richardson |
`6_ 6  )   `-.  ( ).`-.__.`)  |  Perl Training Australia|
(_Y_.)'  ._   )  `._ `. ``-..-'   |  +61 3 9354 6001|
  _..`--'_..-_/  /--'_.' ,'   | cont...@perltraining.com.au |
 (il),-''  (li),'  ((!.-' |   www.perltraining.com.au   |


___
Bug-gnupod mailing list
Bug-gnupod@nongnu.org
http://lists.nongnu.org/mailman/listinfo/bug-gnupod

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

[Richard van den Berg]

On Fri, June 19, 2009 10:27, H. Langos wrote:
 Good thing you mention tunes2pod. Please try to make your changes
 optional.
 Something like a --merge option to express that you are going to merge
 iTunesDB and GNUtunesDB.xml

I thought about this myself, but we would have to make this a parameter in
.gnupodrc instead because of the automatic tunes2pod run by the other
tools. The reason that I didn't implement it, is that there is no real
downside to do the merging. The attributes in iTunesDB always overwrite
those in GNUtunesDB.xml. If they are the same, no problem. If a file is in
iTunesDB and not in GNUtunesDB.xml, no problem either.

Let me know if you absolutely want it to be optional and I'll make it so.

Cheers,

Richard



___
Bug-gnupod mailing list
Bug-gnupod@nongnu.org
http://lists.nongnu.org/mailman/listinfo/bug-gnupod

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

[Richard van den Berg]

On Fri, June 19, 2009 11:07, H. Langos wrote:
 Maybe the merge code should only be active if your memory saving feature
 is active?

That makes a lot of sense, and it easy to implement. :-) I'll post another
patch tonight.

Cheers,

Richard




___
Bug-gnupod mailing list
Bug-gnupod@nongnu.org
http://lists.nongnu.org/mailman/listinfo/bug-gnupod

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

[Richard van den Berg]

On 6/19/09 11:07 AM, H. Langos wrote:

Maybe the merge code should only be active if your memory saving feature is
active?
  


Here is the new patch that does just that. I love git, it's super fast. :-)

Cheers,

Richard
diff --git a/doc/gnupodrc.example b/doc/gnupodrc.example
index e290224..27b6bc0 100644
--- a/doc/gnupodrc.example
+++ b/doc/gnupodrc.example
@@ -33,24 +33,38 @@
 # NON GLOBAL OPTIONS ##
 
 # *** mktunes.pl ***
+
 ## Specify the iPods name
 # mktunes.ipod-name = Wurstli
 
+## Set --volume boost to +10 percent 
+# mktunes.volume = +10
+
+## Enforce iPod serial number:
+# mktunes.fwguid = 000ba3100310abcf
+
+## Only keep some attributes to make the iTunesDB fit inside small RAM
+## The minimum attributes needed by the iPod are path and title
+## Valid attributes are:
+## title path album artist genre fdesc eq comment category composer group
+## desc podcastguid podcastrss chapterdata subtitle tvshow tvepisode
+## tvnetwork albumartist artistthe keywords sorttitle sortalbum
+## sortalbumartist sortcomposer sorttvshow
+# low_ram_attr = path title artist album
 
 # *** on the go sync (V2 Firmware) ***
+
 ## Uncomment this to skip 'on-the-go' sync
 # otgsync.nosync = 1
 
 # *** tunes2pod.pl ***
+
 ## Uncomment to set '--force' switch to true (DANGEROUS)
 # tunes2pod.force = 1
 
-
-# *** mktunes.pl ***
-## Set --volume boost to +10 percent 
-# mktunes.volume = +10
-## Enforce iPod serial number:
-# mktunes.fwguid = 000ba3100310abcf
+## Setting the low_ram_attr option above causes tunes2pod.pl to sync
+## the attibutes in iTunesDB with those in GNUtunesDB.xml to make sure
+## attributes not present in iTunesDB will be lost
 
 # *** gnupod_search.pl ***
 
diff --git a/src/ext/Mktunes.pm b/src/ext/Mktunes.pm
index c47d679..b503058 100644
--- a/src/ext/Mktunes.pm
+++ b/src/ext/Mktunes.pm
@@ -34,7 +34,7 @@ package GNUpod::Mktunes;

#
# Create and write the iTunesDB file
sub WriteItunesDB {
-   my($self) = @_;
+   my($self,%args) = @_;

my $mhbd_size = 0;
my $mhsd_size = 0;
@@ -52,7 +52,7 @@ package GNUpod::Mktunes;
$mhsd_size = tell(ITUNES);
print ITUNES 
GNUpod::iTunesDB::mk_mhlt({songs=$self-GetFileCount});
foreach my $item (@{$self-GetFiles}) {
-   print ITUNES $self-AssembleMhit($item);
+   print ITUNES $self-AssembleMhit(object=$item, 
keep=$args{keep});
print \r $i files assembled  if ($i++ % 96 == 0);
}
$mhsd_size = tell(ITUNES)-$mhsd_size;
@@ -267,7 +267,9 @@ package GNUpod::Mktunes;

#
# Builds a single mhit with mhod childs
sub AssembleMhit {
-   my($self, $object) = @_;
+   my($self, %args) = @_;
+   my $object  = $args{object};
+   my $keep= $args{keep};
my $mhit= ''; # Buffer for the new mhit
my $mhod_chunks = ''; # Buffer for the childs (mhods)
my $mhod_count  = 0;  # Child counter
@@ -275,6 +277,7 @@ package GNUpod::Mktunes;
foreach my $key (sort keys(%$object)) {
my $value = $object-{$key};
next unless $value; # Do not write empty values
+   next if (scalar keys %$keep  !$keep-{$key}); # Only 
keep specific mhods
my $new_mhod = GNUpod::iTunesDB::mk_mhod({stype=$key, 
string=$value});
next unless $new_mhod; # Something went wrong
$mhod_chunks .= $new_mhod;
diff --git a/src/ext/XMLhelper.pm b/src/ext/XMLhelper.pm
index 748ce22..bb7c88b 100755
--- a/src/ext/XMLhelper.pm
+++ b/src/ext/XMLhelper.pm
@@ -301,19 +301,23 @@ sub mkh {
 }
 
 
-
 #
-# Parses the XML File and do events
-sub doxml {
-   my($xmlin, %opts) = @_;
-   return undef unless (-r $xmlin);
-   ### reset some stuff if we do a second run
+# Reset some stuff if we do a second run
+sub resetxml {
$cpn = undef; #Current PlaylistName
@idpub = ();
@plorder = ();
$xid = 1;
$XDAT = undef;
-   ###
+}
+
+
+#
+# Parses the XML File and do events
+sub doxml {
+   my($xmlin, %opts) = @_;
+   return undef unless (-r $xmlin);
+   resetxml;
my $p;
my $ref = eval {
$p = new XML::Parser(ErrorContext = 0, 
Handlers={Start=\eventer});
diff --git a/src/mktunes.pl b/src/mktunes.pl
index fab4ce3..a35ef62 100644
--- a/src/mktunes.pl
+++ b/src/mktunes.pl
@@ -41,7 +41,7 @@ print mktunes.pl ###__VERSION__### (C) Adrian Ulrich\n;
 
 $opts{mount} 

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

[H. Langos]

Hi Richard,

On Wed, Jun 17, 2009 at 10:00:16PM +0200, Richard van den Berg wrote:
 On 6/17/09 10:41 AM, H. Langos wrote:
 -   $mktunes-WriteItunesDB;
 +   $mktunes-WriteItunesDB(Keep=$opts{'keepattr'});
 

 Shouldn't that be :

 $mktunes-WriteItunesDB( {Keep = $opts{'keepattr'}} );
   

 It turns out it depends on your usage. The form I chose you can used as:

 ($self,%args) = @_;
 @keep=split(/[ ,]+/, $args{Keep});


This form passes the whole hash as a key/value list and creates a new one in
the sub. It will break if you add other parameters as the receiving %args
hash will try to suck up all he parameters that you pass.

$mktunes-WriteItunesDB(Keep=$opts{'keepattr'},foo,bar); 
will let foo and bar end up in %args:

You might try to separate them but 

($self,%args,$localfoo,$localbar) = @_;
will end up
%args == ( Keep = value of $opts{'keepattr'},
  foo  = bar )
$localfoo == undef
$localbar == undef

 While the form with the {} can be used as:

 ($self,$args) = @_;
 @keep=split(/[ ,]+/, $args-{Keep});


This form only passed a single hash reference.

thus 
($self,$args,$localfoo,$localbar) = @_;
will assign a single scalar to $args and $localfoo and $localbar will
get their values.

Did I mention before that I hate perl? :-)


 Both forms are in use in the gnupod code at the moment, I just picked  
 the first..

Check if they are used for the same purpose. :-)

 Here is a new patch with your suggestions and the tunes2pod code. The  
 diff for XMLhelper.pm looks a bit weird, but all I did was move the  
 reset code to a new resetxml() sub. The reset is needed because the  
 playlists are stored in XDAT by doxml() and won't be overwritten by the  
 playlists from the iTunesDB otherwise.

I'll take a look at a revised patch later ... got to hurry to work.

-henrik



___
Bug-gnupod mailing list
Bug-gnupod@nongnu.org
http://lists.nongnu.org/mailman/listinfo/bug-gnupod

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

[Richard van den Berg]

On Thu, June 18, 2009 08:22, H. Langos wrote:
 Did I mention before that I hate perl? :-)

In about every other post to this list. ;-) To me, the things about perl
that I sometimes hate let me very quickly write code that does what I want
at other times. When I started with the tunes2pod code I thought damn,
this is going to be very hard to do. Then one hour later I was done and
it worked. :-)

 Both forms are in use in the gnupod code at the moment, I just picked
 the first..

 Check if they are used for the same purpose. :-)

They are. I'm very good at copy-and-paste and mimicking other people's
style. I realize that most of the gnupod code is not yours, so don't blame
me for using Adrian's coding style. :-)

Cheers,

Richard



___
Bug-gnupod mailing list
Bug-gnupod@nongnu.org
http://lists.nongnu.org/mailman/listinfo/bug-gnupod

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

[H. Langos]

Hi Richard,

On Tue, Jun 16, 2009 at 10:05:16PM +0200, Richard van den Berg wrote:
 On 6/16/09 12:44 AM, H. Langos wrote:
 My first idea now is to
 strip the iTunesDB of the stuff that is optional.
   

 Thanks again for the suggestion, that worked really well. I can now see  
 all my 30415 songs! Whoohoo. :-)

Congratulations! Now please don't go overboard with the patch. :-)

The default behavior for our users should not change. Meaning every possible
mhod should be written. 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.
Your mhod skipping code should only be active if that option is set in your
.gnupodrc or given as command line switch.


Did you find out if shrinking the existing attributes (in contrast to
skipping) had any effect? If it did we could keep some counters for mhods 
and their cumulative size in mktunes and either print out those stats if we
run in verbose mode (which would have to be added :) ) or print out warnings
if certain limits are reached.

 A second patch is needed for tunes2pod  
 which will read the missing attributes from GNUtunesDB.xml, otherwise  
 they are gone forever after tunes2pod is run. I'll work on that later.

I wouldn't worry about that, AFAIK tunes2pod is only run once when you start
working with gnupod. Afterwards the flow of information is mostly this:

OTG Playlist
||
vv
GNUtunesDB.xml==   iTunesDB
^^
||
Play Counts

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. 


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


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.)


 - $mktunes-WriteItunesDB;
 + $mktunes-WriteItunesDB(Keep=$opts{'keepattr'});

Shouldn't that be :

$mktunes-WriteItunesDB( {Keep = $opts{'keepattr'}} );

With the additional curly brackets to express that you are defining an
anonymous hash? 

Maybe perl quesses right but I am not trusting perl to always do the 
right thing. And how would that line look if you add another hashkey?
Would perl put both into one hash or would it pass two separate hashes?


 + my($self, %args) = @_;
 + my $object  = $args{Item};
 + my @keep= split(/[ ,]+/, $args{Keep});
   my $mhit= ''; # Buffer for the new mhit
   my $mhod_chunks = ''; # Buffer for the childs (mhods)
   my $mhod_count  = 0;  # Child counter
 @@ -275,6 +277,7 @@
   foreach my $key (sort keys(%$object)) {
   my $value = $object-{$key};
   next unless $value; # Do not write empty values
 + next unless ($#keep0 || grep(/^$key$/,@keep)); # Only 
 keep specific mhods
   my $new_mhod = GNUpod::iTunesDB::mk_mhod({stype=$key, 
 string=$value});
   next unless $new_mhod; # Something went wrong
   $mhod_chunks .= $new_mhod;

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.


cheers
-henrik

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





___
Bug-gnupod mailing list
Bug-gnupod@nongnu.org
http://lists.nongnu.org/mailman/listinfo/bug-gnupod

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



___
Bug-gnupod mailing list
Bug-gnupod@nongnu.org
http://lists.nongnu.org/mailman/listinfo/bug-gnupod

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

[H. Langos]

On Wed, Jun 17, 2009 at 12:11:06PM +0200, Richard van den Berg wrote:
 On Wed, June 17, 2009 10:41, H. Langos wrote:
  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.

Ahh, I see ... my brain isn't used to unless statements...
I need to translate it to 
  next unless ($#keep0 || grep(/^$key$/,@keep)); #Only keep specific mhods
to
  next if ($#keep=0  !grep(/^$key$/,@keep)); #Only keep specific mhods
to understand it right :-)

  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.

I don't think we need tu hurry that. It just would be nice to have some
numbers when warning users about possible memory issues.
 
  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.

Wow, Floola looks realy nice. I understand your concern now. This would also 
be important for people who use gnupod and gtkpod or any other libgpod based 
applications.

So tunes2pod would have to be improved to incorporate new files and playlists 
and accept changes to existing attributes but to keep those attributes that 
it already finds in GNUtunesDB.xml.

  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.

That would be fun and interesting to see which strategy iTunes chooses to
save space.
 
  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.

Yes but please keep those changes capitalization changes limited to your new
code. I'd liek to put of big cleanup actions until we get the git repository
up and runing. 

I've got things like whitespace cleanup and use warnings; for the old
code on my TODO list.


 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.

from man perlsyn:
   Truth and Falsehood
   The number 0, the strings '0' and '', the empty list (), and
   undef are all false in a boolean context. All other values are true.
   Negation of a true value by ! or not returns a special false value.
   When evaluated as a string it is treated as '', but as a number, it is 
   treated as 0.

Cheers
-henrik




___
Bug-gnupod mailing list
Bug-gnupod@nongnu.org
http://lists.nongnu.org/mailman/listinfo/bug-gnupod

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

[Richard van den Berg]

On 6/17/09 10:41 AM, H. Langos wrote:

-   $mktunes-WriteItunesDB;
+   $mktunes-WriteItunesDB(Keep=$opts{'keepattr'});



Shouldn't that be :

$mktunes-WriteItunesDB( {Keep = $opts{'keepattr'}} );
  


It turns out it depends on your usage. The form I chose you can used as:

($self,%args) = @_;
@keep=split(/[ ,]+/, $args{Keep});

While the form with the {} can be used as:

($self,$args) = @_;
@keep=split(/[ ,]+/, $args-{Keep});

Both forms are in use in the gnupod code at the moment, I just picked 
the first..


Here is a new patch with your suggestions and the tunes2pod code. The 
diff for XMLhelper.pm looks a bit weird, but all I did was move the 
reset code to a new resetxml() sub. The reset is needed because the 
playlists are stored in XDAT by doxml() and won't be overwritten by the 
playlists from the iTunesDB otherwise.


Cheers,

Richard


? .gnupod_version
? Makefile
? autom4te.cache
? config.log
? config.status
? configure
Index: doc/gnupodrc.example
===
RCS file: /sources/gnupod/gnupod/doc/gnupodrc.example,v
retrieving revision 1.7
diff -u -r1.7 gnupodrc.example
--- doc/gnupodrc.example5 Jun 2009 12:55:56 -   1.7
+++ doc/gnupodrc.example17 Jun 2009 19:56:32 -
@@ -51,6 +51,14 @@
 # mktunes.volume = +10
 ## Enforce iPod serial number:
 # mktunes.fwguid = 000ba3100310abcf
+## Only keep some attributes to make the iTunesDB fit inside small RAM
+## The minimum attributes needed by the iPod are path and title
+## Valid attributes are:
+## title path album artist genre fdesc eq comment category composer group
+## desc podcastguid podcastrss chapterdata subtitle tvshow tvepisode
+## tvnetwork albumartist artistthe keywords sorttitle sortalbum
+## sortalbumartist sortcomposer sorttvshow
+# low_ram_attr = path title artist album
 
 # *** gnupod_search.pl ***
 
Index: src/mktunes.pl
===
RCS file: /sources/gnupod/gnupod/src/mktunes.pl,v
retrieving revision 1.86
diff -u -r1.86 mktunes.pl
--- src/mktunes.pl  8 Dec 2007 10:26:08 -   1.86
+++ src/mktunes.pl  17 Jun 2009 19:56:32 -
@@ -41,7 +41,7 @@
 
 $opts{mount} = $ENV{IPOD_MOUNTPOINT};
 GetOptions(\%opts, version, help|h, ipod-name|n=s, mount|m=s, 
volume|v=i, energy|e, fwguid|g=s);
-GNUpod::FooBar::GetConfig(\%opts, {'ipod-name'='s', mount='s', volume='i', 
energy='b', fwguid='s', model='s'}, mktunes);
+GNUpod::FooBar::GetConfig(\%opts, {'ipod-name'='s', mount='s', volume='i', 
energy='b', fwguid='s', model='s', low_ram_attr='s'}, mktunes);
 $opts{'ipod-name'} ||= GNUpod ###__VERSION__###;
 
 
@@ -69,7 +69,12 @@
GNUpod::XMLhelper::doxml($con-{xml}) or usage(Could not read 
$con-{xml}, did you run gnupod_INIT.pl ?);

print \r .$mktunes-GetFileCount. files parsed, assembling 
iTunesDB...\n;
-   $mktunes-WriteItunesDB;
+
+   my $keep = {};
+   foreach(split(/[ ,]+/,$opts{'low_ram_attr'})) {
+   $keep-{$_}++;
+   }
+   $mktunes-WriteItunesDB(keep=$keep);

if($fwguid) {
my $k = GNUpod::Hash58::HashItunesDB(FirewireId=$fwguid, 
iTunesDB=$con-{itunesdb});
Index: src/tunes2pod.pl
===
RCS file: /sources/gnupod/gnupod/src/tunes2pod.pl,v
retrieving revision 1.49
diff -u -r1.49 tunes2pod.pl
--- src/tunes2pod.pl2 Feb 2008 11:42:58 -   1.49
+++ src/tunes2pod.pl17 Jun 2009 19:56:32 -
@@ -35,6 +35,8 @@
 use vars qw(%opts);
 $| = 1;
 
+my $xml_files_parsed=0;
+my $gtdb = {};
 
 print tunes2pod.pl Version ###__VERSION__### (C) Adrian Ulrich\n;
 
@@ -65,6 +67,11 @@
exit(1);
}

+   print  Parsing XML document...\n;
+   GNUpod::XMLhelper::doxml($con-{xml}) or usage(Could not read 
$con-{xml}, did you run gnupod_INIT.pl ?);
+   GNUpod::XMLhelper::resetxml;
+   print \r .$xml_files_parsed. files parsed, converting 
iTunesDB...\n;
+
open(ITUNES, $con-{itunesdb}) or usage(Could not open 
$con-{itunesdb});

while(ITUNES) {}; sysseek(ITUNES,0,0); # the iPod is a slw 
mass-storage device, slurp it into the fs-cache
@@ -197,7 +204,7 @@
 sub MhitEnd {
my($self, %args) = @_;
if($self-{mode} == MODE_SONGS) {
-   GNUpod::XMLhelper::mkfile({file=$self-{ctx}});
  # Add file element to xml
+   GNUpod::XMLhelper::mkfile({file=MergeGtdbCtx($self-{ctx})});  
  # Add file element to xml
$self-{ctx} = ();  
  # And drop this buffer
my $i = ++$self-{count_songs_done};
if($i % 32 == 0) {
@@ -344,11 +351,35 @@
$self-ResetPlaylists; # Resets podcast and normal playlist data
 }
 
-
-
-
-
-
+#
+# Merge GNUtunesDB with ctx
+sub MergeGtdbCtx {
+   

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

[Richard van den Berg]

Patch 2 bound the iTunesDB and GNUtunes.xml on id. Since gnupod 
regenerates the id field on every mktunes run, other software might do 
the same. So I figured binding them on the path attribute would be a 
better idea.


Cheers,

Richard
? .gnupod_version
? Makefile
? autom4te.cache
? config.log
? config.status
? configure
Index: doc/gnupodrc.example
===
RCS file: /sources/gnupod/gnupod/doc/gnupodrc.example,v
retrieving revision 1.7
diff -u -r1.7 gnupodrc.example
--- doc/gnupodrc.example5 Jun 2009 12:55:56 -   1.7
+++ doc/gnupodrc.example18 Jun 2009 04:44:09 -
@@ -51,6 +51,14 @@
 # mktunes.volume = +10
 ## Enforce iPod serial number:
 # mktunes.fwguid = 000ba3100310abcf
+## Only keep some attributes to make the iTunesDB fit inside small RAM
+## The minimum attributes needed by the iPod are path and title
+## Valid attributes are:
+## title path album artist genre fdesc eq comment category composer group
+## desc podcastguid podcastrss chapterdata subtitle tvshow tvepisode
+## tvnetwork albumartist artistthe keywords sorttitle sortalbum
+## sortalbumartist sortcomposer sorttvshow
+# low_ram_attr = path title artist album
 
 # *** gnupod_search.pl ***
 
Index: src/mktunes.pl
===
RCS file: /sources/gnupod/gnupod/src/mktunes.pl,v
retrieving revision 1.86
diff -u -r1.86 mktunes.pl
--- src/mktunes.pl  8 Dec 2007 10:26:08 -   1.86
+++ src/mktunes.pl  18 Jun 2009 04:44:09 -
@@ -41,7 +41,7 @@
 
 $opts{mount} = $ENV{IPOD_MOUNTPOINT};
 GetOptions(\%opts, version, help|h, ipod-name|n=s, mount|m=s, 
volume|v=i, energy|e, fwguid|g=s);
-GNUpod::FooBar::GetConfig(\%opts, {'ipod-name'='s', mount='s', volume='i', 
energy='b', fwguid='s', model='s'}, mktunes);
+GNUpod::FooBar::GetConfig(\%opts, {'ipod-name'='s', mount='s', volume='i', 
energy='b', fwguid='s', model='s', low_ram_attr='s'}, mktunes);
 $opts{'ipod-name'} ||= GNUpod ###__VERSION__###;
 
 
@@ -69,7 +69,12 @@
GNUpod::XMLhelper::doxml($con-{xml}) or usage(Could not read 
$con-{xml}, did you run gnupod_INIT.pl ?);

print \r .$mktunes-GetFileCount. files parsed, assembling 
iTunesDB...\n;
-   $mktunes-WriteItunesDB;
+
+   my $keep = {};
+   foreach(split(/[ ,]+/,$opts{'low_ram_attr'})) {
+   $keep-{$_}++;
+   }
+   $mktunes-WriteItunesDB(keep=$keep);

if($fwguid) {
my $k = GNUpod::Hash58::HashItunesDB(FirewireId=$fwguid, 
iTunesDB=$con-{itunesdb});
Index: src/tunes2pod.pl
===
RCS file: /sources/gnupod/gnupod/src/tunes2pod.pl,v
retrieving revision 1.49
diff -u -r1.49 tunes2pod.pl
--- src/tunes2pod.pl2 Feb 2008 11:42:58 -   1.49
+++ src/tunes2pod.pl18 Jun 2009 04:44:09 -
@@ -35,6 +35,8 @@
 use vars qw(%opts);
 $| = 1;
 
+my $xml_files_parsed=0;
+my $gtdb = {};
 
 print tunes2pod.pl Version ###__VERSION__### (C) Adrian Ulrich\n;
 
@@ -65,6 +67,11 @@
exit(1);
}

+   print  Parsing XML document...\n;
+   GNUpod::XMLhelper::doxml($con-{xml}) or usage(Could not read 
$con-{xml}, did you run gnupod_INIT.pl ?);
+   GNUpod::XMLhelper::resetxml;
+   print \r .$xml_files_parsed. files parsed, converting 
iTunesDB...\n;
+
open(ITUNES, $con-{itunesdb}) or usage(Could not open 
$con-{itunesdb});

while(ITUNES) {}; sysseek(ITUNES,0,0); # the iPod is a slw 
mass-storage device, slurp it into the fs-cache
@@ -197,7 +204,7 @@
 sub MhitEnd {
my($self, %args) = @_;
if($self-{mode} == MODE_SONGS) {
-   GNUpod::XMLhelper::mkfile({file=$self-{ctx}});
  # Add file element to xml
+   GNUpod::XMLhelper::mkfile({file=MergeGtdbCtx($self-{ctx})});  
  # Add file element to xml
$self-{ctx} = ();  
  # And drop this buffer
my $i = ++$self-{count_songs_done};
if($i % 32 == 0) {
@@ -344,11 +351,35 @@
$self-ResetPlaylists; # Resets podcast and normal playlist data
 }
 
-
-
-
-
-
+#
+# Merge GNUtunesDB with ctx
+sub MergeGtdbCtx {
+   my($Ctx) = @_;
+   return $Ctx unless $Ctx-{path}  $gtdb-{$Ctx-{path}};
+   return {%{$gtdb-{$Ctx-{path}}}, %$Ctx};
+}
+
+#
+# Called by doxml if it finds a new file tag
+sub newfile {
+   my($item) = @_;
+   my $file  = $item-{file};
+   my $path  = $file-{path};
+
+   $xml_files_parsed++;
+   print \r .$xml_files_parsed. files parsed if $xml_files_parsed % 
96 == 0;
+
+   return unless $path;
+   $gtdb-{$path} = {};
+   foreach(keys(%$file)){
+   $gtdb-{$path}-{$_}=$file-{$_};
+   }
+}
+ 

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

[H. Langos]

Hi Richard,

On Tue, Jun 16, 2009 at 11:46:47AM +0200, Richard van den Berg wrote:
 On Tue, June 16, 2009 00:44, H. Langos wrote:
  My first idea now is to
  strip the iTunesDB of the stuff that is optional.
 
 Absolutely. I was thinking the same thing. More along the lines of setting
 all titles, artists and albums to a single character. But leaving out
 complete mhods makes a lot more sense.

Shortening might not change a lot if memory management is generous and 
memory alignment of those objects occures on something more than word
borders. But you could find out simply by taking the first not-ok state, 
shorten lots of attributes and try again if the newly genereated iTunesDB 
works.

That would be valuable information regardless of how we solve the problem in
the end.

  Last friday I wrote a patch that will not add them if they are empty. I
  just need to commit it.
 
 Please do, or send me the patch. I'm planning to work on it tonight, and
 it would be good if my code did not conflict with yours. 

It's a simple one line patch .. I already commited it.

 If it works, I'll probably make a switch for it to mktunes.pl.

That might be a good idea. My patch only takes out one attribute for one
file format. Having a universal switch for big collections could be a good
idea. The code there could weed out the not-so-important fields and maybe
limit the size of others. Kind of a last-resort switch for big collections.

  BTW: I've started converting the CVS gnupod repository into a git
  repository.
  If Adrian agrees I'd like to give you write access to it.
 
 That would be very helpful, thanks. I now keep my custom patches in files,
 and reapply them when needed.

To do that you don't even need write access. You simply clone the upstream
repository, create your own local branch and from time to time you pull the 
changes from origin and either merge the master into your private branch
or rebase your private branch onto the HEAD of the master branch.

Lets hope Adrian finds time to answer some of my questions so that I can
compete the transition to git realy soon.

 After rcs, sccs, cvs and svn git will be the 5th versioning system I'll
 have to master. :-)

You will love it. I worked with rcs and cvs before but git simply feels
better. Working with local branches makes things also much easier. And I
actually _enjoy_ working with git. Something that never happend with CVS.

Just one example.. 
When weeding out those empty desc attributes, I inittially commited the 
patch with a '== ' comparison. Comparing a string numerically with the 
empty string (Did I mention that I hate weakly typed languages). Then I 
did some other changes and, commited those. Then I realised that the 
comparison was wrong, reverted that commit fixed the problem once more. 
Now my commit history was correct but looked messy.

1. wrong fix
2. some other change
3, revert of 1.
4. correct fix

git-rebase to the rescue! You can give it a past commit and it will replay
the patches and commits from that past commit to the current state.

So I used git-rebase -i to edit the requence of patches, simply took 
out the initial wrong commit, and the revert and only left 2. and 4. in 
there.

I also could have squashed 1.,3. and 4. into one commit. 

Or instead of reverting in 3. and commiting a new patch in 4. I could have 
used rebase when I realized that 1. was wrong. 
I could have used rebase to replay both commits but pause after 
applying 1. before commiting it, manually  fixed the fix, and then 
continue that git-rebase.

With git you are realy in control of your repository and if you screw 
up you while manipulating the local history can even reset those 
changes (before you run garbage collect to remove unaccessible objects 
from the git repository).

cheers
-henrik



___
Bug-gnupod mailing list
Bug-gnupod@nongnu.org
http://lists.nongnu.org/mailman/listinfo/bug-gnupod

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

[Richard van den Berg]

On 6/16/09 12:44 AM, H. Langos wrote:

My first idea now is to
strip the iTunesDB of the stuff that is optional.
  


Thanks again for the suggestion, that worked really well. I can now see 
all my 30415 songs! Whoohoo. :-)


These are the mhods that gnupod supports:

my %mhod_id = (  title=1, path=2, album=3, artist=4, genre=5, 
fdesc=6, eq=7, comment=8, category=9, composer=12, group=13,
desc=14, podcastguid=15, podcastrss=16, 
chapterdata=17, subtitle=18, tvshow=19, tvepisode=20,
tvnetwork=21, albumartist=22, artistthe=23, 
keywords=24, sorttitle=27, sortalbum=28, sortalbumartist=29,

sortcomposer=30, sorttvshow=31);

I've determined that all you really need is the path and title. This 
created a 25MB iTunesDB. That's only 57% of what I started with. I've 
settled for keeping these mhods:


path
artist
title
album
desc

This gave me an iTunesDB of 32MB (34MB with my title patch) which works 
just fine on my 64MB iPod Video.


Since different users will want to keep different attributes I went for 
a keepattr option in gnupodrc. A second patch is needed for tunes2pod 
which will read the missing attributes from GNUtunesDB.xml, otherwise 
they are gone forever after tunes2pod is run. I'll work on that later.


Cheers,

Richard
? .gnupod_version
? Makefile
? autom4te.cache
? config.log
? config.status
? configure
Index: doc/gnupodrc.example
===
RCS file: /sources/gnupod/gnupod/doc/gnupodrc.example,v
retrieving revision 1.7
diff -u -r1.7 gnupodrc.example
--- doc/gnupodrc.example5 Jun 2009 12:55:56 -   1.7
+++ doc/gnupodrc.example16 Jun 2009 20:01:51 -
@@ -51,6 +51,14 @@
 # mktunes.volume = +10
 ## Enforce iPod serial number:
 # mktunes.fwguid = 000ba3100310abcf
+## Only keep some attributes to make the iTunesDB size as small as possible
+## The minimum attributes needed by the iPod are path and title
+## Valid attributes are:
+## title path album artist genre fdesc eq comment category composer group
+## desc podcastguid podcastrss chapterdata subtitle tvshow tvepisode
+## tvnetwork albumartist artistthe keywords sorttitle sortalbum
+## sortalbumartist sortcomposer sorttvshow
+# keepattr = path title artist album
 
 # *** gnupod_search.pl ***
 
Index: src/mktunes.pl
===
RCS file: /sources/gnupod/gnupod/src/mktunes.pl,v
retrieving revision 1.86
diff -u -r1.86 mktunes.pl
--- src/mktunes.pl  8 Dec 2007 10:26:08 -   1.86
+++ src/mktunes.pl  16 Jun 2009 20:01:51 -
@@ -41,7 +41,7 @@
 
 $opts{mount} = $ENV{IPOD_MOUNTPOINT};
 GetOptions(\%opts, version, help|h, ipod-name|n=s, mount|m=s, 
volume|v=i, energy|e, fwguid|g=s);
-GNUpod::FooBar::GetConfig(\%opts, {'ipod-name'='s', mount='s', volume='i', 
energy='b', fwguid='s', model='s'}, mktunes);
+GNUpod::FooBar::GetConfig(\%opts, {'ipod-name'='s', mount='s', volume='i', 
energy='b', fwguid='s', model='s', keepattr='s'}, mktunes);
 $opts{'ipod-name'} ||= GNUpod ###__VERSION__###;
 
 
@@ -69,7 +69,7 @@
GNUpod::XMLhelper::doxml($con-{xml}) or usage(Could not read 
$con-{xml}, did you run gnupod_INIT.pl ?);

print \r .$mktunes-GetFileCount. files parsed, assembling 
iTunesDB...\n;
-   $mktunes-WriteItunesDB;
+   $mktunes-WriteItunesDB(Keep=$opts{'keepattr'});

if($fwguid) {
my $k = GNUpod::Hash58::HashItunesDB(FirewireId=$fwguid, 
iTunesDB=$con-{itunesdb});
Index: src/ext/Mktunes.pm
===
RCS file: /sources/gnupod/gnupod/src/ext/Mktunes.pm,v
retrieving revision 1.6
diff -u -r1.6 Mktunes.pm
--- src/ext/Mktunes.pm  6 Oct 2007 07:26:52 -   1.6
+++ src/ext/Mktunes.pm  16 Jun 2009 20:01:51 -
@@ -34,7 +34,7 @@

#
# Create and write the iTunesDB file
sub WriteItunesDB {
-   my($self) = @_;
+   my($self,%args) = @_;

my $mhbd_size = 0;
my $mhsd_size = 0;
@@ -52,7 +52,7 @@
$mhsd_size = tell(ITUNES);
print ITUNES 
GNUpod::iTunesDB::mk_mhlt({songs=$self-GetFileCount});
foreach my $item (@{$self-GetFiles}) {
-   print ITUNES $self-AssembleMhit($item);
+   print ITUNES $self-AssembleMhit(Item=$item, 
Keep=$args{Keep});
print \r $i files assembled  if ($i++ % 96 == 0);
}
$mhsd_size = tell(ITUNES)-$mhsd_size;
@@ -267,7 +267,9 @@

#
# Builds a single mhit with mhod childs
sub AssembleMhit {
-   my($self, $object) = @_;
+   my($self, %args) = @_;
+   my $object  = $args{Item};

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

[H. Langos]

On Fri, Jun 12, 2009 at 04:33:43PM +0200, Richard van den Berg wrote:
 On Fri, June 12, 2009 15:21, Heinrich Langos wrote:
  I'd like to have a chain like this:  ok - ok - broken
 
 Here is such a chain:
 
 http://richard.vdberg.org/gnupod/ok/11181/iTunesDB
 http://richard.vdberg.org/gnupod/ok/11182/iTunesDB
 http://richard.vdberg.org/gnupod/ok/11183/iTunesDB
 http://richard.vdberg.org/gnupod/not_ok/11184/iTunesDB
 
  Does this file have anything in its attributes that the files that you
  added successfully (until #11531) did not have? artwork maybe?
 
 Nope. In fact it breaks in the middle of an album. All those songs had
 been added with the same gnupod_addsong.pl --artwork run.

I took a look at that chain (11182-11183-11184) using hexdump, sed and diff
and i came pretty much the the same conclusion... Nothing obviously wrong.
I wonder if it has anything to do with the IDs that playlist entries get.
currently they are just sequentially continued after the last track ID.
Thats why you get such a large number of tiny differences in the master
playlist even when adding just one track.

I don't have much time this weekend but maybe you could check out what
happens if you start giving those IDs from the highest value and decrement
with each entry.

I doubt that this will solve the problem but it probably will help decrease
the noise in the diffs.

BTW thanks for the hint with vbindiff. I was using some mixture of hexdump,
sed (to do linebreaks on mh.. patterns) and diff.

  Could you try to add the SAME file over and over until you get a non
  working state? That would make hunt for an overflow in the internal
  structures even easier. (gnupod_addsong -d allow duplicates).
 
 I can do so tonight, but I was rather hoping of finding this bug without
 having to wipe my iPod. Can't I just create a fake GNUtunesDB.xml with the
 same file over and over again? I don't have the space left to add another
 11000 actual songs to my iPod (unless it's 1 kb or smaller). In my testing
 so far I've just been manually editing GNUtunesDB.xml and running
 mktunes.pl to generate a new iTunesDB.

Skip that. I was hoping to find some pattern in the added file's
characteristic but that will not help.
 
  Best way to get there is a binary search.
 
 That's exactly what I used last night. :-) 15 iterations for 3 files
 is the best you can do.

Good to kown that we can talk more than just basic patterns.

  Did you try adding your collection with gtkpod? I assume it will be much
  faster as it is not written in perl.
 
 Nope. I really don't need a GUI and yet another local database to keep
 track off. I want gnupod! :-)

Me too, I was just wondering if their code produced an iTunesDB that
wouldn't have the same problem. Then we could look for the differences.


cheers
-henrik



___
Bug-gnupod mailing list
Bug-gnupod@nongnu.org
http://lists.nongnu.org/mailman/listinfo/bug-gnupod

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

[Richard van den Berg]

The results I got today were not consistent with yesterday (ok iTunesDBs 
would now be not ok). I learned a lot about the binary format, and 
figured out that by messing with the ArtworkDB (for the performance 
tests) the dbid_1 numbers were now out of whack. However, restoring the 
whole iPod_Control/Artwork directory from yesterday didn't help.


So I decided to clear the iPod_Control/Artwork directory and take out 
the dbid_1, artworkcnt and has_artwork parameters from my GNUtunesDB.xml 
files. I had to do a complete new binary search to find the following ok 
- ok - not_ok sequence:


http://richard.vdberg.org/gnupod/no_artwork/ok/11124/iTunesDB
http://richard.vdberg.org/gnupod/no_artwork/ok/11125/iTunesDB
http://richard.vdberg.org/gnupod/no_artwork/not_ok/11126/iTunesDB

This time file number 11126 seems to be the problem:

 file addtime=3327445665 album=Love Train - The Sound Of 
Philadelphia - Disc 2 artist=Billy Paul bitrate=215 cdnum=0 
cds=0 comment= compilation=1 composer= 
desc=/d01/mp3/marcel/Love Train/The Sound Of Philadelphia/32 - Billy 
Paul - Thanks For Saving My Life.mp3 fdesc=MPEG 1 layer 3 file 
filesize=4813000 genre=Soul id=11152 mediatype=1 
path=:iPod_Control:Music:f23:g0_For_Saving_My_Life.mp3 playcount=0 
songnum=12 songs=0 soundcheck=5070 srate=44100 time=178311 
title=Thanks For Saving My Life volume=0 year=2008 /


On 6/13/09 12:43 PM, H. Langos wrote:

I don't have much time this weekend but maybe you could check out what
happens if you start giving those IDs from the highest value and decrement
with each entry.
  


Interesting idea. See attached patch. It does make the diff a whole lot 
easier to read. And it made a difference as well. With this patch 
applied 11126 suddenly worked.


During the binary search for the next limit I found that if an iTunesDB 
does not work right after I unplug the USB, it *might* work after a 
reset of the iPod. How weird is that? I verified that after the reboot 
the iTunesDB is exactly (cmp -bl) the same as before.


I'm now starting to think it's a RAM issue. According to 
http://ipodlinux.org/wiki/Generations#Fifth_Generation_.285G.29_.2F_Fifth_Generation_Enhanced_.285.5G.29 
my 30GB turned 240GB iPod has 32MB of RAM. Maybe I should try to get a 
hold of a 60GB/80GB version (64GB of RAM)?


Using a binary search including a reset cycle, I was able to get it to 
work with 17759 files (then I got bored):


25026442 not_ok/18053/iTunesDB-plid
25826240 not_ok/18640/iTunesDB-plid
28968870 not_ok/20987/iTunesDB-plid
42047276 not_ok/30415/iTunesDB-plid
22566800 ok/16293/iTunesDB-plid
24192148 ok/17466/iTunesDB-plid
24621122 ok/17759/iTunesDB-plid

You can find the files here: http://richard.vdberg.org/gnupod/no_artwork/
I used the plid extension to show I've used the decrement-plid patch 
to produce those iTunesDBs.


Cheers,

Richard
? .gnupod_version
? Makefile
? autom4te.cache
? config.log
? config.status
? configure
Index: src/ext/Mktunes.pm
===
RCS file: /sources/gnupod/gnupod/src/ext/Mktunes.pm,v
retrieving revision 1.6
diff -u -r1.6 Mktunes.pm
--- src/ext/Mktunes.pm  6 Oct 2007 07:26:52 -   1.6
+++ src/ext/Mktunes.pm  13 Jun 2009 17:05:41 -
@@ -12,7 +12,7 @@
my($class,%args) = @_;

my $self = { Connection=$args{Connection}, Mode=MODE_ADDFILE, 
Artwork=$args{Artwork},
-ArrayFiles = [], CountFiles = 0, Sequence = 0,  
iPodName = $args{iPodName},
+ArrayFiles = [], CountFiles = 0, Sequence = 0, 
PlSequence = 0x, iPodName = $args{iPodName},
 MasterPlaylist = [], Playlists = {}, 
SmartPlaylists = {},
 FuzzyDb_Normal = {}, FuzzyDb_Lowercase = {} };
bless($self,$class);
@@ -91,6 +91,8 @@
sub GetFileCount{ my($self) = @_; return $self-{CountFiles};   
 }
# Increments Sequence counter
sub GetNextId   { my($self) = @_; return ++$self-{Sequence};   
 }
+   # Decrements PlSequence counter
+   sub GetPrevId   { my($self) = @_; return --$self-{PlSequence}; 
 }
# Dispatch connector
sub GetConnection   { my($self) = @_; return $self-{Connection}
 }
# Returns array to files
@@ -240,7 +242,7 @@


foreach my $fqid (@{$cont}) {
-   my $current_id = $self-GetNextId;
+   my $current_id = $self-GetPrevId;
my $current_mhod = 
GNUpod::iTunesDB::mk_mhod({fqid=$fqid});
my $current_mhip = 
GNUpod::iTunesDB::mk_mhip({childs = 1, plid = $current_id, sid=$fqid, 
size=length($current_mhod)});
next unless (defined($current_mhod)  
defined($current_mhip));
___

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

[Heinrich Langos]

Hi Richard,

On Thu, Jun 11, 2009 at 11:15:38PM +0200, Richard van den Berg wrote:
 On 11-6-2009 22:46, Richard van den Berg wrote:
  Maybe it doesn't have to do with the iTunesDB size but a limit of the
  internal structures?

 
 I do think it is a limitation somewhere in iTunesDB. I tried to add back
 my 7 smart playlists (I took them out for testing purposes), which make
 the two largest ok iTunesDB not ok now:
 
 15892984 not_ok/11403p/iTunesDB
 16062708 not_ok/11530p/iTunesDB

I'd look for something that goes in a 3 byte field or maybe something in
iTunesDB.pm uses only 24 bit where it should use 32 bit. Maybe some sub
that is used for the ipod shuffle's iTunesSD (most structures there 
are 3 byte long) gets reused for the the iTunesDB.

I'll try to take a look at your iTunesDB files tomorrow but I don't know if
I'll really have the time.

cheers
-henrik



___
Bug-gnupod mailing list
Bug-gnupod@nongnu.org
http://lists.nongnu.org/mailman/listinfo/bug-gnupod