Diff
Modified: trunk/LayoutTests/ChangeLog (239242 => 239243)
--- trunk/LayoutTests/ChangeLog 2018-12-15 00:55:23 UTC (rev 239242)
+++ trunk/LayoutTests/ChangeLog 2018-12-15 01:07:18 UTC (rev 239243)
@@ -1,3 +1,13 @@
+2018-12-14 Youenn Fablet <[email protected]>
+
+ MediaRecorderPrivateAVFImpl should have a Ref<MediaRecorderPrivateWriter> as member
+ https://bugs.webkit.org/show_bug.cgi?id=192720
+
+ Reviewed by Eric Carlson.
+
+ * http/wpt/mediarecorder/MediaRecorder-onremovetrack-expected.txt: Added.
+ * http/wpt/mediarecorder/MediaRecorder-onremovetrack.html: Added.
+
2018-12-14 Matt Baker <[email protected]>
Web Inspector: Cookies view should use model objects instead of raw payload data
Added: trunk/LayoutTests/http/wpt/mediarecorder/MediaRecorder-onremovetrack-expected.txt (0 => 239243)
--- trunk/LayoutTests/http/wpt/mediarecorder/MediaRecorder-onremovetrack-expected.txt (rev 0)
+++ trunk/LayoutTests/http/wpt/mediarecorder/MediaRecorder-onremovetrack-expected.txt 2018-12-15 01:07:18 UTC (rev 239243)
@@ -0,0 +1,3 @@
+
+PASS MediaRecorder should stop when track is removed
+
Added: trunk/LayoutTests/http/wpt/mediarecorder/MediaRecorder-onremovetrack.html (0 => 239243)
--- trunk/LayoutTests/http/wpt/mediarecorder/MediaRecorder-onremovetrack.html (rev 0)
+++ trunk/LayoutTests/http/wpt/mediarecorder/MediaRecorder-onremovetrack.html 2018-12-15 01:07:18 UTC (rev 239243)
@@ -0,0 +1,52 @@
+<!doctype html>
+<html>
+<head>
+ <title>MediaRecorder should stop when track is removed</title
+ <link rel="help" href=""
+ <script src=""
+ <script src=""
+</head>
+<body>
+<script src=""
+<canvas id="canvas" width="200" height="200">
+</canvas>
+<script>
+const ac = new webkitAudioContext();
+const osc = ac.createOscillator();
+const dest = ac.createMediaStreamDestination();
+const audio = dest.stream;
+osc.connect(dest);
+
+function finishTest()
+{
+ gc();
+ setTimeout(() => {
+ done();
+ ac.close();
+ }, 100);
+}
+
+function removeTrack()
+{
+ audio.removeTrack(audio.getAudioTracks()[0]);
+ setTimeout(finishTest, 100);
+}
+
+function doTest()
+{
+ const recorder = new MediaRecorder(audio);
+
+ recorder._onerror_ = () => {
+ assert_equals(recorder.state, 'inactive', 'MediaRecorder is inactive');
+ };
+ recorder.start();
+ osc.start();
+ assert_equals(recorder.state, 'recording', 'MediaRecorder has been started successfully');
+ setTimeout(removeTrack, 100);
+}
+
+doTest();
+
+</script>
+</body>
+</html>
Modified: trunk/Source/WebCore/ChangeLog (239242 => 239243)
--- trunk/Source/WebCore/ChangeLog 2018-12-15 00:55:23 UTC (rev 239242)
+++ trunk/Source/WebCore/ChangeLog 2018-12-15 01:07:18 UTC (rev 239243)
@@ -1,5 +1,41 @@
2018-12-14 Youenn Fablet <[email protected]>
+ MediaRecorderPrivateAVFImpl should have a Ref<MediaRecorderPrivateWriter> as member
+ https://bugs.webkit.org/show_bug.cgi?id=192720
+
+ Reviewed by Eric Carlson.
+
+ Make sure that MediaRecorderPrivateAVFImpl takes a Ref<MediaRecorderPrivateWriter> as member,
+ as the latter is a ref counted object.
+ Made some refactoring to return early in case of error.
+
+ Also made sure that in the case of a MediaRecorder stopped by a track removal in the recorded stream
+ the MediaRecorder will stop listening for its tracks.
+ Otherwise, the tracks will continue calling the MediaRecorder even after it is dead.
+
+ Test: http/wpt/mediarecorder/MediaRecorder-onremovetrack.html
+
+ * Modules/mediarecorder/MediaRecorder.cpp:
+ (WebCore::MediaRecorder::didAddOrRemoveTrack):
+ (WebCore::MediaRecorder::setNewRecordingState): Deleted.
+ * Modules/mediarecorder/MediaRecorder.h:
+ * platform/mediarecorder/MediaRecorderPrivateAVFImpl.cpp:
+ (WebCore::MediaRecorderPrivateAVFImpl::create):
+ (WebCore::MediaRecorderPrivateAVFImpl::MediaRecorderPrivateAVFImpl):
+ (WebCore::MediaRecorderPrivateAVFImpl::sampleBufferUpdated):
+ (WebCore::MediaRecorderPrivateAVFImpl::audioSamplesAvailable):
+ (WebCore::MediaRecorderPrivateAVFImpl::stopRecording):
+ (WebCore::MediaRecorderPrivateAVFImpl::fetchData):
+ * platform/mediarecorder/MediaRecorderPrivateAVFImpl.h:
+ * platform/mediarecorder/cocoa/MediaRecorderPrivateWriterCocoa.h:
+ * platform/mediarecorder/cocoa/MediaRecorderPrivateWriterCocoa.mm:
+ (WebCore::MediaRecorderPrivateWriter::create):
+ (WebCore::MediaRecorderPrivateWriter::MediaRecorderPrivateWriter):
+ (WebCore::MediaRecorderPrivateWriter::appendAudioSampleBuffer):
+ (WebCore::MediaRecorderPrivateWriter::setupWriter): Deleted.
+
+2018-12-14 Youenn Fablet <[email protected]>
+
getSenders/getReceivers() should not return closed transceiver senders/receivers
https://bugs.webkit.org/show_bug.cgi?id=192706
Modified: trunk/Source/WebCore/Modules/mediarecorder/MediaRecorder.cpp (239242 => 239243)
--- trunk/Source/WebCore/Modules/mediarecorder/MediaRecorder.cpp 2018-12-15 00:55:23 UTC (rev 239242)
+++ trunk/Source/WebCore/Modules/mediarecorder/MediaRecorder.cpp 2018-12-15 01:07:18 UTC (rev 239243)
@@ -161,8 +161,9 @@
scheduleDeferredTask([this] {
if (!m_isActive || state() == RecordingState::Inactive)
return;
+ stopRecordingInternal();
auto event = MediaRecorderErrorEvent::create(eventNames().errorEvent, Exception { UnknownError, "Track cannot be added to or removed from the MediaStream while recording is happening"_s });
- setNewRecordingState(RecordingState::Inactive, WTFMove(event));
+ dispatchEvent(WTFMove(event));
});
}
@@ -187,12 +188,6 @@
m_private->audioSamplesAvailable(track, mediaTime, audioData, description, sampleCount);
}
-void MediaRecorder::setNewRecordingState(RecordingState newState, Ref<Event>&& event)
-{
- m_state = newState;
- dispatchEvent(WTFMove(event));
-}
-
void MediaRecorder::scheduleDeferredTask(Function<void()>&& function)
{
ASSERT(function);
Modified: trunk/Source/WebCore/Modules/mediarecorder/MediaRecorder.h (239242 => 239243)
--- trunk/Source/WebCore/Modules/mediarecorder/MediaRecorder.h 2018-12-15 00:55:23 UTC (rev 239242)
+++ trunk/Source/WebCore/Modules/mediarecorder/MediaRecorder.h 2018-12-15 01:07:18 UTC (rev 239243)
@@ -103,7 +103,6 @@
void audioSamplesAvailable(MediaStreamTrackPrivate&, const MediaTime&, const PlatformAudioData&, const AudioStreamDescription&, size_t) final;
void scheduleDeferredTask(Function<void()>&&);
- void setNewRecordingState(RecordingState, Ref<Event>&&);
static creatorFunction m_customCreator;
Modified: trunk/Source/WebCore/platform/mediarecorder/MediaRecorderPrivateAVFImpl.cpp (239242 => 239243)
--- trunk/Source/WebCore/platform/mediarecorder/MediaRecorderPrivateAVFImpl.cpp 2018-12-15 00:55:23 UTC (rev 239242)
+++ trunk/Source/WebCore/platform/mediarecorder/MediaRecorderPrivateAVFImpl.cpp 2018-12-15 01:07:18 UTC (rev 239243)
@@ -38,58 +38,55 @@
std::unique_ptr<MediaRecorderPrivateAVFImpl> MediaRecorderPrivateAVFImpl::create(const MediaStreamPrivate& stream)
{
- auto instance = std::unique_ptr<MediaRecorderPrivateAVFImpl>(new MediaRecorderPrivateAVFImpl(stream));
- if (!instance->m_isWriterReady)
- return nullptr;
- return instance;
-}
+ // FIXME: we will need to implement support for multiple audio/video tracks
+ // Currently we only choose the first track as the recorded track.
+ // FIXME: We would better to throw an exception to _javascript_ if writer creation fails.
-MediaRecorderPrivateAVFImpl::MediaRecorderPrivateAVFImpl(const MediaStreamPrivate& stream)
-{
- if (!m_writer.setupWriter())
- return;
- auto tracks = stream.tracks();
- bool videoSelected = false;
- bool audioSelected = false;
- for (auto& track : tracks) {
+ String audioTrackId;
+ String videoTrackId;
+ const MediaStreamTrackPrivate* audioTrack { nullptr };
+ const MediaStreamTrackPrivate* videoTrack { nullptr };
+ for (auto& track : stream.tracks()) {
if (!track->enabled() || track->ended())
continue;
switch (track->type()) {
case RealtimeMediaSource::Type::Video: {
auto& settings = track->settings();
- if (videoSelected || !settings.supportsWidth() || !settings.supportsHeight())
- break;
- // FIXME: we will need to implement support for multiple video tracks, currently we only choose the first track as the recorded track.
- // FIXME: we would better to throw an exception to _javascript_ if setVideoInput failed
- if (!m_writer.setVideoInput(settings.width(), settings.height()))
- return;
- m_recordedVideoTrackID = track->id();
- videoSelected = true;
+ if (!videoTrack && settings.supportsWidth() && settings.supportsHeight()) {
+ videoTrack = track.get();
+ videoTrackId = videoTrack->id();
+ }
break;
}
- case RealtimeMediaSource::Type::Audio: {
- if (audioSelected)
- break;
- // FIXME: we will need to implement support for multiple audio tracks, currently we only choose the first track as the recorded track.
- // FIXME: we would better to throw an exception to _javascript_ if setAudioInput failed
- if (!m_writer.setAudioInput())
- return;
- m_recordedAudioTrackID = track->id();
- audioSelected = true;
+ case RealtimeMediaSource::Type::Audio:
+ if (!audioTrack) {
+ audioTrack = track.get();
+ audioTrackId = audioTrack->id();
+ }
break;
- }
case RealtimeMediaSource::Type::None:
break;
}
}
- m_isWriterReady = true;
+ auto writer = MediaRecorderPrivateWriter::create(audioTrack, videoTrack);
+ if (!writer)
+ return nullptr;
+
+ return std::make_unique<MediaRecorderPrivateAVFImpl>(writer.releaseNonNull(), WTFMove(audioTrackId), WTFMove(videoTrackId));
}
+MediaRecorderPrivateAVFImpl::MediaRecorderPrivateAVFImpl(Ref<MediaRecorderPrivateWriter>&& writer, String&& audioTrackId, String&& videoTrackId)
+ : m_writer(WTFMove(writer))
+ , m_recordedAudioTrackID(WTFMove(audioTrackId))
+ , m_recordedVideoTrackID(WTFMove(videoTrackId))
+{
+}
+
void MediaRecorderPrivateAVFImpl::sampleBufferUpdated(MediaStreamTrackPrivate& track, MediaSample& sampleBuffer)
{
if (track.id() != m_recordedVideoTrackID)
return;
- m_writer.appendVideoSampleBuffer(sampleBuffer.platformSample().sample.cmSampleBuffer);
+ m_writer->appendVideoSampleBuffer(sampleBuffer.platformSample().sample.cmSampleBuffer);
}
void MediaRecorderPrivateAVFImpl::audioSamplesAvailable(MediaStreamTrackPrivate& track, const WTF::MediaTime& mediaTime, const PlatformAudioData& data, const AudioStreamDescription& description, size_t sampleCount)
@@ -98,17 +95,17 @@
return;
ASSERT(is<WebAudioBufferList>(data));
ASSERT(description.platformDescription().type == PlatformDescription::CAAudioStreamBasicType);
- m_writer.appendAudioSampleBuffer(data, description, mediaTime, sampleCount);
+ m_writer->appendAudioSampleBuffer(data, description, mediaTime, sampleCount);
}
void MediaRecorderPrivateAVFImpl::stopRecording()
{
- m_writer.stopRecording();
+ m_writer->stopRecording();
}
RefPtr<SharedBuffer> MediaRecorderPrivateAVFImpl::fetchData()
{
- return m_writer.fetchData();
+ return m_writer->fetchData();
}
const String& MediaRecorderPrivateAVFImpl::mimeType()
Modified: trunk/Source/WebCore/platform/mediarecorder/MediaRecorderPrivateAVFImpl.h (239242 => 239243)
--- trunk/Source/WebCore/platform/mediarecorder/MediaRecorderPrivateAVFImpl.h 2018-12-15 00:55:23 UTC (rev 239242)
+++ trunk/Source/WebCore/platform/mediarecorder/MediaRecorderPrivateAVFImpl.h 2018-12-15 01:07:18 UTC (rev 239243)
@@ -38,8 +38,10 @@
static std::unique_ptr<MediaRecorderPrivateAVFImpl> create(const MediaStreamPrivate&);
private:
- explicit MediaRecorderPrivateAVFImpl(const MediaStreamPrivate&);
+ MediaRecorderPrivateAVFImpl(Ref<MediaRecorderPrivateWriter>&&, String&& audioTrackId, String&& videoTrackId);
+ friend std::unique_ptr<MediaRecorderPrivateAVFImpl> std::make_unique<MediaRecorderPrivateAVFImpl>(Ref<MediaRecorderPrivateWriter>&&, String&&, String&&);
+
void sampleBufferUpdated(MediaStreamTrackPrivate&, MediaSample&) final;
void audioSamplesAvailable(MediaStreamTrackPrivate&, const WTF::MediaTime&, const PlatformAudioData&, const AudioStreamDescription&, size_t) final;
RefPtr<SharedBuffer> fetchData() final;
@@ -46,12 +48,9 @@
const String& mimeType() final;
void stopRecording();
+ Ref<MediaRecorderPrivateWriter> m_writer;
+ String m_recordedAudioTrackID;
String m_recordedVideoTrackID;
- String m_recordedAudioTrackID;
-
- MediaRecorderPrivateWriter m_writer;
-
- bool m_isWriterReady { false };
};
} // namespace WebCore
Modified: trunk/Source/WebCore/platform/mediarecorder/cocoa/MediaRecorderPrivateWriterCocoa.h (239242 => 239243)
--- trunk/Source/WebCore/platform/mediarecorder/cocoa/MediaRecorderPrivateWriterCocoa.h 2018-12-15 00:55:23 UTC (rev 239242)
+++ trunk/Source/WebCore/platform/mediarecorder/cocoa/MediaRecorderPrivateWriterCocoa.h 2018-12-15 01:07:18 UTC (rev 239243)
@@ -45,11 +45,12 @@
namespace WebCore {
class AudioStreamDescription;
+class MediaStreamTrackPrivate;
class PlatformAudioData;
class MediaRecorderPrivateWriter : public ThreadSafeRefCounted<MediaRecorderPrivateWriter, WTF::DestructionThread::Main>, public CanMakeWeakPtr<MediaRecorderPrivateWriter> {
public:
- MediaRecorderPrivateWriter() = default;
+ static RefPtr<MediaRecorderPrivateWriter> create(const MediaStreamTrackPrivate* audioTrack, const MediaStreamTrackPrivate* videoTrack);
~MediaRecorderPrivateWriter();
bool setupWriter();
@@ -61,6 +62,7 @@
RefPtr<SharedBuffer> fetchData();
private:
+ MediaRecorderPrivateWriter(RetainPtr<AVAssetWriter>&&, String&& path);
void clear();
RetainPtr<AVAssetWriter> m_writer;
Modified: trunk/Source/WebCore/platform/mediarecorder/cocoa/MediaRecorderPrivateWriterCocoa.mm (239242 => 239243)
--- trunk/Source/WebCore/platform/mediarecorder/cocoa/MediaRecorderPrivateWriterCocoa.mm 2018-12-15 00:55:23 UTC (rev 239242)
+++ trunk/Source/WebCore/platform/mediarecorder/cocoa/MediaRecorderPrivateWriterCocoa.mm 2018-12-15 01:07:18 UTC (rev 239243)
@@ -31,6 +31,7 @@
#include "AudioStreamDescription.h"
#include "FileSystem.h"
#include "Logging.h"
+#include "MediaStreamTrackPrivate.h"
#include "WebAudioBufferList.h"
#include <AVFoundation/AVAssetWriter.h>
#include <AVFoundation/AVAssetWriterInput.h>
@@ -86,6 +87,41 @@
using namespace PAL;
+RefPtr<MediaRecorderPrivateWriter> MediaRecorderPrivateWriter::create(const MediaStreamTrackPrivate* audioTrack, const MediaStreamTrackPrivate* videoTrack)
+{
+ NSString *directory = FileSystem::createTemporaryDirectory(@"videos");
+ NSString *filename = [NSString stringWithFormat:@"/%lld.mp4", CMClockGetTime(CMClockGetHostTimeClock()).value];
+ NSString *path = [directory stringByAppendingString:filename];
+
+ NSURL *outputURL = [NSURL fileURLWithPath:path];
+ String filePath = [path UTF8String];
+ NSError *error = nil;
+ auto avAssetWriter = adoptNS([allocAVAssetWriterInstance() initWithURL:outputURL fileType:AVFileTypeMPEG4 error:&error]);
+ if (error) {
+ RELEASE_LOG_ERROR(MediaStream, "create AVAssetWriter instance failed with error code %ld", (long)error.code);
+ return nullptr;
+ }
+
+ auto writer = adoptRef(*new MediaRecorderPrivateWriter(WTFMove(avAssetWriter), WTFMove(filePath)));
+
+ if (audioTrack && !writer->setAudioInput())
+ return nullptr;
+
+ if (videoTrack) {
+ auto& settings = videoTrack->settings();
+ if (!writer->setVideoInput(settings.width(), settings.height()))
+ return nullptr;
+ }
+
+ return WTFMove(writer);
+}
+
+MediaRecorderPrivateWriter::MediaRecorderPrivateWriter(RetainPtr<AVAssetWriter>&& avAssetWriter, String&& filePath)
+ : m_writer(WTFMove(avAssetWriter))
+ , m_path(WTFMove(filePath))
+{
+}
+
MediaRecorderPrivateWriter::~MediaRecorderPrivateWriter()
{
clear();
@@ -105,26 +141,6 @@
m_writer.clear();
}
-bool MediaRecorderPrivateWriter::setupWriter()
-{
- ASSERT(!m_writer);
-
- NSString *directory = FileSystem::createTemporaryDirectory(@"videos");
- NSString *filename = [NSString stringWithFormat:@"/%lld.mp4", CMClockGetTime(CMClockGetHostTimeClock()).value];
- NSString *path = [directory stringByAppendingString:filename];
-
- NSURL *outputURL = [NSURL fileURLWithPath:path];
- m_path = [path UTF8String];
- NSError *error = nil;
- m_writer = adoptNS([allocAVAssetWriterInstance() initWithURL:outputURL fileType:AVFileTypeMPEG4 error:&error]);
- if (error) {
- RELEASE_LOG_ERROR(MediaStream, "create AVAssetWriter instance failed with error code %ld", (long)error.code);
- m_writer = nullptr;
- return false;
- }
- return true;
-}
-
bool MediaRecorderPrivateWriter::setVideoInput(int width, int height)
{
ASSERT(!m_videoInput);
@@ -260,7 +276,7 @@
}
m_isFirstAudioSample = false;
RefPtr<MediaRecorderPrivateWriter> protectedThis = this;
- [m_audioInput requestMediaDataWhenReadyOnQueue:m_audioPullQueue usingBlock:[this, protectedThis] {
+ [m_audioInput requestMediaDataWhenReadyOnQueue:m_audioPullQueue usingBlock:[this, protectedThis = WTFMove(protectedThis)] {
do {
if (![m_audioInput isReadyForMoreMediaData])
break;
