Bug 219822 – REGRESSION(r270612) [GStreamer][MSE] Some play/seek related failures

• Status: RESOLVED
• Resolution: FIXED
• Component: Media

[Lauro Moura]

After the fix of bug219565, there are still some failures playing/seeking, likely related to r270612.

List of tests failing locally (to be gardened together with the fix of bug219565):

# Failures

imported/w3c/web-platform-tests/media-source/mediasource-buffered-seek.html [ Failure ]
imported/w3c/web-platform-tests/media-source/mediasource-changetype-play-negative.html [ Failure ]
imported/w3c/web-platform-tests/media-source/mediasource-h264-play-starved.html [ Failure ]
imported/w3c/web-platform-tests/media-source/mediasource-liveseekable.html [ Failure ]
imported/w3c/web-platform-tests/media-source/mediasource-play-then-seek-back.html [ Failure ]
imported/w3c/web-platform-tests/media-source/mediasource-redundant-seek.html [ Failure ]
imported/w3c/web-platform-tests/media-source/mediasource-seek-during-pending-seek.html [ Failure ]
imported/w3c/web-platform-tests/media-source/mediasource-seekable.html [ Failure ]
imported/w3c/web-platform-tests/media-source/mediasource-sequencemode-append-buffer.html [ Failure ]
media/media-source/media-source-canplaythrough-event.html [ Failure ]
media/media-source/media-source-stalled-holds-sleep-assertion.html [ Failure ]
media/media-source/media-source-unnecessary-seek-seeked.html [ Failure ]

# Timeouts
imported/w3c/web-platform-tests/media-source/mediasource-video-is-visible.html [ Timeout ]
media/encrypted-media/clearKey/clearKey-cenc-audio-playback-mse.html [ Timeout ]
media/encrypted-media/clearKey/clearKey-cenc-video-playback-mse.html [ Timeout ]
media/media-source/media-source-paint-to-canvas.html [ Timeout ]
media/media-source/media-source-seek-redundant-append.html [ Timeout ]
media/media-source/media-source-seek-twice.html [ Timeout ]

[Philippe Normand]

Adding MSE experts in CC...

I had a quick look, the append pipeline sink receives no sample and that triggers a typefind error down the line during seeks...

[Philippe Normand]

The biggest issue is that allSamplesInTrackEnqueued() is now called too early and signals EOS even before the pipeline had a chance to preroll. This regression was introduced in r270612.

I'm preparing a patch...

[Philippe Normand]

Another issue is that the SourceBufferPrivateGStreamer doesn't call appendComplete(), so the parent class doesn't correctly synchronize the TrackBuffer buffered ranges.

[Alicia Boya García]

(In reply to Philippe Normand from comment #2)
> The biggest issue is that allSamplesInTrackEnqueued() is now called too
> early and signals EOS even before the pipeline had a chance to preroll. This
> regression was introduced in r270612.
> 
> I'm preparing a patch...

I'm also working on this.

[Philippe Normand]

Created attachment 416660 [details]
Patch

[Xabier Rodríguez Calvar]

Comment on attachment 416660 [details]
Patch

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

r+ conditional to what Alicia thinks of this

> Source/WebCore/platform/graphics/gstreamer/mse/SourceBufferPrivateGStreamer.cpp:196
> +    SourceBufferPrivate::appendCompleted(true, m_mediaSource ? m_mediaSource->isEnded() : true);

Is it necessary to prepend the superclass name? Method is protected and should be available in the subclass, right?

> Source/WebCore/platform/graphics/gstreamer/mse/SourceBufferPrivateGStreamer.cpp:201
> +    SourceBufferPrivate::appendCompleted(false, m_mediaSource ? m_mediaSource->isEnded() : true);

Ditto.

[Alicia Boya García]

Comment on attachment 416660 [details]
Patch

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

LGTM

>> Source/WebCore/platform/graphics/gstreamer/mse/SourceBufferPrivateGStreamer.cpp:196
>> +    SourceBufferPrivate::appendCompleted(true, m_mediaSource ? m_mediaSource->isEnded() : true);
> 
> Is it necessary to prepend the superclass name? Method is protected and should be available in the subclass, right?

It's not necessary, but I appreciate it, since it shows explicitly that we're jumping to the multiplatform code. The refactor mushed together SourceBufferPrivateGStreamer and SourceBufferPrivate and has since made noticing this difference harder.

[EWS]

Committed r271056: <https://trac.webkit.org/changeset/271056>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 416660 [details].

[Radar WebKit Bug Importer]

<rdar://problem/72592069>