QVideoSurfaceFormat API review changes.

Rename YuvColorSpace enum and related functions to YCbCrColorSpace.
Remove ViewportMode enum and make setFrameSize() always reset the
viewport to fill the frame.

Reviewed-by: Justin McPherson

3 files changed, 61 insertions, 102 deletions

diff --git a/src/multimedia/video/qvideosurfaceformat.cpp b/src/multimedia/video/qvideosurfaceformat.cpp
index e6ef8f3..c898e3a 100644
--- a/src/multimedia/video/qvideosurfaceformat.cpp
+++ b/src/multimedia/video/qvideosurfaceformat.cpp
@@ -57,7 +57,7 @@ public:
         , handleType(QAbstractVideoBuffer::NoHandle)
         , scanLineDirection(QVideoSurfaceFormat::TopToBottom)
         , pixelAspectRatio(1, 1)
-        , yuvColorSpace(QVideoSurfaceFormat::YCbCr_Undefined)
+        , ycbcrColorSpace(QVideoSurfaceFormat::YCbCr_Undefined)
         , frameRate(0.0)
     {
     }
@@ -71,7 +71,7 @@ public:
         , scanLineDirection(QVideoSurfaceFormat::TopToBottom)
         , frameSize(size)
         , pixelAspectRatio(1, 1)
