Bug 123322 – [MSE] Add mock MediaSource classes for testing.

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Jer Noble]

[MSE] Add mock MediaSource classes for testing.

[Jer Noble]

Created attachment 215524 [details]
Patch

[Eric Carlson]

Comment on attachment 215524 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=215524&action=review

r=me with a few nits.

> Source/WebCore/platform/graphics/MediaPlayer.cpp:1213
> +void MediaPlayerFactorySupport::callRegisterMediaEngine(MediaEngineRegister registerMediaEngine)

"callRegisterMediaEngine" is an odd name for an externally visible method (even though that is what it does).

> Source/WebCore/platform/graphics/avfoundation/MediaPlayerPrivateAVFoundation.cpp:206
> +#if ENABLE(MEDIA_SOURCE)
> +void MediaPlayerPrivateAVFoundation::load(const String&, PassRefPtr<HTMLMediaSource>)
> +{
> +    m_networkState = MediaPlayer::FormatError;
> +    m_player->networkStateChanged();
> +}
> +#endif

This was part of bug 123593.

> Source/WebCore/platform/graphics/avfoundation/MediaPlayerPrivateAVFoundation.h:133
> +    virtual void load(const String&, PassRefPtr<HTMLMediaSource>);

Ditto.

> Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm:910
> +#if ENABLE(MEDIA_SOURCE)
> +    if (parameters.isMediaSource)
> +        return MediaPlayer::IsNotSupported;
> +#endif

Ditto.

> Source/WebCore/platform/graphics/mac/MediaPlayerPrivateQTKit.h:98
> +    virtual void load(const String&, PassRefPtr<HTMLMediaSource>);

Ditto.

> Source/WebCore/platform/graphics/mac/MediaPlayerPrivateQTKit.mm:678
> +#if ENABLE(MEDIA_SOURCE)
> +void MediaPlayerPrivateQTKit::load(const String&, PassRefPtr<HTMLMediaSource>)
> +{
> +    m_networkState = MediaPlayer::FormatError;
> +    m_player->networkStateChanged();
> +}
> +#endif

Ditto.

> Source/WebCore/platform/graphics/mac/MediaPlayerPrivateQTKit.mm:1481
> +#if ENABLE(MEDIA_SOURCE)
> +    if (parameters.isMediaSource)
> +        return MediaPlayer::IsNotSupported;
> +#endif

Ditto.

> Source/WebCore/platform/mock/mediasource/MockBox.cpp:42
> +MockBox::MockBox(ArrayBuffer* data)
> +{
> +    m_type = peekType(data);
> +    m_length = peekLength(data);
> +}

Should you check that data is big enough for a box?

> Source/WebCore/platform/mock/mediasource/MockBox.cpp:63
> +    RefPtr<JSC::DataView> view = JSC::DataView::create(data, 0, data->byteLength());
> +    m_trackID = view->get<int32_t>(8, true);

Ditto.

> Source/WebCore/platform/mock/mediasource/MockBox.cpp:83
> +    RefPtr<JSC::DataView> view = JSC::DataView::create(data, 0, data->byteLength());

Ditto.

> Source/WebCore/platform/mock/mediasource/MockBox.cpp:110
> +    RefPtr<JSC::DataView> view = JSC::DataView::create(data, 0, data->byteLength());

Ditto.

