Change the frame rate property to a qreal from a QPair<int,int> rational

While a rational number is a common way to represent a frame rate,
QPair<int, int> isn't a proper numeric type meaning it can't be used as
anything more than an identifer for an exact frame rate without being
converted to a real, or extending it to a proper rational type.

Rev by: Justin McPherson

3 files changed, 30 insertions, 68 deletions

diff --git a/src/multimedia/video/qvideosurfaceformat.cpp b/src/multimedia/video/qvideosurfaceformat.cpp
index 2b0de96..e6ef8f3 100644
--- a/src/multimedia/video/qvideosurfaceformat.cpp
+++ b/src/multimedia/video/qvideosurfaceformat.cpp
@@ -58,7 +58,7 @@ public:
         , scanLineDirection(QVideoSurfaceFormat::TopToBottom)
         , pixelAspectRatio(1, 1)
         , yuvColorSpace(QVideoSurfaceFormat::YCbCr_Undefined)
-        , frameRate(0, 0)
+        , frameRate(0.0)
     {
     }
 
@@ -73,7 +73,7 @@ public:
         , pixelAspectRatio(1, 1)
         , yuvColorSpace(QVideoSurfaceFormat::YCbCr_Undefined)
         , viewport(QPoint(0, 0), size)
-        , frameRate(0, 0)
+        , frameRate(0.0)
     {
     }
 
@@ -100,7 +100,7 @@ public:
             && frameSize == other.frameSize
             && pixelAspectRatio == other.pixelAspectRatio
             && viewport == other.viewport
-            && frameRate == other.frameRate
+            && frameRatesEqual(frameRate, other.frameRate)
             && yuvColorSpace == other.yuvColorSpace
             && propertyNames.count() == other.propertyNames.count()) {
             for (int i = 0; i < propertyNames.count(); ++i) {
@@ -115,6 +115,11 @@ public:
         }
     }
 
+    inline static bool frameRatesEqual(qreal r1, qreal r2)
+    {
+        return qAbs(r1 - r2) <= 0.00001 * qMin(qAbs(r1), qAbs(r2));
+    }
+
     QVideoFrame::PixelFormat pixelFormat;
     QAbstractVideoBuffer::HandleType handleType;
     QVideoSurfaceFormat::Direction scanLineDirection;
@@ -122,7 +127,7 @@ public:
     QSize pixelAspectRatio;
     QVideoSurfaceFormat::YuvColorSpace yuvColorSpace;
     QRect viewport;
-    QVideoSurfaceFormat::FrameRate frameRate;
+    qreal frameRate;
     QList<QByteArray> propertyNames;
     QList<QVariant> propertyValues;
 };
@@ -201,15 +206,6 @@ public:
     The full range Y'CbCr color space used in JPEG files.
 */
 
-
-/*!
-    \typedef QVideoSurfaceFormat::FrameRate
-
-    A pair of integers representing the frame rate of a video stream.
-
-    The first number is the numerator and the second the denominator.
-*/
-
 /*!
     Constructs a null video stream format.
 */
@@ -415,41 +411,24 @@ void QVideoSurfaceFormat::setScanLineDirection(Direction direction)
 }
 
 /*!
-    Returns the frame rate of a video stream.
-
-    The frame rate is a rational number represented by a pair of integers.
-    The first integer is the numerator and the second the denominator.
+    Returns the frame rate of a video stream in frames per second.
 */
 
-QVideoSurfaceFormat::FrameRate QVideoSurfaceFormat::frameRate() const
+qreal QVideoSurfaceFormat::frameRate() const
 {
     return d->frameRate;
 }
 
 /*!
-    Sets the frame \a rate of a video stream.
-
-    The frame rate is a rational number represented by a pair of integers.
-    The first integer is the numerator and the second the denominator.
+    Sets the frame \a rate of a video stream in frames per second.
 */
 