-        , yuvColorSpace(QVideoSurfaceFormat::YCbCr_Undefined)
+        , ycbcrColorSpace(QVideoSurfaceFormat::YCbCr_Undefined)
         , viewport(QPoint(0, 0), size)
         , frameRate(0.0)
     {
@@ -84,7 +84,7 @@ public:
         , scanLineDirection(other.scanLineDirection)
         , frameSize(other.frameSize)
         , pixelAspectRatio(other.pixelAspectRatio)
-        , yuvColorSpace(other.yuvColorSpace)
+        , ycbcrColorSpace(other.ycbcrColorSpace)
         , viewport(other.viewport)
         , frameRate(other.frameRate)
         , propertyNames(other.propertyNames)
@@ -101,7 +101,7 @@ public:
             && pixelAspectRatio == other.pixelAspectRatio
             && viewport == other.viewport
             && frameRatesEqual(frameRate, other.frameRate)
-            && yuvColorSpace == other.yuvColorSpace
+            && ycbcrColorSpace == other.ycbcrColorSpace
             && propertyNames.count() == other.propertyNames.count()) {
             for (int i = 0; i < propertyNames.count(); ++i) {
                 int j = other.propertyNames.indexOf(propertyNames.at(i));
@@ -125,7 +125,7 @@ public:
     QVideoSurfaceFormat::Direction scanLineDirection;
     QSize frameSize;
     QSize pixelAspectRatio;
-    QVideoSurfaceFormat::YuvColorSpace yuvColorSpace;
+    QVideoSurfaceFormat::YCbCrColorSpace ycbcrColorSpace;
     QRect viewport;
     qreal frameRate;
     QList<QByteArray> propertyNames;
@@ -168,19 +168,10 @@ public:
     \value BottomToTop Scan lines are arranged from the bottom of the frame to the top.
 */
 
-/*!
-    \enum QVideoSurfaceFormat::ViewportMode
-
-    Enumerates the methods for updating the stream viewport when the frame size is changed.
-
-    \value ResetViewport The viewport is reset to cover an entire frame.
-    \value KeepViewport The viewport is kept within the bounds the frame.
-*/
-
 /*! 
-    \enum QVideoSurfaceFormat::YuvColorSpace
+    \enum QVideoSurfaceFormat::YCbCrColorSpace
  
-    Enumerates the YUV color space of video frames.
+    Enumerates the Y'CbCr color space of video frames.
  
     \value YCbCr_Undefined
     No color space is specified.
@@ -340,21 +331,13 @@ int QVideoSurfaceFormat::frameHeight() const
 /*!
     Sets the size of frames in a video stream to \a size.
 
-    The viewport \a mode indicates how the view port should be updated.
+    This will reset the viewport() to fill the entire frame.
 */
 
-void QVideoSurfaceFormat::setFrameSize(const QSize &size, ViewportMode mode)
+void QVideoSurfaceFormat::setFrameSize(const QSize &size)
 {
     d->frameSize = size;
-
-    switch (mode) {
-    case ResetViewport:
-        d->viewport = QRect(QPoint(0, 0), size);
-        break;
-    case KeepViewport:
-        d->viewport = QRect(QPoint(0, 0), size).intersected(d->viewport);
-        break;
-    }
+    d->viewport = QRect(QPoint(0, 0), size);
 }
 
 /*!
@@ -362,12 +345,13 @@ void QVideoSurfaceFormat::setFrameSize(const QSize &size, ViewportMode mode)
 
     Sets the \a width and \a height of frames in a video stream.
 
-    The viewport \a mode indicates how the view port should be updated.
+    This will reset the viewport() to fill the entire frame.
 */
 
-void QVideoSurfaceFormat::setFrameSize(int width, int height, ViewportMode mode)
+void QVideoSurfaceFormat::setFrameSize(int width, int height)
 {
-    setFrameSize(QSize(width, height), mode);
+    d->frameSize = QSize(width, height);
+    d->viewport = QRect(0, 0, width, height);
 }
 
 /*!
@@ -458,22 +442,22 @@ void QVideoSurfaceFormat::setPixelAspectRatio(int horizontal, int vertical)
 }
 
 /*!
-    Returns a YUV color space of a video stream.
+    Returns the Y'CbCr color space of a video stream.
 */
 
-QVideoSurfaceFormat::YuvColorSpace QVideoSurfaceFormat::yuvColorSpace() const
+QVideoSurfaceFormat::YCbCrColorSpace QVideoSurfaceFormat::yCbCrColorSpace() const
 {
-    return d->yuvColorSpace;
+    return d->ycbcrColorSpace;
 }
 
 /*!
-    Sets a YUV color \a space of a video stream.
+    Sets the Y'CbCr color \a space of a video stream.
     It is only used with raw YUV frame types.
 */
 
-void QVideoSurfaceFormat::setYuvColorSpace(QVideoSurfaceFormat::YuvColorSpace space)
+void QVideoSurfaceFormat::setYCbCrColorSpace(QVideoSurfaceFormat::YCbCrColorSpace space)
 {
-    d->yuvColorSpace = space;
+    d->ycbcrColorSpace = space;
 }
 
 /*!
@@ -508,7 +492,7 @@ QList<QByteArray> QVideoSurfaceFormat::propertyNames() const
             << "frameRate"
             << "pixelAspectRatio"
             << "sizeHint"
-            << "yuvColorSpace")
+            << "yCbCrColorSpace")
             + d->propertyNames;
 }
 
@@ -540,8 +524,8 @@ QVariant QVideoSurfaceFormat::property(const char *name) const
         return qVariantFromValue(d->pixelAspectRatio);
     } else if (qstrcmp(name, "sizeHint") == 0) {
         return sizeHint();
-    } else if (qstrcmp(name, "yuvColorSpace") == 0) {
-        return qVariantFromValue(d->yuvColorSpace);
+    } else if (qstrcmp(name, "yCbCrColorSpace") == 0) {
+        return qVariantFromValue(d->ycbcrColorSpace);
     } else {
         int id = 0;
         for (; id < d->propertyNames.count() && d->propertyNames.at(id) != name; ++id) {}
@@ -585,9 +569,9 @@ void QVideoSurfaceFormat::setProperty(const char *name, const QVariant &value)
             d->pixelAspectRatio = qvariant_cast<QSize>(value);
     } else if (qstrcmp(name, "sizeHint") == 0) {
         // read only.
-    } else if (qstrcmp(name, "yuvColorSpace") == 0) {
-          if (qVariantCanConvert<YuvColorSpace>(value))
-              d->yuvColorSpace = qvariant_cast<YuvColorSpace>(value);
+    } else if (qstrcmp(name, "yCbCrColorSpace") == 0) {
+          if (qVariantCanConvert<YCbCrColorSpace>(value))
+              d->ycbcrColorSpace = qvariant_cast<YCbCrColorSpace>(value);
     } else {
         int id = 0;
         for (; id < d->propertyNames.count() && d->propertyNames.at(id) != name; ++id) {}
diff --git a/src/multimedia/video/qvideosurfaceformat.h b/src/multimedia/video/qvideosurfaceformat.h
index 1f4a5cb..ee60244 100644
--- a/src/multimedia/video/qvideosurfaceformat.h
+++ b/src/multimedia/video/qvideosurfaceformat.h
@@ -68,13 +68,7 @@ public:
         BottomToTop
     };
 
-    enum ViewportMode
-    {
-        ResetViewport,
-        KeepViewport
-    };
-
-    enum YuvColorSpace
+    enum YCbCrColorSpace
     {
         YCbCr_Undefined,
         YCbCr_BT601,
@@ -106,8 +100,8 @@ public:
     QAbstractVideoBuffer::HandleType handleType() const;
 
     QSize frameSize() const;
-    void setFrameSize(const QSize &size, ViewportMode mode = ResetViewport);
-    void setFrameSize(int width, int height, ViewportMode mode = ResetViewport);
+    void setFrameSize(const QSize &size);
+    void setFrameSize(int width, int height);
 
     int frameWidth() const;
     int frameHeight() const;
@@ -125,8 +119,8 @@ public:
     void setPixelAspectRatio(const QSize &ratio);
     void setPixelAspectRatio(int width, int height);
 
-    YuvColorSpace yuvColorSpace() const;
-    void setYuvColorSpace(YuvColorSpace colorSpace);
+    YCbCrColorSpace yCbCrColorSpace() const;
+    void setYCbCrColorSpace(YCbCrColorSpace colorSpace);
 
     QSize sizeHint() const;
 
@@ -145,7 +139,7 @@ Q_MULTIMEDIA_EXPORT QDebug operator<<(QDebug, const QVideoSurfaceFormat &);
 QT_END_NAMESPACE
 
 Q_DECLARE_METATYPE(QVideoSurfaceFormat::Direction)
-Q_DECLARE_METATYPE(QVideoSurfaceFormat::YuvColorSpace)
+Q_DECLARE_METATYPE(QVideoSurfaceFormat::YCbCrColorSpace)
 
 QT_END_HEADER
 
diff --git a/tests/auto/qvideosurfaceformat/tst_qvideosurfaceformat.cpp b/tests/auto/qvideosurfaceformat/tst_qvideosurfaceformat.cpp
index 9623e80..a47cb48 100644
--- a/tests/auto/qvideosurfaceformat/tst_qvideosurfaceformat.cpp
+++ b/tests/auto/qvideosurfaceformat/tst_qvideosurfaceformat.cpp
@@ -68,8 +68,8 @@ private slots:
     void scanLineDirection();
     void frameRate_data();
     void frameRate();
-    void yuvColorSpace_data();
-    void yuvColorSpace();
+    void yCbCrColorSpace_data();
+    void yCbCrColorSpace();
     void pixelAspectRatio_data();
     void pixelAspectRatio();
     void sizeHint_data();
@@ -81,9 +81,6 @@ private slots:
     void assign();
 };
 
-Q_DECLARE_METATYPE(QVideoSurfaceFormat::ViewportMode)
-
-
 tst_QVideoSurfaceFormat::tst_QVideoSurfaceFormat()
 {
 }
@@ -122,7 +119,7 @@ void tst_QVideoSurfaceFormat::constructNull()
     QCOMPARE(format.scanLineDirection(), QVideoSurfaceFormat::TopToBottom);
     QCOMPARE(format.frameRate(), 0.0);
     QCOMPARE(format.pixelAspectRatio(), QSize(1, 1));
-    QCOMPARE(format.yuvColorSpace(), QVideoSurfaceFormat::YCbCr_Undefined);
+    QCOMPARE(format.yCbCrColorSpace(), QVideoSurfaceFormat::YCbCr_Undefined);
 }
 
 void tst_QVideoSurfaceFormat::construct_data()
@@ -161,7 +158,7 @@ void tst_QVideoSurfaceFormat::construct()
     QCOMPARE(format.scanLineDirection(), QVideoSurfaceFormat::TopToBottom);
     QCOMPARE(format.frameRate(), 0.0);
     QCOMPARE(format.pixelAspectRatio(), QSize(1, 1));
-    QCOMPARE(format.yuvColorSpace(), QVideoSurfaceFormat::YCbCr_Undefined);
+    QCOMPARE(format.yCbCrColorSpace(), QVideoSurfaceFormat::YCbCr_Undefined);
 }
 
 void tst_QVideoSurfaceFormat::frameSize_data()
@@ -202,45 +199,23 @@ void tst_QVideoSurfaceFormat::viewport_data()
     QTest::addColumn<QSize>("initialSize");
     QTest::addColumn<QRect>("viewport");
     QTest::addColumn<QSize>("newSize");
-    QTest::addColumn<QVideoSurfaceFormat::ViewportMode>("viewportMode");
     QTest::addColumn<QRect>("expectedViewport");
 
     QTest::newRow("grow reset")
             << QSize(64, 64)
             << QRect(8, 8, 48, 48)
             << QSize(1024, 1024)
-            << QVideoSurfaceFormat::ResetViewport
             << QRect(0, 0, 1024, 1024);
-    QTest::newRow("grow keep")
-            << QSize(64, 64)
-            << QRect(8, 8, 48, 48)
-            << QSize(1024, 1024)
-            << QVideoSurfaceFormat::KeepViewport
-            << QRect(8, 8, 48, 48);
     QTest::newRow("shrink reset")
             << QSize(1024, 1024)
             << QRect(8, 8, 1008, 1008)
             << QSize(64, 64)
-            << QVideoSurfaceFormat::ResetViewport
             << QRect(0, 0, 64, 64);
-    QTest::newRow("shrink keep")
-            << QSize(1024, 1024)
-            << QRect(8, 8, 1008, 1008)
-            << QSize(64, 64)
-            << QVideoSurfaceFormat::KeepViewport
-            << QRect(8, 8, 56, 56);
     QTest::newRow("unchanged reset")
             << QSize(512, 512)
             << QRect(8, 8, 496, 496)
             << QSize(512, 512)
-            << QVideoSurfaceFormat::ResetViewport
             << QRect(0, 0, 512, 512);
-    QTest::newRow("unchanged keep")
-            << QSize(512, 512)
-            << QRect(8, 8, 496, 496)
-            << QSize(512, 512)
-            << QVideoSurfaceFormat::KeepViewport
-            << QRect(8, 8, 496, 496);
 }
 
 void tst_QVideoSurfaceFormat::viewport()
@@ -248,7 +223,6 @@ void tst_QVideoSurfaceFormat::viewport()
     QFETCH(QSize, initialSize);
     QFETCH(QRect, viewport);
     QFETCH(QSize, newSize);
-    QFETCH(QVideoSurfaceFormat::ViewportMode, viewportMode);
     QFETCH(QRect, expectedViewport);
 
     {
@@ -261,7 +235,7 @@ void tst_QVideoSurfaceFormat::viewport()
         QCOMPARE(format.viewport(), viewport);
         QCOMPARE(format.property("viewport").toRect(), viewport);
 
-        format.setFrameSize(newSize, viewportMode);
+        format.setFrameSize(newSize);
 
         QCOMPARE(format.viewport(), expectedViewport);
         QCOMPARE(format.property("viewport").toRect(), expectedViewport);
@@ -350,9 +324,9 @@ void tst_QVideoSurfaceFormat::frameRate()
     }
 }
 
-void tst_QVideoSurfaceFormat::yuvColorSpace_data()
+void tst_QVideoSurfaceFormat::yCbCrColorSpace_data()
 {
-    QTest::addColumn<QVideoSurfaceFormat::YuvColorSpace>("colorspace");
+    QTest::addColumn<QVideoSurfaceFormat::YCbCrColorSpace>("colorspace");
 
     QTest::newRow("undefined")
             << QVideoSurfaceFormat::YCbCr_Undefined;
@@ -364,24 +338,24 @@ void tst_QVideoSurfaceFormat::yuvColorSpace_data()
             << QVideoSurfaceFormat::YCbCr_JPEG;
 }
 
-void tst_QVideoSurfaceFormat::yuvColorSpace()
+void tst_QVideoSurfaceFormat::yCbCrColorSpace()
 {
-    QFETCH(QVideoSurfaceFormat::YuvColorSpace, colorspace);
+    QFETCH(QVideoSurfaceFormat::YCbCrColorSpace, colorspace);
 
     {
         QVideoSurfaceFormat format(QSize(64, 64), QVideoFrame::Format_RGB32);
-        format.setYuvColorSpace(colorspace);
+        format.setYCbCrColorSpace(colorspace);
 
-        QCOMPARE(format.yuvColorSpace(), colorspace);
-        QCOMPARE(qvariant_cast<QVideoSurfaceFormat::YuvColorSpace>(format.property("yuvColorSpace")),
+        QCOMPARE(format.yCbCrColorSpace(), colorspace);
+        QCOMPARE(qvariant_cast<QVideoSurfaceFormat::YCbCrColorSpace>(format.property("yCbCrColorSpace")),
                 colorspace);
     }
     {
         QVideoSurfaceFormat format(QSize(64, 64), QVideoFrame::Format_RGB32);
-        format.setProperty("yuvColorSpace", qVariantFromValue(colorspace));
+        format.setProperty("yCbCrColorSpace", qVariantFromValue(colorspace));
 
-        QCOMPARE(format.yuvColorSpace(), colorspace);
-        QCOMPARE(qvariant_cast<QVideoSurfaceFormat::YuvColorSpace>(format.property("yuvColorSpace")),
+        QCOMPARE(format.yCbCrColorSpace(), colorspace);
+        QCOMPARE(qvariant_cast<QVideoSurfaceFormat::YCbCrColorSpace>(format.property("yCbCrColorSpace")),
                 colorspace);
     }
 }
@@ -483,7 +457,7 @@ void tst_QVideoSurfaceFormat::staticPropertyNames()
     QVERIFY(propertyNames.contains("scanLineDirection"));
     QVERIFY(propertyNames.contains("frameRate"));
     QVERIFY(propertyNames.contains("pixelAspectRatio"));
-    QVERIFY(propertyNames.contains("yuvColorSpace"));
+    QVERIFY(propertyNames.contains("yCbCrColorSpace"));
     QVERIFY(propertyNames.contains("sizeHint"));
     QCOMPARE(propertyNames.count(), 10);
 }
@@ -566,19 +540,26 @@ void tst_QVideoSurfaceFormat::compare()
     QCOMPARE(format1 == format4, false);
     QCOMPARE(format1 != format4, true);
 
-    format2.setFrameSize(1024, 768, QVideoSurfaceFormat::ResetViewport);
+    format2.setFrameSize(1024, 768);
 
     // Not equal, frame size differs.
     QCOMPARE(format1 == format2, false);
     QCOMPARE(format1 != format2, true);
 
-    format1.setFrameSize(1024, 768, QVideoSurfaceFormat::KeepViewport);
+    format1.setFrameSize(1024, 768);
+
+    // Equal.
+    QCOMPARE(format1 == format2, true);
+    QCOMPARE(format1 != format2, false);
+
+    format1.setViewport(QRect(0, 0, 800, 600));
+    format2.setViewport(QRect(112, 84, 800, 600));
 
-    // Not equal, viewport differs.
+    // Not equal, viewports differ.
     QCOMPARE(format1 == format2, false);
     QCOMPARE(format1 != format2, true);
 
-    format1.setViewport(QRect(0, 0, 1024, 768));
+    format1.setViewport(QRect(112, 84, 800, 600));
 
     // Equal.
     QCOMPARE(format1 == format2, true);
@@ -620,13 +601,13 @@ void tst_QVideoSurfaceFormat::compare()
     QCOMPARE(format1 == format2, true);
     QCOMPARE(format1 != format2, false);
 
-    format2.setYuvColorSpace(QVideoSurfaceFormat::YCbCr_xvYCC601);
+    format2.setYCbCrColorSpace(QVideoSurfaceFormat::YCbCr_xvYCC601);
 
     // Not equal yuv color space differs.
     QCOMPARE(format1 == format2, false);
     QCOMPARE(format1 != format2, true);
 
-    format1.setYuvColorSpace(QVideoSurfaceFormat::YCbCr_xvYCC601);
+    format1.setYCbCrColorSpace(QVideoSurfaceFormat::YCbCr_xvYCC601);
 
     // Equal.
     QCOMPARE(format1 == format2, true);