[PATCH 1/3] Return offsets instead of chunk objects from chunkers

[Michael Hanselmann]

This patch removes duplicate code (instantiation of
Brackup::PositionedChunk) and is needed for further changes.
---
 lib/Brackup/Chunker/Default.pm |   22 ++++++++-----------
 lib/Brackup/Chunker/MP3.pm     |   35 +++++++++---------------------
 lib/Brackup/File.pm            |   45 +++++++++++++++++++++++++++++++++++++++-
 3 files changed, 64 insertions(+), 38 deletions(-)

diff --git a/lib/Brackup/Chunker/Default.pm b/lib/Brackup/Chunker/Default.pm
index 9b1f29b..a661f21 100644
--- a/lib/Brackup/Chunker/Default.pm
+++ b/lib/Brackup/Chunker/Default.pm
@@ -1,26 +1,22 @@
 package Brackup::Chunker::Default;
 use strict;
+use warnings;
 
-sub chunks {
-    my ($class, $file) = @_;
-    my @chunk_list;
+sub get_offsets {
+    my ($class, $file, $cb) = @_;
 
-    my $root       = $file->root;
+    my $root = $file->root;
     my $chunk_size = $root->chunk_size;
-    my $size       = $file->size;
-
+    my $size = $file->size;
     my $offset = 0;
+
     while ($offset < $size) {
         my $len = _min($chunk_size, $size - $offset);
-        my $chunk = Brackup::PositionedChunk->new(
-                                                  file   => $file,
-                                                  offset => $offset,
-                                                  length => $len,
-                                                  );
-        push @chunk_list, $chunk;
+
+        $cb->($offset, $len);
+
         $offset += $len;
     }
-    return @chunk_list;
 }
 
 sub _min {
diff --git a/lib/Brackup/Chunker/MP3.pm b/lib/Brackup/Chunker/MP3.pm
index 5bf5b7f..fe5cf46 100644
--- a/lib/Brackup/Chunker/MP3.pm
+++ b/lib/Brackup/Chunker/MP3.pm
@@ -1,15 +1,16 @@
 package Brackup::Chunker::MP3;
 use strict;
+use warnings;
 
 my $HAVE_MP3_INFO;
 BEGIN {
     $HAVE_MP3_INFO = eval "use MP3::Info (); 1;";
 }
 
-sub chunks {
-    my ($class, $file) = @_;
-    my @chunk_list;
-    die "Required module MP3::Info not found.  Needed by the MP3 file chunker.\n"
+sub get_offsets {
+    my ($class, $file, $cb) = @_;
+
+    die "Required module MP3::Info not found. Needed by the MP3 file chunker.\n"
         unless $HAVE_MP3_INFO;
 
     my $file_path = $file->path;
@@ -19,40 +20,26 @@ sub chunks {
     return () unless -e $file_path;
 
     my ($music_offset, $music_size) = main_music_range($file_path);
-    my $size       = $file->size;
+    my $size = $file->size;
 
     # add the ID3v2 header, if necessary:
     if ($music_offset != 0) {
-        push @chunk_list, Brackup::PositionedChunk->new(
-                                                        file   => $file,
-                                                        offset => 0,
-                                                        length => $music_offset,
-                                                        );
+        $cb->(0, $music_offset);
     }
 
     # add the music chunk (one big chunk, at least for now)
-    push @chunk_list, Brackup::PositionedChunk->new(
-                                                    file   => $file,
-                                                    offset => $music_offset,
-                                                    length => $music_size,
-                                                    );
+    $cb->($music_offset, $music_size);
 
     # add the ID3v1 header chunk, if necessary:
     my $music_end = $music_offset + $music_size;
     if ($music_end != $size) {
-        push @chunk_list, Brackup::PositionedChunk->new(
-                                                        file   => $file,
-                                                        offset => $music_end,
-                                                        length => $size - $music_end,
-                                                        );
+        $cb->($music_end, $size - $music_end);
     }
-
-    return @chunk_list;
 }
 
 sub main_music_range {
-    my $file = shift;
-    my $size = -s $file;
+    my ($file) = @_;
+    my $size = $file->size;
 
     # if not an mp3, include the whole file
     unless ($file =~ /\.mp3$/i) {
diff --git a/lib/Brackup/File.pm b/lib/Brackup/File.pm
index 13c0b24..8dc2ee9 100644
--- a/lib/Brackup/File.pm
+++ b/lib/Brackup/File.pm
@@ -120,7 +120,50 @@ sub chunks {
 
     # Get the appropriate FileChunker for this file type,
     # then pass ourselves to it to get our chunks.
-    my @chunk_list = $self->file_chunker->chunks($self);
+    my @chunk_list;
+
+    my $size = $self->size;
+    my $last_offset = undef;
+    my $last_len = undef;
+
+    $self->file_chunker->get_offsets($self, sub {
+        my ($offset, $len) = @_;
+
+        # Lots of verification on offset and length
+        if ($offset + $len > $size) {
+            die "Chunker seeks behind file";
+
+        } elsif ($len <= 0) {
+            die "Chunker returned invalid length";
+
+        }
+
+        if (defined($last_offset)) {
+            if ($offset <= $last_offset) {
+                die "Chunker didn't advance";
+            }
+
+            if (defined($last_len) and $offset != ($last_offset + $last_len)) {
+                die "Chunker jumped";
+            }
+
+        } elsif ($offset != 0) {
+            die "Chunker didn't start at offset 0";
+        }
+
+        push @chunk_list, new Brackup::PositionedChunk(
+            file => $self,
+            offset => $offset,
+            length => $len,
+        );
+
+        $last_offset = $offset;
+        $last_len = $len;
+    });
+
+    unless (($last_offset + $last_len) == $size) {
+        die "Chunker didn't return complete file";
+    }
 
     $self->{chunks} = \@chunk_list;
     return @chunk_list;
-- 
1.5.5.3