-void QVideoSurfaceFormat::setFrameRate(const FrameRate &rate)
+void QVideoSurfaceFormat::setFrameRate(qreal rate)
 {
     d->frameRate = rate;
 }
 
 /*!
-    \overload
-
-    Sets the \a numerator and \a denominator of the frame rate of a video stream.
-*/
-
-void QVideoSurfaceFormat::setFrameRate(int numerator, int denominator)
-{
-    d->frameRate = qMakePair(numerator, denominator);
-}
-
-/*!
     Returns a video stream's pixel aspect ratio.
 */
 
@@ -599,8 +578,8 @@ void QVideoSurfaceFormat::setProperty(const char *name, const QVariant &value)
         if (qVariantCanConvert<Direction>(value))
             d->scanLineDirection = qvariant_cast<Direction>(value);
     } else if (qstrcmp(name, "frameRate") == 0) {
-        if (qVariantCanConvert<FrameRate>(value))
-            d->frameRate = qvariant_cast<FrameRate>(value);
+        if (qVariantCanConvert<qreal>(value))
+            d->frameRate = qvariant_cast<qreal>(value);
     } else if (qstrcmp(name, "pixelAspectRatio") == 0) {
         if (qVariantCanConvert<QSize>(value))
             d->pixelAspectRatio = qvariant_cast<QSize>(value);
diff --git a/src/multimedia/video/qvideosurfaceformat.h b/src/multimedia/video/qvideosurfaceformat.h
index b3005bd..1f4a5cb 100644
--- a/src/multimedia/video/qvideosurfaceformat.h
+++ b/src/multimedia/video/qvideosurfaceformat.h
@@ -87,8 +87,6 @@ public:
 #endif
     };
 
-    typedef QPair<int, int> FrameRate;
-
     QVideoSurfaceFormat();
     QVideoSurfaceFormat(
             const QSize &size,
@@ -120,9 +118,8 @@ public:
     Direction scanLineDirection() const;
     void setScanLineDirection(Direction direction);
 
-    FrameRate frameRate() const;
-    void setFrameRate(const FrameRate &rate);
-    void setFrameRate(int numerator, int denominator = 1);
+    qreal frameRate() const;
+    void setFrameRate(qreal rate);
 
     QSize pixelAspectRatio() const;
     void setPixelAspectRatio(const QSize &ratio);
@@ -147,7 +144,6 @@ Q_MULTIMEDIA_EXPORT QDebug operator<<(QDebug, const QVideoSurfaceFormat &);
 
 QT_END_NAMESPACE
 
-Q_DECLARE_METATYPE(QVideoSurfaceFormat::FrameRate)
 Q_DECLARE_METATYPE(QVideoSurfaceFormat::Direction)
 Q_DECLARE_METATYPE(QVideoSurfaceFormat::YuvColorSpace)
 
diff --git a/tests/auto/qvideosurfaceformat/tst_qvideosurfaceformat.cpp b/tests/auto/qvideosurfaceformat/tst_qvideosurfaceformat.cpp
index bc6fe68..9623e80 100644
--- a/tests/auto/qvideosurfaceformat/tst_qvideosurfaceformat.cpp
+++ b/tests/auto/qvideosurfaceformat/tst_qvideosurfaceformat.cpp
@@ -120,7 +120,7 @@ void tst_QVideoSurfaceFormat::constructNull()
     QCOMPARE(format.frameHeight(), -1);
     QCOMPARE(format.viewport(), QRect());
     QCOMPARE(format.scanLineDirection(), QVideoSurfaceFormat::TopToBottom);
-    QCOMPARE(format.frameRate(), QVideoSurfaceFormat::FrameRate());
+    QCOMPARE(format.frameRate(), 0.0);
     QCOMPARE(format.pixelAspectRatio(), QSize(1, 1));
     QCOMPARE(format.yuvColorSpace(), QVideoSurfaceFormat::YCbCr_Undefined);
 }
