ok just need to set maximum and min file sizes

4 files changed, 126 insertions, 189 deletions

diff --git a/photoblaster/_file.py b/photoblaster/_file.py
index 8308d8c..ba8c0a6 100644
--- a/photoblaster/_file.py
+++ b/photoblaster/_file.py
@@ -94,7 +94,7 @@ class File(object):
             ident = (Popen(
                 [BIN_IDENTIFY, self.get_filepath()],
                 stdout=PIPE).communicate()[0]).split(" ")
-            return ident[2].split("x")
+            return [int(n) for n in ident[2].split("x")]
         except Exception as e:
             sys.stderr.write("Unable to get file dimensions:\n")
             sys.stderr.write("%s \n" % e)
diff --git a/photoblaster/modules/pbconcat/__init__.py b/photoblaster/modules/pbconcat/__init__.py
index 797d90f..67f7f96 100644
--- a/photoblaster/modules/pbconcat/__init__.py
+++ b/photoblaster/modules/pbconcat/__init__.py
@@ -1,26 +1,73 @@
 from photoblaster.modules import ModuleBase
 from photoblaster.config import BIN_CONVERT, BIN_COMPOSITE, DEFAULT_FINALFORMAT
 from photoblaster._file import File
+from subprocess import call
+import copy
 _DEFAULT_DIRECTION = 'right'
 
+class SizeNotOkError(Exception):
+    pass
 
 class PbConcatFile(File):
-    def __init__(self, *args, **kwargs):
-        super(PbConcatFile, self).__init__()
-        self._frames_cache = self.get_frames()
-        self._width = None
-        self._height = None
 
     @classmethod
-    def from_file(cls, file):
-        cls()
+    def from_file(cls, _file):
+        obj = copy.copy(_file)
+        obj.__class__ = PbConcatFile
+        if not obj.is_size_ok():
+            raise SizeNotOkError
+        return obj
 
-    def set_frames_cache(self, frames):
-        self._frames_cache = frames
-
-    def get_frames_cache(self):
+    def get_frames(self):
+        if not hasattr(self, "_frames_cache"):
+            self._frames_cache = super(PbConcatFile, self).get_frames()
         return self._frames_cache
 
+    def set_frames(self, frames):
+        self._frames_cache = frames 
+
+    def get_dimensions(self):
+        if not hasattr(self, "_dimensions_cache"):
+            self._dimensions_cache = super(PbConcatFile, self).get_dimensions()
+        return self._dimensions_cache
+
+    def set_dimensions(self, **kwargs):
+        self._dimensions_cache = kwargs
+
+    def add_frames(self, new_count):
+        """
+        adds frames to a gif to make it the same length as another gif
+        """
+        old_frames = self.get_frames()
+        new_frames = []
+        for i in xrange(0, int(new_count/len(old_frames))):
+            new_frames += old_frames
+        new_frames += old_frames[0:(new_count % len(old_frames))]
+        self.set_frames(new_frames)
+
+    def reduce_frames(self, new_count):
+        self.set_frames(self.get_frames()[0:new_count])
+
+    def compile_frames(self, module=None):
+        if len(self.get_frames()) == 1:
+            return
+        cmd = [BIN_CONVERT]
+        cmd += ['-dispose', '2']
+        cmd += ['-loop', '0']
+        cmd += [frame.get_filepath() for frame in self.get_frames()]
+        cmd += [self.get_filepath()]
+        call(cmd)
+        if module:
+            module.commands.append(" ".join(cmd))
+        self.optimize_gif()
+
+    def is_size_ok(self):
+        ok = True
+        if any(
+            [dimension > _MAX_DIMENSION for dimension in self.get_dimensions()]
+        ) or len(self.get_frames()) > _MAX_FRAME_LENGTH:
+            ok = False
+        return ok 
 
 
 class PbConcat(ModuleBase):
@@ -70,144 +117,65 @@ class PbConcat(ModuleBase):
             kwargs,
             classname=self.__class__.__name__,
             module=self)
