Title: [239587] trunk/Source/WebCore
Revision
239587
Author
[email protected]
Date
2019-01-02 22:42:52 -0800 (Wed, 02 Jan 2019)

Log Message

Leak of CMSampleBuffer (752 bytes) in com.apple.WebKit.WebContent running WebKit layout tests
<https://webkit.org/b/193016>
<rdar://problem/46925703>

Reviewed by Simon Fraser.

* platform/mediarecorder/cocoa/MediaRecorderPrivateWriterCocoa.mm:
(WebCore::copySampleBufferWithCurrentTimeStamp):
- Change to return RetainPtr<CMSampleBufferRef>.
- Check return value of CMSampleBufferCreateCopyWithNewTiming().
(WebCore::MediaRecorderPrivateWriter::appendVideoSampleBuffer):
- Check return value of copySampleBufferWithCurrentTimeStamp().
- Fix leak by using RetainPtr<CMSampleBufferRef> returned from
  copySampleBufferWithCurrentTimeStamp() instead of leaking
  `bufferWithCurrentTime` by using retainPtr().
(WebCore::createAudioFormatDescription):
- Extract method from appendAudioSampleBuffer() to return
  RetainPtr<CMFormatDescriptionRef> after calling
  CMAudioFormatDescriptionCreate().
- Check return value of CMAudioFormatDescriptionCreate().
(WebCore::createAudioSampleBufferWithPacketDescriptions):
- Extract method from appendAudioSampleBuffer() to return
  RetainPtr<CMSampleBufferRef> after calling
  CMAudioSampleBufferCreateWithPacketDescriptions().
(WebCore::MediaRecorderPrivateWriter::appendAudioSampleBuffer):
- Check return values of createAudioFormatDescription() and
  createAudioSampleBufferWithPacketDescriptions().
- Fix leaks by extracting code into helper methods that return
  RetainPtr<> objects instead of leaking CMFormatDescriptionRef
  directly or leaking `sampleBuffer` by using retainPtr().

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (239586 => 239587)


--- trunk/Source/WebCore/ChangeLog	2019-01-03 04:33:52 UTC (rev 239586)
+++ trunk/Source/WebCore/ChangeLog	2019-01-03 06:42:52 UTC (rev 239587)
@@ -1,3 +1,36 @@
+2019-01-02  David Kilzer  <[email protected]>
+
+        Leak of CMSampleBuffer (752 bytes) in com.apple.WebKit.WebContent running WebKit layout tests
+        <https://webkit.org/b/193016>
+        <rdar://problem/46925703>
+
+        Reviewed by Simon Fraser.
+
+        * platform/mediarecorder/cocoa/MediaRecorderPrivateWriterCocoa.mm:
+        (WebCore::copySampleBufferWithCurrentTimeStamp):
+        - Change to return RetainPtr<CMSampleBufferRef>.
+        - Check return value of CMSampleBufferCreateCopyWithNewTiming().
+        (WebCore::MediaRecorderPrivateWriter::appendVideoSampleBuffer):
+        - Check return value of copySampleBufferWithCurrentTimeStamp().
+        - Fix leak by using RetainPtr<CMSampleBufferRef> returned from
+          copySampleBufferWithCurrentTimeStamp() instead of leaking
+          `bufferWithCurrentTime` by using retainPtr().
+        (WebCore::createAudioFormatDescription):
+        - Extract method from appendAudioSampleBuffer() to return
+          RetainPtr<CMFormatDescriptionRef> after calling
+          CMAudioFormatDescriptionCreate().
+        - Check return value of CMAudioFormatDescriptionCreate().
+        (WebCore::createAudioSampleBufferWithPacketDescriptions):
+        - Extract method from appendAudioSampleBuffer() to return
+          RetainPtr<CMSampleBufferRef> after calling
+          CMAudioSampleBufferCreateWithPacketDescriptions().
+        (WebCore::MediaRecorderPrivateWriter::appendAudioSampleBuffer):
+        - Check return values of createAudioFormatDescription() and
+          createAudioSampleBufferWithPacketDescriptions().
+        - Fix leaks by extracting code into helper methods that return
+          RetainPtr<> objects instead of leaking CMFormatDescriptionRef
+          directly or leaking `sampleBuffer` by using retainPtr().
+
 2019-01-02  Wenson Hsieh  <[email protected]>
 
         Add support for using the current text selection as the find string on iOS

Modified: trunk/Source/WebCore/platform/mediarecorder/cocoa/MediaRecorderPrivateWriterCocoa.mm (239586 => 239587)


--- trunk/Source/WebCore/platform/mediarecorder/cocoa/MediaRecorderPrivateWriterCocoa.mm	2019-01-03 04:33:52 UTC (rev 239586)
+++ trunk/Source/WebCore/platform/mediarecorder/cocoa/MediaRecorderPrivateWriterCocoa.mm	2019-01-03 06:42:52 UTC (rev 239587)
@@ -196,7 +196,7 @@
     return true;
 }
 
