Bug 188646 - [GStreamer][MSE] Remove parsers from playback pipeline
Summary: [GStreamer][MSE] Remove parsers from playback pipeline
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Platform (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Philippe Normand
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2018-08-16 06:20 PDT by Philippe Normand
Modified: 2018-08-21 01:50 PDT (History)
5 users (show)

See Also:


Attachments
Patch (8.89 KB, patch)
2018-08-16 06:23 PDT, Philippe Normand
calvaris: review+
ews-watchlist: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from ews202 for win-future (12.83 MB, application/zip)
2018-08-16 19:08 PDT, EWS Watchlist
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Philippe Normand 2018-08-16 06:20:20 PDT
Decodebin already includes parsers in front of the decoders.
Comment 1 Philippe Normand 2018-08-16 06:23:06 PDT
Created attachment 347257 [details]
Patch
Comment 2 EWS Watchlist 2018-08-16 19:08:48 PDT
Comment on attachment 347257 [details]
Patch

Attachment 347257 [details] did not pass win-ews (win):
Output: https://webkit-queues.webkit.org/results/8887030

New failing tests:
http/tests/security/local-video-source-from-remote.html
Comment 3 EWS Watchlist 2018-08-16 19:08:59 PDT
Created attachment 347333 [details]
Archive of layout-test-results from ews202 for win-future

The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews202  Port: win-future  Platform: CYGWIN_NT-6.1-2.10.0-0.325-5-3-x86_64-64bit
Comment 4 Xabier Rodríguez Calvar 2018-08-17 01:40:23 PDT
Comment on attachment 347257 [details]
Patch

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

I would like Enrique or Alicia to comment on this before landing (after fixing the comments)

> Source/WebCore/platform/graphics/gstreamer/mse/PlaybackPipeline.cpp:178
>      const char* mediaType = capsMediaType(caps);
>      GST_DEBUG_OBJECT(webKitMediaSrc, "Configured track %s: appsrc=%s, padId=%u, mediaType=%s", trackPrivate->id().string().utf8().data(), GST_ELEMENT_NAME(stream->appsrc), padId, mediaType);

mediaType would be now used nowhere else, so variable can be removed and result passed directly to GST_DEBUG_OBJECT.

> Source/WebCore/platform/graphics/gstreamer/mse/WebKitMediaSourceGStreamer.cpp:496
>      if (!caps || !stream->parent) {

We need to fix the GST_ERROR bellow this line or it would be misleading :)
Comment 5 Enrique Ocaña 2018-08-17 03:27:04 PDT
Looks fine. Aniway, I'd give a run to the YouTube MSE tests[1] to look for regressions.

[1] http://yt-dash-mse-test.commondatastorage.googleapis.com/unit-tests/2018.html
Comment 6 Philippe Normand 2018-08-17 09:09:38 PDT
(In reply to Enrique Ocaña from comment #5)
> Looks fine. Aniway, I'd give a run to the YouTube MSE tests[1] to look for
> regressions.
> 
> [1]
> http://yt-dash-mse-test.commondatastorage.googleapis.com/unit-tests/2018.html

Do you mean with downstream? If so, can you check it?
With trunk, even without this patch there's quite a few timeouts in the 2018 test-suite, #18, #20, 21, 22, 23, 30, 35, 36, 38, 39, 40, 41, 42 and then I had to stop because this takes ages :)
Comment 7 Alicia Boya García 2018-08-20 03:48:41 PDT
LGTM.
Comment 8 Philippe Normand 2018-08-21 01:49:41 PDT
Committed r235109: <https://trac.webkit.org/changeset/235109>
Comment 9 Radar WebKit Bug Importer 2018-08-21 01:50:24 PDT
<rdar://problem/43550849>