> Source/WebCore/platform/mock/mediasource/MockBox.h:70
> +class MockTrackBox : public MockBox {

Can this be marked FINAL?

> Source/WebCore/platform/mock/mediasource/MockBox.h:97
> +class MockInitializationBox : public MockBox {

Ditto.

> Source/WebCore/platform/mock/mediasource/MockBox.h:121
> +class MockSampleBox : public MockBox {

Ditto.

> Source/WebCore/platform/mock/mediasource/MockMediaPlayerMediaSource.h:38
> +class MockMediaPlayerMediaSource : public MediaPlayerPrivateInterface {

Can this be marked FINAL?

> Source/WebCore/platform/mock/mediasource/MockMediaPlayerMediaSource.h:72
> +    // MediaPlayerPrivate Overrides
> +    virtual void load(const String& url) OVERRIDE;
> +    virtual void load(const String& url, PassRefPtr<HTMLMediaSource>) OVERRIDE;
> +    virtual void cancelLoad() OVERRIDE;
> +    virtual void play() OVERRIDE;
> +    virtual void pause() OVERRIDE;
> +    virtual IntSize naturalSize() const OVERRIDE;
> +    virtual bool hasVideo() const OVERRIDE;
> +    virtual bool hasAudio() const OVERRIDE;
> +    virtual void setVisible(bool) OVERRIDE;
> +    virtual bool seeking() const OVERRIDE;
> +    virtual bool paused() const OVERRIDE;
> +    virtual MediaPlayer::NetworkState networkState() const OVERRIDE;
> +    virtual MediaPlayer::ReadyState readyState() const OVERRIDE;
> +    virtual PassRefPtr<TimeRanges> buffered() const OVERRIDE;
> +    virtual bool didLoadingProgress() const OVERRIDE;
> +    virtual void setSize(const IntSize&) OVERRIDE;
> +    virtual void paint(GraphicsContext*, const IntRect&) OVERRIDE;
> +    virtual double currentTimeDouble() const OVERRIDE;
> +    virtual double durationDouble() const OVERRIDE;
> +    virtual void seekDouble(double time) OVERRIDE;
> +
> +    void advanceCurrentTime();
> +    void updateDuration(double);
> +    void setReadyState(MediaPlayer::ReadyState);

Can these be made private?

> Source/WebCore/platform/mock/mediasource/MockMediaPlayerMediaSource.h:81
> +    bool m_playing;

Nit: packing.

> Source/WebCore/platform/mock/mediasource/MockMediaSourcePrivate.cpp:131
> +    RefPtr<MockSourceBufferPrivate> sourceBuffer = prpSourceBuffer;
> +    return sourceBuffer->hasAudio();

Nit: the local isn't necessary.

> Source/WebCore/platform/mock/mediasource/MockMediaSourcePrivate.cpp:141
> +    RefPtr<MockSourceBufferPrivate> sourceBuffer = prpSourceBuffer;

Ditto.

> Source/WebCore/platform/mock/mediasource/MockMediaSourcePrivate.h:39
> +class MockMediaSourcePrivate : public MediaSourcePrivate {

Can this be marked FINAL?

> Source/WebCore/platform/mock/mediasource/MockMediaSourcePrivate.h:53
> +    // MediaSourcePrivate Overrides
> +    virtual AddStatus addSourceBuffer(const ContentType&, RefPtr<SourceBufferPrivate>&) OVERRIDE;
> +    virtual double duration() OVERRIDE;
> +    virtual void setDuration(double) OVERRIDE;
> +    virtual void markEndOfStream(EndOfStreamStatus) OVERRIDE;
> +    virtual void unmarkEndOfStream() OVERRIDE;
> +    virtual MediaPlayer::ReadyState readyState() const OVERRIDE;
> +    virtual void setReadyState(MediaPlayer::ReadyState) OVERRIDE;
> +
> +    const Vector<MockSourceBufferPrivate*>& activeSourceBuffers() const { return m_activeSourceBuffers; }

Can these be made private?

> Source/WebCore/platform/mock/mediasource/MockSourceBufferPrivate.cpp:44
> +class MockMediaSample : public MediaSample {

FINAL?

> Source/WebCore/platform/mock/mediasource/MockSourceBufferPrivate.cpp:46
> +    static RefPtr<MockMediaSample> create(MockSampleBox box) { return adoptRef(new MockMediaSample(box)); }

const MockSampleBox&

> Source/WebCore/platform/mock/mediasource/MockSourceBufferPrivate.cpp:58
> +    MockMediaSample(MockSampleBox box)

Ditto.

> Source/WebCore/platform/mock/mediasource/MockSourceBufferPrivate.cpp:74
> +class MockMediaDescription : public MediaDescription {

FINAL?

> Source/WebCore/platform/mock/mediasource/MockSourceBufferPrivate.cpp:76
> +    static RefPtr<MockMediaDescription> create(MockTrackBox box) { return adoptRef(new MockMediaDescription(box)); }

const MockTrackBox&

> Source/WebCore/platform/mock/mediasource/MockSourceBufferPrivate.cpp:85
> +    MockMediaDescription(MockTrackBox box) : m_box(box) { }

Ditto.

> Source/WebCore/platform/mock/mediasource/MockSourceBufferPrivate.cpp:143
> +        MockTrackBox trackBox = *it;

MockTrackBox&

> Source/WebCore/platform/mock/mediasource/MockTracks.h:40
> +    static RefPtr<MockAudioTrackPrivate> create(MockTrackBox box) { return adoptRef(new MockAudioTrackPrivate(box)); }

const MockTrackBox&

> Source/WebCore/platform/mock/mediasource/MockTracks.h:46
> +    MockAudioTrackPrivate(MockTrackBox box)

Ditto.

> Source/WebCore/platform/mock/mediasource/MockTracks.h:57
> +    static RefPtr<MockTextTrackPrivate> create(MockTrackBox box) { return adoptRef(new MockTextTrackPrivate(box)); }

Ditto.

> Source/WebCore/platform/mock/mediasource/MockTracks.h:63
> +    MockTextTrackPrivate(MockTrackBox box)

Ditto.

> Source/WebCore/platform/mock/mediasource/MockTracks.h:76
> +    static RefPtr<MockVideoTrackPrivate> create(MockTrackBox box) { return adoptRef(new MockVideoTrackPrivate(box)); }

Ditto.

> Source/WebCore/platform/mock/mediasource/MockTracks.h:82
> +    MockVideoTrackPrivate(MockTrackBox box)

Ditto.

> LayoutTests/ChangeLog:71
> +

Four ChangeLog entries?

[Jer Noble]

(In reply to comment #2)
> (From update of attachment 215524 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=215524&action=review
> 
> r=me with a few nits.
> 
> > Source/WebCore/platform/graphics/MediaPlayer.cpp:1213
> > +void MediaPlayerFactorySupport::callRegisterMediaEngine(MediaEngineRegister registerMediaEngine)
> 
> "callRegisterMediaEngine" is an odd name for an externally visible method (even though that is what it does).

I agonized over this, but couldn't come up with another name with which I was happy.

> > Source/WebCore/platform/graphics/avfoundation/MediaPlayerPrivateAVFoundation.cpp:206
> > +#if ENABLE(MEDIA_SOURCE)
> > +void MediaPlayerPrivateAVFoundation::load(const String&, PassRefPtr<HTMLMediaSource>)
> > +{
> > +    m_networkState = MediaPlayer::FormatError;
> > +    m_player->networkStateChanged();
> > +}
> > +#endif
> 
> This was part of bug 123593.

Removed this and the following dittos.

> > Source/WebCore/platform/mock/mediasource/MockBox.cpp:42
> > +MockBox::MockBox(ArrayBuffer* data)
> > +{
> > +    m_type = peekType(data);
> > +    m_length = peekLength(data);
> > +}
> 
> Should you check that data is big enough for a box?

I added an ASSERT here (and the following dittos).

> > Source/WebCore/platform/mock/mediasource/MockBox.h:70
> > +class MockTrackBox : public MockBox {
> 
> Can this be marked FINAL?

Yes.  And the dittos as well.

> > Source/WebCore/platform/mock/mediasource/MockMediaPlayerMediaSource.h:38
> > +class MockMediaPlayerMediaSource : public MediaPlayerPrivateInterface {
> 
> Can this be marked FINAL?

No.  Marking it as final (somehow) breaks passing this class into Function objects.

> > Source/WebCore/platform/mock/mediasource/MockMediaPlayerMediaSource.h:72
> > +    // MediaPlayerPrivate Overrides
> > +    virtual void load(const String& url) OVERRIDE;
> > +    virtual void load(const String& url, PassRefPtr<HTMLMediaSource>) OVERRIDE;
> > +    virtual void cancelLoad() OVERRIDE;
> > +    virtual void play() OVERRIDE;
> > +    virtual void pause() OVERRIDE;
> > +    virtual IntSize naturalSize() const OVERRIDE;
> > +    virtual bool hasVideo() const OVERRIDE;
> > +    virtual bool hasAudio() const OVERRIDE;
> > +    virtual void setVisible(bool) OVERRIDE;
> > +    virtual bool seeking() const OVERRIDE;
> > +    virtual bool paused() const OVERRIDE;
> > +    virtual MediaPlayer::NetworkState networkState() const OVERRIDE;
> > +    virtual MediaPlayer::ReadyState readyState() const OVERRIDE;
> > +    virtual PassRefPtr<TimeRanges> buffered() const OVERRIDE;
> > +    virtual bool didLoadingProgress() const OVERRIDE;
> > +    virtual void setSize(const IntSize&) OVERRIDE;
> > +    virtual void paint(GraphicsContext*, const IntRect&) OVERRIDE;
> > +    virtual double currentTimeDouble() const OVERRIDE;
> > +    virtual double durationDouble() const OVERRIDE;
> > +    virtual void seekDouble(double time) OVERRIDE;
> > +
> > +    void advanceCurrentTime();
> > +    void updateDuration(double);
> > +    void setReadyState(MediaPlayer::ReadyState);
> 
> Can these be made private?

All except for readyState().

> > Source/WebCore/platform/mock/mediasource/MockMediaPlayerMediaSource.h:81
> > +    bool m_playing;
> 
> Nit: packing.

Sorted.

> > Source/WebCore/platform/mock/mediasource/MockMediaSourcePrivate.cpp:131
> > +    RefPtr<MockSourceBufferPrivate> sourceBuffer = prpSourceBuffer;
> > +    return sourceBuffer->hasAudio();
> 
> Nit: the local isn't necessary.

Removed.

> > Source/WebCore/platform/mock/mediasource/MockMediaSourcePrivate.cpp:141
> > +    RefPtr<MockSourceBufferPrivate> sourceBuffer = prpSourceBuffer;
> 
> Ditto.

Ditto.

> > Source/WebCore/platform/mock/mediasource/MockMediaSourcePrivate.h:39
> > +class MockMediaSourcePrivate : public MediaSourcePrivate {
> 
> Can this be marked FINAL?

Yes.

> > Source/WebCore/platform/mock/mediasource/MockMediaSourcePrivate.h:53
> > +    // MediaSourcePrivate Overrides
> > +    virtual AddStatus addSourceBuffer(const ContentType&, RefPtr<SourceBufferPrivate>&) OVERRIDE;
> > +    virtual double duration() OVERRIDE;
> > +    virtual void setDuration(double) OVERRIDE;
> > +    virtual void markEndOfStream(EndOfStreamStatus) OVERRIDE;
> > +    virtual void unmarkEndOfStream() OVERRIDE;
> > +    virtual MediaPlayer::ReadyState readyState() const OVERRIDE;
> > +    virtual void setReadyState(MediaPlayer::ReadyState) OVERRIDE;
> > +
> > +    const Vector<MockSourceBufferPrivate*>& activeSourceBuffers() const { return m_activeSourceBuffers; }
> 
> Can these be made private?

Yes.

> > Source/WebCore/platform/mock/mediasource/MockSourceBufferPrivate.cpp:44
> > +class MockMediaSample : public MediaSample {
> 
> FINAL?

Yes.

> > Source/WebCore/platform/mock/mediasource/MockSourceBufferPrivate.cpp:46
> > +    static RefPtr<MockMediaSample> create(MockSampleBox box) { return adoptRef(new MockMediaSample(box)); }
> 
> const MockSampleBox&

Yes.

> > Source/WebCore/platform/mock/mediasource/MockSourceBufferPrivate.cpp:58
> > +    MockMediaSample(MockSampleBox box)
> 
> Ditto.

Yes.

> > Source/WebCore/platform/mock/mediasource/MockSourceBufferPrivate.cpp:74
> > +class MockMediaDescription : public MediaDescription {
> 
> FINAL?

Yes.

> > Source/WebCore/platform/mock/mediasource/MockSourceBufferPrivate.cpp:76
> > +    static RefPtr<MockMediaDescription> create(MockTrackBox box) { return adoptRef(new MockMediaDescription(box)); }
> 
> const MockTrackBox&

Yes.

> > Source/WebCore/platform/mock/mediasource/MockSourceBufferPrivate.cpp:85
> > +    MockMediaDescription(MockTrackBox box) : m_box(box) { }
> 
> Ditto.

Yes.

> > Source/WebCore/platform/mock/mediasource/MockSourceBufferPrivate.cpp:143
> > +        MockTrackBox trackBox = *it;
> 
> MockTrackBox&

const MockTrackBox&.

> > Source/WebCore/platform/mock/mediasource/MockTracks.h:40
> > +    static RefPtr<MockAudioTrackPrivate> create(MockTrackBox box) { return adoptRef(new MockAudioTrackPrivate(box)); }
> 
> const MockTrackBox&

Ok.

> > Source/WebCore/platform/mock/mediasource/MockTracks.h:46
> > +    MockAudioTrackPrivate(MockTrackBox box)
> 
> Ditto.

Ok.

> > Source/WebCore/platform/mock/mediasource/MockTracks.h:57
> > +    static RefPtr<MockTextTrackPrivate> create(MockTrackBox box) { return adoptRef(new MockTextTrackPrivate(box)); }
> 
> Ditto.

Ok.

> > Source/WebCore/platform/mock/mediasource/MockTracks.h:63
> > +    MockTextTrackPrivate(MockTrackBox box)
> 
> Ditto.

Ok.

> > Source/WebCore/platform/mock/mediasource/MockTracks.h:76
> > +    static RefPtr<MockVideoTrackPrivate> create(MockTrackBox box) { return adoptRef(new MockVideoTrackPrivate(box)); }
> 
> Ditto.

Ok.

> > Source/WebCore/platform/mock/mediasource/MockTracks.h:82
> > +    MockVideoTrackPrivate(MockTrackBox box)
> 
> Ditto.

Ok.

> > LayoutTests/ChangeLog:71
> > +
> 
> Four ChangeLog entries?

Removed.

[Jer Noble]

Created attachment 216305 [details]
Patch for landing

[Jer Noble]

Created attachment 216306 [details]
Patch for landing

[EFL EWS Bot]

Comment on attachment 216306 [details]
Patch for landing

Attachment 216306 [details] did not pass efl-ews (efl):
Output: http://webkit-queues.appspot.com/results/22748239

[EFL EWS Bot]

Comment on attachment 216306 [details]
Patch for landing

Attachment 216306 [details] did not pass efl-wk2-ews (efl-wk2):
Output: http://webkit-queues.appspot.com/results/22778214

[kov's GTK+ EWS bot]

Comment on attachment 216306 [details]
Patch for landing

Attachment 216306 [details] did not pass gtk-ews (gtk):
Output: http://webkit-queues.appspot.com/results/22778215

[Jer Noble]

Created attachment 216309 [details]
Patch for landing

Fixed the build error in Internals.cpp for ports which do not enable MEDIA_SOURCE.

[Jer Noble]

Committed r158853: <http://trac.webkit.org/changeset/158853>