-static inline CMSampleBufferRef copySampleBufferWithCurrentTimeStamp(CMSampleBufferRef originalBuffer)
+static inline RetainPtr<CMSampleBufferRef> copySampleBufferWithCurrentTimeStamp(CMSampleBufferRef originalBuffer)
 {
     CMTime startTime = CMClockGetTime(CMClockGetHostTimeClock());
     CMItemCount count = 0;
@@ -210,9 +210,11 @@
         timeInfo[i].presentationTimeStamp = startTime;
     }
     
-    CMSampleBufferRef newBuffer;
-    CMSampleBufferCreateCopyWithNewTiming(kCFAllocatorDefault, originalBuffer, count, timeInfo.data(), &newBuffer);
-    return newBuffer;
+    CMSampleBufferRef newBuffer = nullptr;
+    auto error = CMSampleBufferCreateCopyWithNewTiming(kCFAllocatorDefault, originalBuffer, count, timeInfo.data(), &newBuffer);
+    if (error)
+        return nullptr;
+    return adoptCF(newBuffer);
 }
 
 void MediaRecorderPrivateWriter::appendVideoSampleBuffer(CMSampleBufferRef sampleBuffer)
@@ -249,21 +251,42 @@
         }];
         return;
     }
-    CMSampleBufferRef bufferWithCurrentTime = copySampleBufferWithCurrentTimeStamp(sampleBuffer);
+    auto bufferWithCurrentTime = copySampleBufferWithCurrentTimeStamp(sampleBuffer);
+    if (!bufferWithCurrentTime)
+        return;
+
     auto locker = holdLock(m_videoLock);
-    m_videoBufferPool.append(retainPtr(bufferWithCurrentTime));
+    m_videoBufferPool.append(WTFMove(bufferWithCurrentTime));
 }
 
+static inline RetainPtr<CMFormatDescriptionRef> createAudioFormatDescription(const AudioStreamDescription& description)
+{
+    auto basicDescription = WTF::get<const AudioStreamBasicDescription*>(description.platformDescription().description);
+    CMFormatDescriptionRef format = nullptr;
+    auto error = CMAudioFormatDescriptionCreate(kCFAllocatorDefault, basicDescription, 0, NULL, 0, NULL, NULL, &format);
+    if (error)
+        return nullptr;
+    return adoptCF(format);
+}
+
+static inline RetainPtr<CMSampleBufferRef> createAudioSampleBufferWithPacketDescriptions(CMFormatDescriptionRef format, size_t sampleCount)
+{
+    CMTime startTime = CMClockGetTime(CMClockGetHostTimeClock());
+    CMSampleBufferRef sampleBuffer = nullptr;
+    auto error = CMAudioSampleBufferCreateWithPacketDescriptions(kCFAllocatorDefault, NULL, false, NULL, NULL, format, sampleCount, startTime, NULL, &sampleBuffer);
+    if (error)
+        return nullptr;
+    return adoptCF(sampleBuffer);
+}
+
 void MediaRecorderPrivateWriter::appendAudioSampleBuffer(const PlatformAudioData& data, const AudioStreamDescription& description, const WTF::MediaTime&, size_t sampleCount)
 {
     ASSERT(m_audioInput);
     if ((!m_hasStartedWriting && m_videoInput) || m_isStopped)
         return;
-    CMSampleBufferRef sampleBuffer;
-    CMFormatDescriptionRef format;
-    OSStatus error;
-    auto& basicDescription = *WTF::get<const AudioStreamBasicDescription*>(description.platformDescription().description);
-    error = CMAudioFormatDescriptionCreate(kCFAllocatorDefault, &basicDescription, 0, NULL, 0, NULL, NULL, &format);
+    auto format = createAudioFormatDescription(description);
+    if (!format)
+        return;
     if (m_isFirstAudioSample) {
         if (!m_videoInput) {
             // audio-only recording.
@@ -293,17 +316,16 @@
             }
         }];
     }
-    CMTime startTime = CMClockGetTime(CMClockGetHostTimeClock());
 
-    error = CMAudioSampleBufferCreateWithPacketDescriptions(kCFAllocatorDefault, NULL, false, NULL, NULL, format, sampleCount, startTime, NULL, &sampleBuffer);
-    if (error)
+    auto sampleBuffer = createAudioSampleBufferWithPacketDescriptions(format.get(), sampleCount);
+    if (!sampleBuffer)
         return;
-    error = CMSampleBufferSetDataBufferFromAudioBufferList(sampleBuffer, kCFAllocatorDefault, kCFAllocatorDefault, 0, downcast<WebAudioBufferList>(data).list());
+    auto error = CMSampleBufferSetDataBufferFromAudioBufferList(sampleBuffer.get(), kCFAllocatorDefault, kCFAllocatorDefault, 0, downcast<WebAudioBufferList>(data).list());
     if (error)
         return;
 
     auto locker = holdLock(m_audioLock);
-    m_audioBufferPool.append(retainPtr(sampleBuffer));
+    m_audioBufferPool.append(WTFMove(sampleBuffer));
 }
 
 void MediaRecorderPrivateWriter::stopRecording()
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to