Fix time quantisation to circumvent the precision problem

required to re-arrange several functions in order
to use the new util::floordiv and to get all relevant
calculations into time.h

src/lib/time.cpp

@@ -21,6 +21,8 @@
 
 #include "lib/time.h"
 #include "lib/error.hpp"
+#include "lib/util.hpp"
+
 extern "C" {
 #include "lib/tmpbuf.h"
 }
@@ -28,6 +30,15 @@ extern "C" {
 #include <limits.h>
 #include <math.h>
 
+using util::floordiv;
+using lib::time::FSecs;
+using lib::time::FrameRate;
+using boost::rational_cast;
+
+namespace error = lumiera::error;
+
+
+
 
 /* GAVL_TIME_SCALE is the correct factor or dividend when using gavl_time_t for
  * units of whole seconds from gavl_time_t.  Since we want to use milliseconds,
@@ -63,43 +74,48 @@ lumiera_tmpbuf_print_time (gavl_time_t time)
   return buffer;
 }
 
-
-static double
-calculate_quantisation (gavl_time_t time, double grid, gavl_time_t origin)
+gavl_time_t
+lumiera_rational_to_time (FSecs const& fractionalSeconds)
 {
-  double val = time;    //////TODO this solution doesn't work due to precission loss!
-  val -= origin;
-  val /= grid;
-  return floor (val);   //////TODO need a hand coded floor-function for integers
+  return rational_cast<gavl_time_t> (GAVL_TIME_SCALE * fractionalSeconds);
 }
 
-static double
-clip_to_64bit (double val)
+
+gavl_time_t
+lumiera_frame_duration (FrameRate const& fps)
 {
-  if (val > LLONG_MAX)
-    val = LLONG_MAX;
-  else
-  if (val < LLONG_MIN)
-    val = LLONG_MIN;
+  if (!fps)
+    throw error::Logic ("Impossible to quantise to an zero spaced frame grid"
+                       , error::LUMIERA_ERROR_BOTTOM_VALUE);
   
-  return val;
+  FSecs duration = rational_cast<FSecs> (1/fps);
+  return lumiera_rational_to_time (duration);
 }
 
 
-int64_t
-lumiera_quantise_frames (gavl_time_t time, double grid, gavl_time_t origin)
+namespace { // implementation helper
+  
+  inline long
+  calculate_quantisation (gavl_time_t time, gavl_time_t origin, gavl_time_t grid)
+  {
+    time -= origin;
+    return floordiv (time,grid);
+  }
+}
+
+
+long
+lumiera_quantise_frames (gavl_time_t time, gavl_time_t grid, gavl_time_t origin)
 {
-  double gridNr = calculate_quantisation (time, grid, origin);
-  gridNr = clip_to_64bit (gridNr);
-  return (int64_t) gridNr;
+  return calculate_quantisation (time, origin, grid);
 }
 
 gavl_time_t
-lumiera_quantise_time (gavl_time_t time, double grid, gavl_time_t origin)
+lumiera_quantise_time (gavl_time_t time, gavl_time_t grid, gavl_time_t origin)
 {
-  double count = calculate_quantisation (time, grid, origin);
-  double alignedTime = clip_to_64bit (count * grid);
-  return (gavl_time_t) alignedTime;
+  int64_t count = calculate_quantisation (time, origin, grid);
+  gavl_time_t alignedTime = count * grid;
+  return alignedTime;
 }
 
 

src/lib/time.h

@@ -61,12 +61,21 @@
 /**
  * Converts a fraction of seconds to Lumiera's internal opaque time scale.
  * @param fractionalSeconds given as rational number
+ * @note inconsistent with Lumiera's general quantisation behaviour,
+ *       here negative fractional micro-ticks are truncated towards zero.
+ *       This was deemed irrelevant in practice.
  */
-inline gavl_time_t
-lumiera_rational_to_time (lib::time::FSecs const& fractionalSeconds)
-{
-  return boost::rational_cast<gavl_time_t> (GAVL_TIME_SCALE * fractionalSeconds);
-}
+gavl_time_t
+lumiera_rational_to_time (lib::time::FSecs const& fractionalSeconds);
+
+
+/**
+ * Calculates the duration of one frame in Lumiera time units.
+ * @param framerate underlying framerate as rational number
+ * @throw error::Logic on zero framerate
+ */
+gavl_time_t
+lumiera_frame_duration (lib::time::FrameRate const& fps);
 
 
 
@@ -92,7 +101,7 @@ lumiera_tmpbuf_print_time (gavl_time_t time);
  * @return number of the grid interval containing the given time.
  * @warning the resulting value is limited to (Time::Min, Time::MAX) 
  */
-int64_t
+long
 lumiera_quantise_frames (gavl_time_t time, double grid, gavl_time_t origin);
 
 /**
@@ -105,7 +114,7 @@ lumiera_quantise_frames (gavl_time_t time, double grid, gavl_time_t origin);
  *         clipped, because the result, relative to origin, needs to be <= Time::MAX 
  */
 gavl_time_t
-lumiera_quantise_time (gavl_time_t time, double grid, gavl_time_t origin);
+lumiera_quantise_time (gavl_time_t time, gavl_time_t origin, gavl_time_t grid);
 
 /**
  * Builds a time value by summing up the given components.

src/lib/time/lumitime.cpp

@@ -35,16 +35,6 @@ using std::string;
 namespace lib {
 namespace time {
   
-  namespace {
-    /** implementation detail: convert a rational number denoting fractionalSeconds
-     *  into the internal time scale used by GAVL and Lumiera internal time values.
-     */
-    inline gavl_time_t
-    rational2time (FSecs const& fractionalSeconds)
-    {
-      return boost::rational_cast<gavl_time_t> (GAVL_TIME_SCALE * fractionalSeconds);
-    }
-  }
   
   
   /** @note the allowed time range is explicitly limited to help overflow protection */
