Changeset 239243 in webkit


Ignore:
Timestamp:
Dec 14, 2018 5:07:18 PM (5 years ago)
Author:
youenn@apple.com
Message:

MediaRecorderPrivateAVFImpl should have a Ref<MediaRecorderPrivateWriter> as member
​https://bugs.webkit.org/show_bug.cgi?id=192720

Reviewed by Eric Carlson.

Source/WebCore:

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.

LayoutTests:

  • http/wpt/mediarecorder/MediaRecorder-onremovetrack-expected.txt: Added.
  • http/wpt/mediarecorder/MediaRecorder-onremovetrack.html: Added.
Location:
trunk
Files:
2 added
8 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r239226 r239243  
     12018-12-14  Youenn Fablet  <youenn@apple.com>
     2
     3        MediaRecorderPrivateAVFImpl should have a Ref<MediaRecorderPrivateWriter> as member
     4        https://bugs.webkit.org/show_bug.cgi?id=192720
     5
     6        Reviewed by Eric Carlson.
     7
     8        * http/wpt/mediarecorder/MediaRecorder-onremovetrack-expected.txt: Added.
     9        * http/wpt/mediarecorder/MediaRecorder-onremovetrack.html: Added.
     10
    1112018-12-14  Matt Baker  <mattbaker@apple.com>
    212

  • trunk/Source/WebCore/ChangeLog

    r239238 r239243  
     12018-12-14  Youenn Fablet  <youenn@apple.com>
     2
     3        MediaRecorderPrivateAVFImpl should have a Ref<MediaRecorderPrivateWriter> as member
     4        https://bugs.webkit.org/show_bug.cgi?id=192720
     5
     6        Reviewed by Eric Carlson.
     7
     8        Make sure that MediaRecorderPrivateAVFImpl takes a Ref<MediaRecorderPrivateWriter> as member,
     9        as the latter is a ref counted object.
     10        Made some refactoring to return early in case of error.
     11
     12        Also made sure that in the case of a MediaRecorder stopped by a track removal in the recorded stream
     13        the MediaRecorder will stop listening for its tracks.
     14        Otherwise, the tracks will continue calling the MediaRecorder even after it is dead.
     15
     16        Test: http/wpt/mediarecorder/MediaRecorder-onremovetrack.html
     17
     18        * Modules/mediarecorder/MediaRecorder.cpp:
     19        (WebCore::MediaRecorder::didAddOrRemoveTrack):
     20        (WebCore::MediaRecorder::setNewRecordingState): Deleted.
     21        * Modules/mediarecorder/MediaRecorder.h:
     22        * platform/mediarecorder/MediaRecorderPrivateAVFImpl.cpp:
     23        (WebCore::MediaRecorderPrivateAVFImpl::create):
     24        (WebCore::MediaRecorderPrivateAVFImpl::MediaRecorderPrivateAVFImpl):
     25        (WebCore::MediaRecorderPrivateAVFImpl::sampleBufferUpdated):
     26        (WebCore::MediaRecorderPrivateAVFImpl::audioSamplesAvailable):
     27        (WebCore::MediaRecorderPrivateAVFImpl::stopRecording):
     28        (WebCore::MediaRecorderPrivateAVFImpl::fetchData):
     29        * platform/mediarecorder/MediaRecorderPrivateAVFImpl.h:
     30        * platform/mediarecorder/cocoa/MediaRecorderPrivateWriterCocoa.h:
     31        * platform/mediarecorder/cocoa/MediaRecorderPrivateWriterCocoa.mm:
     32        (WebCore::MediaRecorderPrivateWriter::create):
     33        (WebCore::MediaRecorderPrivateWriter::MediaRecorderPrivateWriter):
     34        (WebCore::MediaRecorderPrivateWriter::appendAudioSampleBuffer):
     35        (WebCore::MediaRecorderPrivateWriter::setupWriter): Deleted.
     36
    1372018-12-14  Youenn Fablet  <youenn@apple.com>
    238

  • trunk/Source/WebCore/Modules/mediarecorder/MediaRecorder.cpp

    r239145 r239243  
    162162        if (!m_isActive || state() == RecordingState::Inactive)
    163163            return;
     164        stopRecordingInternal();
    164165        auto event = MediaRecorderErrorEvent::create(eventNames().errorEvent, Exception { UnknownError, "Track cannot be added to or removed from the MediaStream while recording is happening"_s });
    165         setNewRecordingState(RecordingState::Inactive, WTFMove(event));
     166        dispatchEvent(WTFMove(event));
    166167    });
    167168}
    … …  
    186187{
    187188    m_private->audioSamplesAvailable(track, mediaTime, audioData, description, sampleCount);
    188 }
    189 
    190 void MediaRecorder::setNewRecordingState(RecordingState newState, Ref<Event>&& event)
    191 {
    192     m_state = newState;
    193     dispatchEvent(WTFMove(event));
    194189}
    195190

  • trunk/Source/WebCore/Modules/mediarecorder/MediaRecorder.h

    r239145 r239243  
    104104   
    105105    void scheduleDeferredTask(Function<void()>&&);
    106     void setNewRecordingState(RecordingState, Ref<Event>&&);
    107106   
    108107    static creatorFunction m_customCreator;

  • trunk/Source/WebCore/platform/mediarecorder/MediaRecorderPrivateAVFImpl.cpp

    r239145 r239243  
    3939std::unique_ptr<MediaRecorderPrivateAVFImpl> MediaRecorderPrivateAVFImpl::create(const MediaStreamPrivate& stream)
    4040{
    41     auto instance = std::unique_ptr<MediaRecorderPrivateAVFImpl>(new MediaRecorderPrivateAVFImpl(stream));
    42     if (!instance->m_isWriterReady)
    43         return nullptr;
    44     return instance;
    45 }
     41    // FIXME: we will need to implement support for multiple audio/video tracks
     42    // Currently we only choose the first track as the recorded track.
     43    // FIXME: We would better to throw an exception to JavaScript if writer creation fails.
    4644
    47 MediaRecorderPrivateAVFImpl::MediaRecorderPrivateAVFImpl(const MediaStreamPrivate& stream)
    48 {
    49     if (!m_writer.setupWriter())
    50         return;
    51     auto tracks = stream.tracks();
    52     bool videoSelected = false;
    53     bool audioSelected = false;
    54     for (auto& track : tracks) {
     45    String audioTrackId;
     46    String videoTrackId;
     47    const MediaStreamTrackPrivate* audioTrack { nullptr };
     48    const MediaStreamTrackPrivate* videoTrack { nullptr };
     49    for (auto& track : stream.tracks()) {
    5550        if (!track->enabled() || track->ended())
    5651            continue;
    … …  
    5853        case RealtimeMediaSource::Type::Video: {
    5954            auto& settings = track->settings();
    60             if (videoSelected || !settings.supportsWidth() || !settings.supportsHeight())
    61                 break;
    62             // FIXME: we will need to implement support for multiple video tracks, currently we only choose the first track as the recorded track.
    63             // FIXME: we would better to throw an exception to JavaScript if setVideoInput failed
    64             if (!m_writer.setVideoInput(settings.width(), settings.height()))
    65                 return;
    66             m_recordedVideoTrackID = track->id();
    67             videoSelected = true;
     55            if (!videoTrack && settings.supportsWidth() && settings.supportsHeight()) {
     56                videoTrack = track.get();
     57                videoTrackId = videoTrack->id();
     58            }
    6859            break;
    6960        }
    70         case RealtimeMediaSource::Type::Audio: {
    71             if (audioSelected)
    72                 break;
    73             // FIXME: we will need to implement support for multiple audio tracks, currently we only choose the first track as the recorded track.
    74             // FIXME: we would better to throw an exception to JavaScript if setAudioInput failed
    75             if (!m_writer.setAudioInput())
    76                 return;
    77             m_recordedAudioTrackID = track->id();
    78             audioSelected = true;
     61        case RealtimeMediaSource::Type::Audio:
     62            if (!audioTrack) {
     63                audioTrack = track.get();
     64                audioTrackId = audioTrack->id();
     65            }
    7966            break;
    80         }
    8167        case RealtimeMediaSource::Type::None:
    8268            break;
    8369        }
    8470    }
    85     m_isWriterReady = true;
     71    auto writer = MediaRecorderPrivateWriter::create(audioTrack, videoTrack);
     72    if (!writer)
     73        return nullptr;
     74
     75    return std::make_unique<MediaRecorderPrivateAVFImpl>(writer.releaseNonNull(), WTFMove(audioTrackId), WTFMove(videoTrackId));
     76}
     77
     78MediaRecorderPrivateAVFImpl::MediaRecorderPrivateAVFImpl(Ref<MediaRecorderPrivateWriter>&& writer, String&& audioTrackId, String&& videoTrackId)
     79    : m_writer(WTFMove(writer))
     80    , m_recordedAudioTrackID(WTFMove(audioTrackId))
     81    , m_recordedVideoTrackID(WTFMove(videoTrackId))
     82{
    8683}
    8784
    … …  
    9087    if (track.id() != m_recordedVideoTrackID)
    9188        return;
    92     m_writer.appendVideoSampleBuffer(sampleBuffer.platformSample().sample.cmSampleBuffer);
     89    m_writer->appendVideoSampleBuffer(sampleBuffer.platformSample().sample.cmSampleBuffer);
    9390}
    9491
    … …  
    9996    ASSERT(is<WebAudioBufferList>(data));
    10097    ASSERT(description.platformDescription().type == PlatformDescription::CAAudioStreamBasicType);
    101     m_writer.appendAudioSampleBuffer(data, description, mediaTime, sampleCount);
     98    m_writer->appendAudioSampleBuffer(data, description, mediaTime, sampleCount);
    10299}
    103100
    104101void MediaRecorderPrivateAVFImpl::stopRecording()
    105102{
    106     m_writer.stopRecording();
     103    m_writer->stopRecording();
    107104}
    108105
    109106RefPtr<SharedBuffer> MediaRecorderPrivateAVFImpl::fetchData()
    110107{
    111     return m_writer.fetchData();
     108    return m_writer->fetchData();
    112109}
    113110

  • trunk/Source/WebCore/platform/mediarecorder/MediaRecorderPrivateAVFImpl.h

    r239145 r239243  
    3939
    4040private:
    41     explicit MediaRecorderPrivateAVFImpl(const MediaStreamPrivate&);
     41    MediaRecorderPrivateAVFImpl(Ref<MediaRecorderPrivateWriter>&&, String&& audioTrackId, String&& videoTrackId);
     42
     43    friend std::unique_ptr<MediaRecorderPrivateAVFImpl> std::make_unique<MediaRecorderPrivateAVFImpl>(Ref<MediaRecorderPrivateWriter>&&, String&&, String&&);
    4244
    4345    void sampleBufferUpdated(MediaStreamTrackPrivate&, MediaSample&) final;
    … …  
    4749    void stopRecording();
    4850   
     51    Ref<MediaRecorderPrivateWriter> m_writer;
     52    String m_recordedAudioTrackID;
    4953    String m_recordedVideoTrackID;
    50     String m_recordedAudioTrackID;
    51 
    52     MediaRecorderPrivateWriter m_writer;
    53    
    54     bool m_isWriterReady { false };
    5554};
    5655

  • trunk/Source/WebCore/platform/mediarecorder/cocoa/MediaRecorderPrivateWriterCocoa.h

    r239145 r239243  
    4646
    4747class AudioStreamDescription;
     48class MediaStreamTrackPrivate;
    4849class PlatformAudioData;
    4950
    5051class MediaRecorderPrivateWriter : public ThreadSafeRefCounted<MediaRecorderPrivateWriter, WTF::DestructionThread::Main>, public CanMakeWeakPtr<MediaRecorderPrivateWriter> {
    5152public:
    52     MediaRecorderPrivateWriter() = default;
     53    static RefPtr<MediaRecorderPrivateWriter> create(const MediaStreamTrackPrivate* audioTrack, const MediaStreamTrackPrivate* videoTrack);
    5354    ~MediaRecorderPrivateWriter();
    5455   
    … …  
    6263   
    6364private:
     65    MediaRecorderPrivateWriter(RetainPtr<AVAssetWriter>&&, String&& path);
    6466    void clear();
    6567

  • trunk/Source/WebCore/platform/mediarecorder/cocoa/MediaRecorderPrivateWriterCocoa.mm

    r239145 r239243  
    3232#include "FileSystem.h"
    3333#include "Logging.h"
     34#include "MediaStreamTrackPrivate.h"
    3435#include "WebAudioBufferList.h"
    3536#include <AVFoundation/AVAssetWriter.h>
    … …  
    8788using namespace PAL;
    8889
     90RefPtr<MediaRecorderPrivateWriter> MediaRecorderPrivateWriter::create(const MediaStreamTrackPrivate* audioTrack, const MediaStreamTrackPrivate* videoTrack)
     91{
     92    NSString *directory = FileSystem::createTemporaryDirectory(@"videos");
     93    NSString *filename = [NSString stringWithFormat:@"/%lld.mp4", CMClockGetTime(CMClockGetHostTimeClock()).value];
     94    NSString *path = [directory stringByAppendingString:filename];
     95
     96    NSURL *outputURL = [NSURL fileURLWithPath:path];
     97    String filePath = [path UTF8String];
     98    NSError *error = nil;
     99    auto avAssetWriter = adoptNS([allocAVAssetWriterInstance() initWithURL:outputURL fileType:AVFileTypeMPEG4 error:&error]);
     100    if (error) {
     101        RELEASE_LOG_ERROR(MediaStream, "create AVAssetWriter instance failed with error code %ld", (long)error.code);
     102        return nullptr;
     103    }
     104
     105    auto writer = adoptRef(*new MediaRecorderPrivateWriter(WTFMove(avAssetWriter), WTFMove(filePath)));
     106
     107    if (audioTrack && !writer->setAudioInput())
     108        return nullptr;
     109
     110    if (videoTrack) {
     111        auto& settings = videoTrack->settings();
     112        if (!writer->setVideoInput(settings.width(), settings.height()))
     113            return nullptr;
     114    }
     115
     116    return WTFMove(writer);
     117}
     118
     119MediaRecorderPrivateWriter::MediaRecorderPrivateWriter(RetainPtr<AVAssetWriter>&& avAssetWriter, String&& filePath)
     120    : m_writer(WTFMove(avAssetWriter))
     121    , m_path(WTFMove(filePath))
     122{
     123}
     124
    89125MediaRecorderPrivateWriter::~MediaRecorderPrivateWriter()
    90126{
    … …  
    104140    if (m_writer)
    105141        m_writer.clear();
    106 }
    107 
    108 bool MediaRecorderPrivateWriter::setupWriter()
    109 {
    110     ASSERT(!m_writer);
    111    
    112     NSString *directory = FileSystem::createTemporaryDirectory(@"videos");
    113     NSString *filename = [NSString stringWithFormat:@"/%lld.mp4", CMClockGetTime(CMClockGetHostTimeClock()).value];
    114     NSString *path = [directory stringByAppendingString:filename];
    115 
    116     NSURL *outputURL = [NSURL fileURLWithPath:path];
    117     m_path = [path UTF8String];
    118     NSError *error = nil;
    119     m_writer = adoptNS([allocAVAssetWriterInstance() initWithURL:outputURL fileType:AVFileTypeMPEG4 error:&error]);
    120     if (error) {
    121         RELEASE_LOG_ERROR(MediaStream, "create AVAssetWriter instance failed with error code %ld", (long)error.code);
    122         m_writer = nullptr;
    123         return false;
    124     }
    125     return true;
    126142}
    127143
    … …  
    261277        m_isFirstAudioSample = false;
    262278        RefPtr<MediaRecorderPrivateWriter> protectedThis = this;
    263         [m_audioInput requestMediaDataWhenReadyOnQueue:m_audioPullQueue usingBlock:[this, protectedThis] {
     279        [m_audioInput requestMediaDataWhenReadyOnQueue:m_audioPullQueue usingBlock:[this, protectedThis = WTFMove(protectedThis)] {
    264280            do {
    265281                if (![m_audioInput isReadyForMoreMediaData])
Note: See TracChangeset for help on using the changeset viewer.