Merge pull request #88 from shadlaws/fix_1941

#1941, 1948 - Fix possible warnings in movie and graphics helpers, add functions to convert between seconds and hh:mm:ss.dd (and their unit tests).

3 files changed, 78 insertions, 8 deletions

diff --git a/modules/gallery/helpers/graphics.php b/modules/gallery/helpers/graphics.php
index e7c5da68..c840b920 100644
--- a/modules/gallery/helpers/graphics.php
+++ b/modules/gallery/helpers/graphics.php
@@ -157,7 +157,7 @@ class graphics_Core {
         if ($input_item->is_movie()) {
           // Convert the movie filename to a JPG first, delete anything that might already be there
           $output_file = legal_file::change_extension($output_file, "jpg");
-          unlink($output_file);
+          @unlink($output_file);
           // Run movie_extract_frame events, which can either:
           //  - generate an output file, bypassing the ffmpeg-based movie::extract_frame
           //  - add to the options sent to movie::extract_frame (e.g. change frame extract time,
diff --git a/modules/gallery/helpers/movie.php b/modules/gallery/helpers/movie.php
index 6d70ab2d..3524a8f9 100644
--- a/modules/gallery/helpers/movie.php
+++ b/modules/gallery/helpers/movie.php
@@ -66,7 +66,7 @@ class movie_Core {
    * @param string     $output_file
    * @param array      $movie_options (optional)
    */
-   static function extract_frame($input_file, $output_file, $movie_options=NULL) {
+  static function extract_frame($input_file, $output_file, $movie_options=null) {
     $ffmpeg = movie::find_ffmpeg();
     if (empty($ffmpeg)) {
       throw new Exception("@todo MISSING_FFMPEG");
@@ -74,17 +74,17 @@ class movie_Core {
 
     list($width, $height, $mime_type, $extension, $duration) = movie::get_file_metadata($input_file);
 
-    if (is_numeric($movie_options["start_time"])) {
+    if (isset($movie_options["start_time"]) && is_numeric($movie_options["start_time"])) {
       $start_time = max(0, $movie_options["start_time"]); // ensure it's non-negative
     } else {
       $start_time = module::get_var("gallery", "movie_extract_frame_time", 3); // use default
     }
     // extract frame at start_time, unless movie is too short
     $start_time_arg = ($duration >= $start_time + 0.1) ?
-      "-ss " . date("H:i:s", mktime(0,0,$start_time,0,0,0,0)) : "";
+      "-ss " . movie::seconds_to_hhmmssdd($start_time) : "";
       
-    $input_args = $movie_options["input_args"] ? $movie_options["input_args"] : "";
-    $output_args = $movie_options["output_args"] ? $movie_options["output_args"] : "";
+    $input_args = isset($movie_options["input_args"]) ? $movie_options["input_args"] : "";
+    $output_args = isset($movie_options["output_args"]) ? $movie_options["output_args"] : "";
 
     $cmd = escapeshellcmd($ffmpeg) . " $input_args -i " . escapeshellarg($input_file) .
       " -an $start_time_arg -an -r 1 -vframes 1" .
@@ -149,8 +149,8 @@ class movie_Core {
     $mime_type = legal_file::get_movie_types_by_extension($extension);
     $mime_type = $mime_type ? $mime_type : "video/x-flv"; // No MIME found?  Default to video/x-flv.
 
-    if (preg_match("/Duration: (\d+):(\d+):(\d+\.\d+)/", $result, $matches)) {
-      $duration = 3600 * $matches[1] + 60 * $matches[2] + $matches[3];
+    if (preg_match("/Duration: (\d+:\d+:\d+\.\d+)/", $result, $matches)) {
+      $duration = movie::hhmmssdd_to_seconds($matches[1]);
     } else if (preg_match("/duration.*?:.*?(\d+)/", $result, $matches)) {
       $duration = $matches[1];
     } else {
@@ -160,4 +160,25 @@ class movie_Core {
     return array($width, $height, $mime_type, $extension, $duration);
   }
 
+  /**
+   * Return the time/duration formatted in hh:mm:ss.dd from a number of seconds.
+   * Useful for inputs to ffmpeg.
+   *
+   * Note that this is similar to date("H:i:s", mktime(0,0,$seconds,0,0,0,0)), but unlike this 
+   * approach avoids potential issues with time zone and DST mismatch and/or using deprecated
+   * features (the last argument of mkdate above, which disables DST, is deprecated as of PHP 5.3).
+   */
+  static function seconds_to_hhmmssdd($seconds) {
+    return sprintf("%02d:%02d:%05.2f", floor($seconds / 3600), floor(($seconds % 3600) / 60),
+                   floor(100 * $seconds % 6000) / 100);
+  }
+  
+  /**
+   * Return the number of seconds from a time/duration formatted in hh:mm:ss.dd.
+   * Useful for outputs from ffmpeg.
+   */
+  static function hhmmssdd_to_seconds($hhmmssdd) {
+    preg_match("/(\d+):(\d+):(\d+\.\d+)/", $hhmmssdd, $matches);
+    return 3600 * $matches[1] + 60 * $matches[2] + $matches[3];
+  }
 }
diff --git a/modules/gallery/tests/Movie_Helper_Test.php b/modules/gallery/tests/Movie_Helper_Test.php
new file mode 100644
index 00000000..78ac21a1
--- /dev/null
+++ b/modules/gallery/tests/Movie_Helper_Test.php
@@ -0,0 +1,49 @@
+<?php defined("SYSPATH") or die("No direct script access.");
+/**
+ * Gallery - a web based photo album viewer and editor
+ * Copyright (C) 2000-2012 Bharat Mediratta
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or (at
+ * your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street - Fifth Floor, Boston, MA  02110-1301, USA.
+ */
+class Movie_Helper_Test extends Gallery_Unit_Test_Case {
+  public function seconds_to_hhmmssdd_test() {
+    $times = array("00:00:00.50" => 0.5,
+                   "00:00:06.00" => 6,
+                   "00:00:59.99" => 59.999,
+                   "00:01:00.00" => 60.001,
+                   "00:07:00.00" => 7 * 60,
+                   "00:45:19.00" => 45 * 60 + 19,
+                   "03:45:19.00" => 3 * 3600 + 45 * 60 + 19,
+                   "126:45:19.00" => 126 * 3600 + 45 * 60 + 19);
+    foreach ($times as $hhmmssdd => $seconds) {
+      $this->assert_equal($hhmmssdd, movie::seconds_to_hhmmssdd($seconds));
+    }
+  }
+
+  public function hhmmssdd_to_seconds_test() {
+    $times = array("0:00:00.01" => 0.01,
+                   "00:00:00.50" => 0.5,
+                   "00:00:06.00" => 6,
+                   "00:00:59.99" => 59.99,
+                   "00:01:00.00" => 60.00,
+                   "00:07:00.00" => 7 * 60,
+                   "00:45:19.00" => 45 * 60 + 19,
+                   "03:45:19.00" => 3 * 3600 + 45 * 60 + 19,
+                   "126:45:19.00" => 126 * 3600 + 45 * 60 + 19);
+    foreach ($times as $hhmmssdd => $seconds) {
+      $this->assert_equal($seconds, movie::hhmmssdd_to_seconds($hhmmssdd));
+    }
+  }
+}