@@ -76,7 +66,7 @@ namespace time {
    *  An example would be to the time unit of a framerate.
    */
   Time::Time (FSecs const& fractionalSeconds)
-    : TimeValue(rational2time (fractionalSeconds))
+    : TimeValue(lumiera_rational_to_time (fractionalSeconds))
     { }
   
   
@@ -108,6 +98,38 @@ namespace time {
   
   
   
+  /** predefined constant for PAL framerate */
+  const FrameRate FrameRate::PAL  (25);
+  const FrameRate FrameRate::NTSC (3000,1001);
+  
+  
+  /** @return time span of one frame of this rate,
+   *   cast into internal Lumiera time scale */
+  Duration
+  FrameRate::duration() const
+  {
+    if (0 == *this)
+      throw error::Logic ("Impossible to quantise to an zero spaced frame grid"
+                         , error::LUMIERA_ERROR_BOTTOM_VALUE);
+    
+    return Duration (1, *this);
+  }
+  
+  
+  /** duration of the given number of frames */
+  Duration::Duration (ulong count, FrameRate const& fps)
+    : Offset(TimeValue (count? lumiera_frame_duration (fps/count) : _raw(Duration::NIL)))
+    { }
+  
+  
+  /** constant to indicate "no duration" */
+  const Duration Duration::NIL = Offset(TimeValue(0));
+  
+  
+  
+  
+  
+  
 }} // namespace lib::Time
 
 ///////////////////////////////////////////////////////////////////////////TODO leftover of the existing/initial lumitime-Implementation

src/lib/time/quantiser.cpp

