# HG changeset patch
# User Pantxo Diribarne <pantxo.diribarne@gmail.com>
# Date 1544879185 -3600
#      Sat Dec 15 14:06:25 2018 +0100
# Node ID cb822f46f64a982179a59e27b81954dedb7eaee4
# Parent  40e1857e14d66da141ac34ca75a542214f82afbb
movie.m: Improve input checking and FPS accuracy

* movie.m: Improve input checking. Add corresponding BIST. Make use of a local
  timer rather than reseting the global timer. Call "tic" only once and compare
  "toc" to time table for better FPS accuracy. Tag target image so that movie
  can eventually reuse it for further movie play.

diff -r 40e1857e14d6 -r cb822f46f64a scripts/image/movie.m
--- a/scripts/image/movie.m	Sat Dec 15 12:05:58 2018 +0100
+++ b/scripts/image/movie.m	Sat Dec 15 14:06:25 2018 +0100
@@ -1,5 +1,5 @@
 ## Copyright (C) 2017 Pantxo Diribarne
-## 
+##
 ## This file is part of Octave.
 ##
 ## Octave is free software: you can redistribute it and/or modify it
@@ -56,55 +56,58 @@
 
 function movie (varargin)
 
-  if (nargin == 0)
+  nargs = numel (varargin);
+
+  if (nargs == 0 || nargs > 4)
     print_usage ();
   endif
 
   ## Extract possible handle argument
   hax = [];
-  if (isaxes (varargin{1}))
-    hax = varargin{1};
-    varargin(1) = [];
-  elseif (isfigure (varargin{1}))
-    hax = get (varargin{1}, "currentaxes");
-    if (isempty (hax))
-      hax = axes ("parent", varargin{1});
+  if (ishghandle (varargin{1}))
+    if (isaxes (varargin{1}))
+      hax = varargin{1};
+    elseif (isfigure (varargin{1}))
+      figure (varargin{1});
+      hax = gca ();
+    else
+      error ("movie: expect H to be a handle to an axes or figure")
     endif
     varargin(1) = [];
+    nargs--;
   endif
 
   ## Extract other arguments
   n = 1;
   fps = 12;
   idx = [];
-  nargs = numel (varargin);
-  if (nargs == 0)
-    print_usage ();
-  elseif (nargs >= 1)
-    mov = varargin{1};
-    if (! isfield (mov, "cdata") || ! isfield (mov, "colormap"))
-      error ("movie: MOV must be a frame struct array");
+
+  mov = varargin{1};
+  if (! isfield (mov, "cdata") || ! isfield (mov, "colormap"))
+    error ("movie: expect MOV to be a frame struct array")
+  elseif (numel (mov) < 2)
+    error ("movie: expect MOV to contain at least two frames")
+  endif
+
+  if (nargs > 1)
+    n = varargin{2};
+    if (! isnumeric (n) || any (fix(n) != n) || any (n == 0))
+      error ("movie: expect N to be non zero integer")
     endif
 
-    if (nargs >= 2)
-      n = varargin{2};
-      if (! isindex (abs (n(1))))
-        error ("movie: N must be a non-zero integer");
-      elseif (! isscalar (n))
-        idx = n(2:end)(:)';
-        n = n(1);
-        if (! isindex (idx, numel (mov)))
-          error (["movie: All elements N(2:end) must be valid indices ", ...
-                  "into the MOV struct array"]);
+    if (! isscalar (n))
+      idx = n(2:end)(:).';
+      n = n(1);
+      if (any (idx < 1) || any (idx > numel (mov)))
+        error (["movie: expect all elements N(2:end) to be indices ", ...
+                "in the MOV struct array"]);
+      endif
+    endif
 
-        endif
-      endif
-        
-      if (nargs >= 3)
-        fps = varargin{3};
-        if (! (isnumeric (fps) && isscalar (fps) && fps > 0))
-          error ("movie: FPS must be a numeric scalar > 0");
-        endif
+    if (nargs > 2)
+      fps = varargin{3};
+      if (! isnumeric (fps) || ! isscalar (fps))
+        error ("movie: expect FPS to be numeric scalar")
       endif
     endif
   endif
