[WTF] Allow MediaTime static constants
authoraboya@igalia.com <aboya@igalia.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 8 Jan 2020 15:57:03 +0000 (15:57 +0000)
committeraboya@igalia.com <aboya@igalia.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 8 Jan 2020 15:57:03 +0000 (15:57 +0000)
https://bugs.webkit.org/show_bug.cgi?id=205723

Reviewed by Darin Adler.

Source/WebCore:

Since MediaTime no longer has a non-trivial destructor, declaring
MediaTime variables no longer has potential side effects as far as the
compiler is concerned and it can therefore print "unused variable"
errors where that is the case.

This patch marks one of such variable as intentionally unused, since
it's only used in Debug and would otherwise break the Release build.
Actually unused variables are also removed.

* platform/graphics/cocoa/WebCoreDecompressionSession.mm:
(WebCore::WebCoreDecompressionSession::imageForTime):
* Modules/mediasource/SourceBuffer.cpp:
(WebCore::SourceBuffer::sourceBufferPrivateFastSeekTimeForMediaTime):
* Modules/mediasource/SourceBuffer.cpp:
(WebCore::SourceBuffer::removeCodedFrames):

Source/WTF:

Despite all its convenience methods, at its core MediaTime is a rather
trivial class with only integral members. Despite this, since it had a
destructor declared, this made the class non-trivially destructible
even if the implementation was empty, and therefore clang did not
allow to use it for static variables unless done in form of a pointer.

By removing the destructor this restriction is lifted and we don't
need heap allocations for static MediaTime objects.

Previous usages of heap allocation for static MediaTime objects have
been rewritten to take advantage of this. Test coverage is provided by
successful compilation without [-Werror,-Wexit-time-destructors]
errors and existing tests.

* wtf/MediaTime.cpp:
(WTF::MediaTime::zeroTime):
(WTF::MediaTime::invalidTime):
(WTF::MediaTime::positiveInfiniteTime):
(WTF::MediaTime::negativeInfiniteTime):
(WTF::MediaTime::indefiniteTime):
(WTF::MediaTime::~MediaTime): Deleted.
* wtf/MediaTime.h:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@254200 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WTF/ChangeLog
Source/WTF/wtf/MediaTime.cpp
Source/WTF/wtf/MediaTime.h
Source/WebCore/ChangeLog
Source/WebCore/Modules/mediasource/SourceBuffer.cpp
Source/WebCore/platform/graphics/cocoa/WebCoreDecompressionSession.mm

index e681bf7..d22b3b6 100644 (file)
@@ -1,3 +1,33 @@
+2020-01-08  Alicia Boya García  <aboya@igalia.com>
+
+        [WTF] Allow MediaTime static constants
+        https://bugs.webkit.org/show_bug.cgi?id=205723
+
+        Reviewed by Darin Adler.
+
+        Despite all its convenience methods, at its core MediaTime is a rather
+        trivial class with only integral members. Despite this, since it had a
+        destructor declared, this made the class non-trivially destructible
+        even if the implementation was empty, and therefore clang did not
+        allow to use it for static variables unless done in form of a pointer.
+
+        By removing the destructor this restriction is lifted and we don't
+        need heap allocations for static MediaTime objects.
+
+        Previous usages of heap allocation for static MediaTime objects have
+        been rewritten to take advantage of this. Test coverage is provided by
+        successful compilation without [-Werror,-Wexit-time-destructors]
+        errors and existing tests.
+
+        * wtf/MediaTime.cpp:
+        (WTF::MediaTime::zeroTime):
+        (WTF::MediaTime::invalidTime):
+        (WTF::MediaTime::positiveInfiniteTime):
+        (WTF::MediaTime::negativeInfiniteTime):
+        (WTF::MediaTime::indefiniteTime):
+        (WTF::MediaTime::~MediaTime): Deleted.
+        * wtf/MediaTime.h:
+
 2020-01-07  Said Abou-Hallawa  <sabouhallawa@apple.com>
 
         Implement css3-images image-orientation