@@ -159,7 +159,7 @@ void tst_QVideoSurfaceFormat::construct()
     QCOMPARE(format.frameHeight(), frameSize.height());
     QCOMPARE(format.viewport(), viewport);
     QCOMPARE(format.scanLineDirection(), QVideoSurfaceFormat::TopToBottom);
-    QCOMPARE(format.frameRate(), QVideoSurfaceFormat::FrameRate());
+    QCOMPARE(format.frameRate(), 0.0);
     QCOMPARE(format.pixelAspectRatio(), QSize(1, 1));
     QCOMPARE(format.yuvColorSpace(), QVideoSurfaceFormat::YCbCr_Undefined);
 }
@@ -315,21 +315,21 @@ void tst_QVideoSurfaceFormat::scanLineDirection()
 
 void tst_QVideoSurfaceFormat::frameRate_data()
 {
-    QTest::addColumn<QVideoSurfaceFormat::FrameRate>("frameRate");
+    QTest::addColumn<qreal>("frameRate");
 
     QTest::newRow("null")
-            << QVideoSurfaceFormat::FrameRate(0, 0);
+            << 0.0;
     QTest::newRow("1/1")
-            << QVideoSurfaceFormat::FrameRate(1, 1);
+            << 1.0;
     QTest::newRow("24/1")
-            << QVideoSurfaceFormat::FrameRate(24, 1);
+            << 24.0;
     QTest::newRow("15/2")
-            << QVideoSurfaceFormat::FrameRate(15, 2);
+            << 7.5;
 }
 
 void tst_QVideoSurfaceFormat::frameRate()
 {
-    QFETCH(QVideoSurfaceFormat::FrameRate, frameRate);
+    QFETCH(qreal, frameRate);
 
     {
         QVideoSurfaceFormat format(QSize(64, 64), QVideoFrame::Format_RGB32);
@@ -337,29 +337,16 @@ void tst_QVideoSurfaceFormat::frameRate()
         format.setFrameRate(frameRate);
 
         QCOMPARE(format.frameRate(), frameRate);
-        QCOMPARE(qvariant_cast<QVideoSurfaceFormat::FrameRate>(format.property("frameRate")),
-                frameRate);
-    }
-    {
-        QVideoSurfaceFormat format(QSize(64, 64), QVideoFrame::Format_RGB32);
-
-        format.setFrameRate(frameRate.first, frameRate.second);
-
-        QCOMPARE(format.frameRate(), frameRate);
-        QCOMPARE(
-                qvariant_cast<QVideoSurfaceFormat::FrameRate>(format.property("frameRate")),
-                frameRate);
+        QCOMPARE(qvariant_cast<qreal>(format.property("frameRate")), frameRate);
     }
     {
         QVideoSurfaceFormat format(QSize(64, 64), QVideoFrame::Format_RGB32);
 
         format.setFrameRate(frameRate);
-        format.setProperty(
-                "frameRate", qVariantFromValue<QVideoSurfaceFormat::FrameRate>(frameRate));
+        format.setProperty("frameRate", frameRate);
 
         QCOMPARE(format.frameRate(), frameRate);
-        QCOMPARE(qvariant_cast<QVideoSurfaceFormat::FrameRate>(format.property("frameRate")),
-                frameRate);
+        QCOMPARE(qvariant_cast<qreal>(format.property("frameRate")), frameRate);
     }
 }
 
@@ -609,13 +596,13 @@ void tst_QVideoSurfaceFormat::compare()
     QCOMPARE(format1 == format2, true);
     QCOMPARE(format1 != format2, false);
 
-    format1.setFrameRate(QVideoSurfaceFormat::FrameRate(15, 2));
+    format1.setFrameRate(7.5);
 
     // Not equal frame rate differs.
     QCOMPARE(format1 == format2, false);
     QCOMPARE(format1 != format2, true);
 
-    format2.setFrameRate(15, 2);
+    format2.setFrameRate(7.50001);
 
     // Equal.
     QCOMPARE(format1 == format2, true);