@@ -124,28 +127,52 @@
     n = -n;
     tmp = repmat ([idx fliplr(idx)], 1, fix (n/2));
     if (fix (n/2) != n/2)
-      idx = [tmp, idx];
+      idx = [tmp idx];
     else
       idx = tmp;
     endif
   endif
 
-  tau = 1/fps;
+  ## Initialize graphics objects
+  if (isempty (mov(1).cdata))
+    error ("movie: found empty image data");
+  endif
+
+  him = findobj (hax, "-depth", 1, "tag", "__movie_frame__");
+  if (isempty (him))
+    him = image ("parent", hax, "tag", "__movie_frame__", ...
+                 "cdata", mov(1).cdata);
+  else
+    set (him, "cdata", mov(1).cdata)
+  endif
+
   set (hax, "ydir", "reverse", "visible", "off");
-  tic ();
-  him = image ("parent", hax, "cdata", mov(1).cdata);
+
+  ## Initialize the timer
+  t = 1/fps;
+  tid = tic ();
+
   for ii = idx
-    pause (tau - toc ());
-    tic ();
-    if (! isempty (mov(ii).colormap))
+    cdata = mov(ii).cdata;
+    if (isempty (cdata))
+      error ("movie: found empty image data");
+    endif
+
+    if(! isempty (mov(ii).colormap))
       set (hax, "colormap", mov(ii).colormap)
     endif
-    set (him, "cdata", mov(ii).cdata);
+
+    laps = t - toc (tid);
+    if (laps > 0)
+      pause (laps);
+    endif
+
+    set (him, "cdata", cdata);
+    t += 1/fps;
   endfor
-  
+
 endfunction
 
-
 %!demo
 %! nframes = 20;
 %! colors = jet (nframes);
@@ -191,7 +218,7 @@
 %!   mov(ii).cdata = im;
 %! endfor
 %! clf ();
-%! title "Play downsampled movie 5 times" 
+%! title "Play downsampled movie 5 times"
 %! movie (mov, [5 1:3:nframes]);
 
 %!demo
@@ -206,12 +233,24 @@
 %!   mov(ii) = getframe ();
 %! endfor
 %! clf ();
-%! movie (mov, 5, 25);
+%! movie (mov, 3, 25);
 
-## Test input validation
-%!error movie ()
-%!error <MOV must be a frame struct array> movie ({2})
-%!error <N must be a non-zero integer>
-%! movie (struct ("cdata", [], "colormap", []), 2.3);
-%!error <N must be a non-zero integer>
-%! movie (struct ("cdata", [], "colormap", []), [2.3 -6]);
+%!error <expect MOV to be a frame struct array> movie (3.4)
+%!error <expect MOV to contain at least two frames>
+%! movie (struct ("cdata", [], "colormap", []))
+%!error <expect N to be non zero integer>
+%! movie (struct ("cdata", {[], []}, "colormap", []), 2.3)
+%!error <expect all elements N\(2:end\) to be indices in the MOV struct array>
+%! movie (struct ("cdata", {[], []}, "colormap", []), [1 -1])
+%!error <expect all elements N\(2:end\) to be indices in the MOV struct array>
+%! movie (struct ("cdata", {[], []}, "colormap", []), [1 5])
+%!error <expect FPS to be numeric scalar>
+%! movie (struct ("cdata", {[], []}, "colormap", []), 1, "a")
+%!error <expect FPS to be numeric scalar>
+%! movie (struct ("cdata", {[], []}, "colormap", []), 1, {})
+%!error <expect FPS to be numeric scalar>
+%! movie (struct ("cdata", {[], []}, "colormap", []), 1, [1 1])
+%!error <found empty image data>
+%! movie (struct ("cdata", {[], []}, "colormap", []))
+%!error <found empty image data>
+%! movie (struct ("cdata", {rand(2), []}, "colormap", []))