@@ -74,9 +74,9 @@ namespace time {
    *  as origin of the scale. Quantisation then means to determine the grid interval containing
    *  a given raw time value. Here, the grid interval number zero \em starts at the origin;
    *  each interval includes its lower bound and excludes its upper bound.*/
-  FixedFrameQuantiser::FixedFrameQuantiser (FSecs frames_per_second, TimeValue referencePoint)
+  FixedFrameQuantiser::FixedFrameQuantiser (FrameRate const& frames_per_second, TimeValue referencePoint)
     : origin_(referencePoint)
-    , raster_(rational_cast<double> (GAVL_TIME_SCALE / __ensure_notZero(frames_per_second)))
+    , raster_(frames_per_second.duration())
     { }
   
   
@@ -96,7 +96,7 @@ namespace time {
   TimeValue
   FixedFrameQuantiser::gridAlign (TimeValue const& rawTime)
   {
-    return TimeValue (lumiera_quantise_time (_raw(rawTime), raster_, _raw(origin_)));
+    return TimeValue (lumiera_quantise_time (_raw(rawTime), _raw(origin_), _raw(raster_)));
   }
   
   

src/lib/time/quantiser.hpp

@@ -102,11 +102,11 @@ namespace time {
   class FixedFrameQuantiser
     : public Quantiser
     {
-      Time   origin_;
-      double raster_;
+      Time     origin_;
+      Duration raster_;
       
     public:
-      FixedFrameQuantiser (FSecs frames_per_second, TimeValue referencePoint  =TimeValue(0));
+      FixedFrameQuantiser (FrameRate const& frames_per_second, TimeValue referencePoint  =TimeValue(0));
       
       
       TimeValue gridAlign (TimeValue const&);

src/lib/time/timevalue.hpp

@@ -24,6 +24,8 @@
 #ifndef LIB_TIME_TIMEVALUE_H
 #define LIB_TIME_TIMEVALUE_H
 
+#include "lib/error.hpp"
+
 #include <boost/operators.hpp>
 #include <boost/rational.hpp>
 #include <cstdlib>
@@ -38,6 +40,8 @@ extern "C" {
 namespace lib {
 namespace time {
   
+  namespace error = lumiera::error;
+  
   
   /**
    * basic constant internal time value.
@@ -199,7 +203,9 @@ namespace time {
   /** rational representation of fractional seconds
    * @warning do not mix up gavl_time_t and FSecs */
   typedef boost::rational<long> FSecs;
-
+  
+  class FrameRate;
+  
   /**
    * Lumiera's internal time value datatype.
    * This is a TimeValue, but now more specifically denoting
@@ -270,6 +276,10 @@ namespace time {
       Duration (Offset const& distance)
         : Offset(distance.abs())
         { }
+      
+      Duration (ulong count, FrameRate const& fps);
+      
+      static const Duration NIL;
     };
   
   
@@ -311,9 +321,48 @@ namespace time {
     };
   
   
+  /**
+   * Framerate specified as frames per second.
+   * Implemented as rational number.
+   */
+  class FrameRate
+    : public boost::rational<uint>
+    {
+      typedef boost::rational<uint> IFrac;
+      
+    public:
+      FrameRate (uint fps) ;
+      FrameRate (uint num, uint denom);
+      FrameRate (IFrac const& fractionalRate);
+      
+      // standard copy acceptable;
+      
+      static const FrameRate PAL;
+      static const FrameRate NTSC;
+      
+      /** duration of one frame */
+      Duration duration() const;
+    };
+  
+  
   
   
   /* == implementations == */
+    
+  namespace { // implementation helpers...
+    
+    template<typename NUM>
+    inline NUM
+    __ensure_nonzero (NUM n)
+    {
+      if (!n)
+        throw error::Logic ("Zero spaced grid not allowed"
+                           , error::LUMIERA_ERROR_BOTTOM_VALUE);
+      return n;
+    }
+  }//(End) implementation helpers
+  
+  
   
   /** @internal applies a limiter on the provided
    * raw time value to keep it within the arbitrary
@@ -330,6 +379,33 @@ namespace time {
          :                  raw;
   }
   
+  inline
+  FrameRate::FrameRate (uint fps)
+    : IFrac (__ensure_nonzero(fps))
+    { }
+  
+  inline
+  FrameRate::FrameRate (uint num, uint denom)
+    : IFrac (__ensure_nonzero(num), denom)
+    { }
+  
+  inline
+  FrameRate::FrameRate (IFrac const& fractionalRate)
+    : IFrac (__ensure_nonzero(fractionalRate))
+    { }
+  
+  
   
 }} // lib::time
+
+
+namespace util {
+    
+  inline bool
+  isnil (lib::time::Duration const& dur)
+  {
+    return 0 == dur;
+  }
+
+}
 #endif

src/lib/util.hpp

@@ -70,10 +70,10 @@ namespace util {
     if (0 < (num^den))
       return num/den;
     else
-      {
+      { // truncate similar to floor()
         ldiv_t res = ldiv(num,den);
-        return (res.rem)? res.quot-1 
-                        : res.quot;
+        return (res.rem)? res.quot-1   // negative results truncated towards next smaller int
+                        : res.quot;   //..unless the division result not truncated at all
       }
   }
   

tests/lib/time/quantiser-basics-test.cpp

@@ -101,7 +101,7 @@ namespace test{
         : FixedFrameQuantiser
         {
           TestQuant (int origin=0)
-            : FixedFrameQuantiser(FSecs(GAVL_TIME_SCALE,3), TimeValue(origin))
+            : FixedFrameQuantiser( FrameRate(3,GAVL_TIME_SCALE), TimeValue(origin))
             { }
           
           int