-        self.set_output_file(
-            File(
+        output_file = File(
                 classname=self.__class__.__name__,
                 extension=self.params.finalformat,
                 username=self.params.username
-            )
+        )
+        self.set_output_file(
+            PbConcatFile.from_file(output_file)
+        )
+        self.params.img_url1.set_file(
+            PbConcatFile.from_file(self.params.img_url1.get_file())
+        )
+        self.params.img_url2.set_file(
+            PbConcatFile.from_file(self.params.img_url2.get_file())
         )
         self._db_url_param = self.params.img_url1.url
 
-    def _get_frames_of_equal_count(self):
+
+    def _make_frames_count_equal(self):
+        im1 = self.params.img_url1.get_file()
+        im2 = self.params.img_url2.get_file()
         if self.params.finalformat in ["png", "jpg"]:
-            self.params.img_url1.get_file().compress_to_single_frame()
-            self.params.img_url2.get_file().compress_to_single_frame()
-            return ([self.params.img_url1], [self.params.img_url2])
+            im1.compress_to_single_frame()
+            im2.compress_to_single_frame()
+            return
+        im_shorter = im1 if len(im1.get_frames()) < len(im2.get_frames()) else im2
+        im_longer  = im2 if im_shorter == im1 else im1
+        if self.params.frames_match == "shorter":
+            im_longer.reduce_frames(len(im_shorter.get_frames()))
+        else:
+            im_shorter.add_frames(len(im_longer.get_frames()))
+
+    def _merge_frames(self):
         im1_frames = self.params.img_url1.get_file().get_frames()
         im2_frames = self.params.img_url2.get_file().get_frames()
-        #here well it's similar but not exactly, let's just keep it local for each method
-        if len(im1_frames) > len(im2_frames):
-            if self.params.frames_match == "shorter":
-                im1_frames = self._reduce_frames(
-                    im1_frames, len(im2_frames)
-                )
-            else:
-                im2_frames = self._add_frames(
-                    im2_frames, len(im1_frames)
-                )
-        elif len(im1_frames) < len(im2_frames):
-            if self.params.frames_match == "shorter":
-                im2_frames = self._reduce_frames(
-                    im2_frames, len(im1_frames)
-                )
-            else:
-                im1_frames = self._add_frames(
-                    im1_frames, len(im2_frames)
-                )
-        return (im1_frames, im2_frames)
-
-    def _merge_frames_of_equal_count(self, frames1, frames2):
         frames = []
-        for i in xrange(0, len(frames1)):
-            frames.append(self._concat_static(frames1[i], frames2[i]))
-        return frames
+        for i in xrange(0, len(im1_frames)):
+            frames.append(self._concat_static(im1_frames[i], im2_frames[i]))
+        self.get_output_file().set_frames(frames)
 
     def _match_dimensions(self):
-        #so here I get dimensions store them into variables
-        im1 = {
-            'image': self.params.img_url1.get_file(),
-        }
-        im2 = {
-            'image': self.params.img_url2.get_file(),  # => def _im2   but without hashes, hash will be only used in this function, rest can use just plain object.
-        }
-        #thing is that I have no set_frames method for the image_file
-        #also get_frames() calls a shell command, and I'd like to avoid calling it a ton, unless you think it's fine you can make a subclass for file like giffile or
-        #something, and as for get_frames, you just add caching, call shell once and write into internal variable, if it's set - return it, if not, call shell.
-        #yeah of course but how this works is
-        #I get the frames of two gifs
-        #I add frames one of the frames arrays by looping
-        #I merge the two frame arrays of equal length
-        #then I make a new gif
-
-        #so I wouldn't want to set the frames on the original gifs at all, just need to store them into a variable, do you know what I mean?
-        #i think so, just don't call set_frames on original gifs, it's fine to have code that does something only for part of application.
-        #but if I set the frames on the original gifs, I have to recompile the gif and that will make the program run wayy slower
-        #so yu need frames only as variable, not actually change it? right
-        #i need them only to merge two gifs together frame by frame, but the gifs I'm merging I don't need to change well keep get_frames then, and make
-        #something like local variable that you can set on file, like a temporary variable inside object, this way you don't have to recomplie gif.
-        #what could it look like?
-        #gif = GifFile("some.gif"); gif.get_frames(); # calls shell,  gif.get_frames() # gets from local variable.  gif._temporary_frames = myframes
-        #i guess something like this. hmm. not sure about this because I only need to do this temporary frames method in this plugin
-        #I'll never need it outside of this script. shouldn't I just store it locally? yeah you can store it in local variable, or make a local subclass of file or giffile
-        #which implemente just this method. ahhh awesome
-        #so I call it MyGifFile(GifFile) something like that? PbConcatGifFile maybe.
-        #ok. I find this part of programming, learning how to structure a project, very difficult and elusive, is it essentially something that
-        #not many coders know who to do well? looks so. ok I understand, and the only way to learn is to just code a lot and make mistakes,
-        #gradually get better that way? yep I see...one more quick question
-
-
-
-        images = [ im1, im2 ]
-        map(lambda i: i['width'], i['height'] = i['image'].get_dimensions(), images)
-
-        param_name = self.params.merge_direction == "right" ? "height" : "width"
-        if self.params.merge_direction == "right" and im1[param_name] != im2[param_name]:
-            im_scale = im1[param_name] > im2[param_name] and self.params.dimensions_match == "smallest" ?
-                im1 :
-                im2
-            im_scale_by = im_scale == im1 ? im2 : im1
-            im_scale['image'].resize(height=im_scale_by[param_name], module=self)
-
-        # something like this, could be bugs but you get the idea.
-        #so the main idea that I need to ask about is that
-        #file is already a class
-        #the image url param is also already a class
-        #but it's cool to store things in hashes that refer to the classes? sure it's temporary "object" that helps to solve this
-        #so hashes are just fair game for anything procedural and I can throw them out without worrying about integrating
-        #them into the class based system? yep
-        #should im1 and im2 hashes be hidden attributes of this class? i guess no need, is there more code like this? yep
-
-#        if self.params.merge_direction == "right" and \
-#                im1_height != im2_height:
-#            """match heights"""
-#            #ok so if the first image is taller than the second,
-#            #and the dimensions param says to go by the smallest, shrink
-#            #the first image, if the dimensions_param says to go by the tallest
-#            #make the second image bigger...do you see what I mean when I say this
-#            #is sort of confusing? yeah, let's rewrite it
-#            if im1_height > im2_height:
-#                if self.params.dimensions_match == "smallest":
-#                    self.params.img_url1.get_file().resize(
-#                        height=im2_height, module=self)
-#                else:
-#                    self.params.img_url2.get_file().resize(
-#                        height=im1_height, module=self)
-#            else:
-#                if self.params.dimensions_match == "smallest":
-#                    self.params.img_url2.get_file().resize(
-#                        height=im1_height, module=self)
-#                else:
-#                    self.params.img_url1.get_file().resize(
-#                        height=im2_height, module=self)
-#        elif self.params.merge_direction == "down" and \
-#                im1_width != im2_width:
-#            """match widths"""
-#            if im1_width > im2_width:
-#                if self.params.dimensions_match == "smallest":
-#                    self.params.img_url1.get_file().resize(
-#                        width=im2_width, module=self)
-#                else:
-#                    self.params.img_url2.get_file().resize(
-#                        width=im1_width, module=self)
-#            else:
-#                if self.params.dimensions_match == "smallest":
-#                    self.params.img_url2.get_file().resize(
-#                        width=im1_width, module=self)
-#                else:
-#                    self.params.img_url1.get_file().resize(
-#                        width=im2_width, module=self)
+        im1 = self.params.img_url1.get_file()
+        im2 = self.params.img_url2.get_file()
+        """if merging right, match height, else width
+        """
+        dimensions_idx = 1 if self.params.merge_direction == "right" else 0
+        dimension_name = "height" if self.params.merge_direction == "right" else "width"
+        #if heights are the same, return
+        if im1.get_dimensions()[dimensions_idx] == im2.get_dimensions()[dimensions_idx]:
+            return
+        is_im1_larger = True if im1.get_dimensions()[dimensions_idx] > \
+            im2.get_dimensions()[dimensions_idx] and \
+            self.params.dimensions_match == "smaller" else False #flips logic
 
+        im_scale, im_scale_by = (im1, im2) if is_im1_larger else (im2, im1) 
+        im_scale.resize(**{
+            dimension_name: im_scale_by.get_dimensions()[dimensions_idx],
+            'module': self
+        })
+        
     def _concat_static(self, image1, image2):
         tempfile = File(
             is_temp=True,
@@ -229,56 +197,19 @@ class PbConcat(ModuleBase):
         self._call_cmd(cmd)
         return tempfile
 
-    def _merge_all_frames(self, frames):
-        if len(frames) > 1 and self.params.finalformat == "gif":
-            self._frames_to_gif(frames)
-        else:
-            frames[0].set_filepath(
-               self.get_output_file().get_filepath(),
-               module=self
-            )
-
-    def _frames_to_gif(self, frames):
-        cmd = [BIN_CONVERT]
-        cmd += ['-dispose', '2']
-        cmd += ['-loop', '0']
-        cmd += [frame.get_filepath() for frame in frames]
-        cmd += [self.get_output_file().get_filepath()]
-        self._call_cmd(cmd)
-        self.get_output_file().optimize_gif()
-
-    def _add_frames(self, frames, frame_number):
-        """
-        adds frames to a gif to make it the same length as another gif
-        """
-        new_frames = []
-        for i in xrange(0, int(frame_number/len(frames))):
-            new_frames += frames
-        new_frames += frames[0:(frame_number % len(frames))]
-        return new_frames
-
-    def _reduce_frames(self, frames, frame_number):
-        return frames[0:frame_number]
+    def _finalize_image(self):
+        if len(self.get_output_file().get_frames()) > 1:
+            self.get_output_file().compile_frames(module=self)
 
     def create(self):
         self._match_dimensions()
-        #also see here how I'm passing back a tuple and just passing the tuple into the next function for no reason?
-        #if I had a global im1 and im2 hash, I could just keep the frames in there, you know? well could be
-        #so the new create would look like this
-        #match_dimensions()
-        #make_frames_count_equal()
-        #merge_frames()
-        #finalize_image()
-
-        #does that seem more clear? yeah i think you just need an alias for self.parameters.
-        frames1, frames2 = self._get_frames_of_equal_count()
-        frames_merged = self._merge_frames_of_equal_count(frames1, frames2)
-        self._merge_all_frames(frames_merged)
+        self._make_frames_count_equal()
+        self._merge_frames()
+        self._finalize_image() 
         super(PbConcat, self).create()
 
-    #in general this looks ok, right? yeah but code in ifs that looks same way looks a bit weird
-    #yeah I was trying to think of ways to wrap it into lambdas but it was very confusing
     def _add_canvas(self, image, canvas_color="transparent"):
+        """maybe remove"""
         canvasfile = File(
             is_temp=True,
             extension="jpg",
diff --git a/photoblaster/param/img_url.py b/photoblaster/param/img_url.py
index 6e87c71..f85fa63 100644
--- a/photoblaster/param/img_url.py
+++ b/photoblaster/param/img_url.py
@@ -117,3 +117,9 @@ class Img_url(Param):
 
     def get_file(self):
         return self._file
+
+    def set_file(self, _file):
+        if not isinstance(_file, File):
+            raise TypeError(
+                "%s is not type %s" % (type(_file), File.__class__.__name__))
+        self._file = _file
diff --git a/run_module_examples.py b/run_module_examples.py
index 1aae58c..27fd13c 100644
--- a/run_module_examples.py
+++ b/run_module_examples.py
@@ -4,11 +4,11 @@ from photoblaster.modules import Modules
 
 modules = Modules()
 for module_name in modules.list_modules():
-#    if module_name == "pbconcat":
-    print "\n\n\n"
-    print "running example for %s" % module_name
-    cls = modules.get_module(module_name)
-    instance = cls.example_run()
-    instance.get_output_file().s3move()
-    print instance.get_output_file().as_dict()
-    instance.db_send()
+    if module_name == "pbconcat":
+        print "\n\n\n"
+        print "running example for %s" % module_name
+        cls = modules.get_module(module_name)
+        instance = cls.example_run()
+        instance.get_output_file().s3move()
+        print instance.get_output_file().as_dict()
+        instance.db_send()