index 6eddfb6..89d2ab7 100644 (file)
@@ -41,6 +41,8 @@
 
 namespace WTF {
 
+static_assert(std::is_trivially_destructible_v<MediaTime>, "MediaTime should be trivially destructible.");
+
 static uint32_t greatestCommonDivisor(uint32_t a, uint32_t b)
 {
     ASSERT(a);
@@ -88,10 +90,6 @@ MediaTime::MediaTime(int64_t value, uint32_t scale, uint8_t flags)
     *this = value < 0 ? negativeInfiniteTime() : positiveInfiniteTime();
 }
 
-MediaTime::~MediaTime()
-{
-}
-
 MediaTime::MediaTime(const MediaTime& rhs)
 {
     *this = rhs;
@@ -457,32 +455,32 @@ bool MediaTime::isBetween(const MediaTime& a, const MediaTime& b) const
 
 const MediaTime& MediaTime::zeroTime()
 {
-    static const MediaTime* time = new MediaTime(0, 1, Valid);
-    return *time;
+    static const MediaTime time(0, 1, Valid);
+    return time;
 }
 
 const MediaTime& MediaTime::invalidTime()
 {
-    static const MediaTime* time = new MediaTime(-1, 1, 0);
-    return *time;
+    static const MediaTime time(-1, 1, 0);
+    return time;
 }
 
 const MediaTime& MediaTime::positiveInfiniteTime()
 {
-    static const MediaTime* time = new MediaTime(0, 1, PositiveInfinite | Valid);
-    return *time;
+    static const MediaTime time(0, 1, PositiveInfinite | Valid);
+    return time;
 }
 
 const MediaTime& MediaTime::negativeInfiniteTime()
 {
-    static const MediaTime* time = new MediaTime(-1, 1, NegativeInfinite | Valid);
-    return *time;
+    static const MediaTime time(-1, 1, NegativeInfinite | Valid);
+    return time;
 }
 
 const MediaTime& MediaTime::indefiniteTime()
 {
-    static const MediaTime* time = new MediaTime(0, 1, Indefinite | Valid);
-    return *time;
+    static const MediaTime time(0, 1, Indefinite | Valid);
+    return time;
 }
 
 MediaTime MediaTime::toTimeScale(uint32_t timeScale, RoundingFlags flags) const
index 56f55b0..b26f5a8 100644 (file)
@@ -56,7 +56,6 @@ public:
     MediaTime();
     MediaTime(int64_t value, uint32_t scale, uint8_t flags = Valid);
     MediaTime(const MediaTime& rhs);
-    ~MediaTime();
 
     static MediaTime createWithFloat(float floatTime);
     static MediaTime createWithFloat(float floatTime, uint32_t timeScale);
index 359eee0..e52aad9 100644 (file)
@@ -1,3 +1,26 @@
+2020-01-08  Alicia Boya García  <aboya@igalia.com>
+
+        [WTF] Allow MediaTime static constants
+        https://bugs.webkit.org/show_bug.cgi?id=205723
+
+        Reviewed by Darin Adler.
+
+        Since MediaTime no longer has a non-trivial destructor, declaring
+        MediaTime variables no longer has potential side effects as far as the
+        compiler is concerned and it can therefore print "unused variable"
+        errors where that is the case.
+
+        This patch marks one of such variable as intentionally unused, since
+        it's only used in Debug and would otherwise break the Release build.
+        Actually unused variables are also removed.
+
+        * platform/graphics/cocoa/WebCoreDecompressionSession.mm:
+        (WebCore::WebCoreDecompressionSession::imageForTime):
+        * Modules/mediasource/SourceBuffer.cpp:
+        (WebCore::SourceBuffer::sourceBufferPrivateFastSeekTimeForMediaTime):
+        * Modules/mediasource/SourceBuffer.cpp:
+        (WebCore::SourceBuffer::removeCodedFrames):
+
 2020-01-08  Diego Pino Garcia  <dpino@igalia.com>
 
         REGRESSION(r253939): [GTK] Broke build with !USE(WPE_RENDERER) 
index c511c42..dd5397c 100644 (file)
@@ -495,8 +495,6 @@ void SourceBuffer::seekToTime(const MediaTime& time)
 MediaTime SourceBuffer::sourceBufferPrivateFastSeekTimeForMediaTime(const MediaTime& targetTime, const MediaTime& negativeThreshold, const MediaTime& positiveThreshold)
 {
     MediaTime seekTime = targetTime;
-    MediaTime lowerBoundTime = targetTime - negativeThreshold;
-    MediaTime upperBoundTime = targetTime + positiveThreshold;
 
     for (auto& trackBuffer : m_trackBufferMap.values()) {
         // Find the sample which contains the target time time.
@@ -810,7 +808,6 @@ void SourceBuffer::removeCodedFrames(const MediaTime& start, const MediaTime& en
     // https://dvcs.w3.org/hg/html-media/raw-file/tip/media-source/media-source.html#sourcebuffer-coded-frame-removal
 
     // 1. Let start be the starting presentation timestamp for the removal range.
-    MediaTime durationMediaTime = m_source->duration();
     MediaTime currentMediaTime = m_source->currentTime();
 
     // 2. Let end be the end presentation timestamp for the removal range.
index f378411..e5c9b64 100644 (file)
@@ -474,7 +474,9 @@ RetainPtr<CVPixelBufferRef> WebCoreDecompressionSession::imageForTime(const Medi
     bool allowLater = flags == WebCoreDecompressionSession::AllowLater;
 
     MediaTime startTime = PAL::toMediaTime(CMBufferQueueGetFirstPresentationTimeStamp(m_producerQueue.get()));
+#if !LOG_DISABLED
     MediaTime endTime = PAL::toMediaTime(CMBufferQueueGetEndPresentationTimeStamp(m_producerQueue.get()));
+#endif
     if (!allowLater && time < startTime) {
         LOG(Media, "WebCoreDecompressionSession::imageForTime(%p) - time(%s) too early for queue(%s -> %s)", this, toString(time).utf8().data(), toString(startTime).utf8().data(), toString(endTime).utf8().data());
         return nullptr;