Bug 178074 - [MSE][GStreamer] Add dump of append pipeline
Summary: [MSE][GStreamer] Add dump of append pipeline
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-10-09 02:20 PDT by Alicia Boya García
Modified: 2017-10-11 02:33 PDT (History)
5 users (show)

See Also:


Attachments
Patch (4.05 KB, patch)
2017-10-09 02:45 PDT, Alicia Boya García
no flags Details | Formatted Diff | Diff
Patch (4.05 KB, patch)
2017-10-09 03:32 PDT, Alicia Boya García
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alicia Boya García 2017-10-09 02:20:12 PDT
Pipeline dumps are very useful for debugging. Right now we have one for
PlaybackPipeline but none for AppendPipeline.
Comment 1 Alicia Boya García 2017-10-09 02:45:48 PDT
Created attachment 323164 [details]
Patch
Comment 2 Enrique Ocaña 2017-10-09 03:16:09 PDT
Comment on attachment 323164 [details]
Patch

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

> Source/WebCore/ChangeLog:3
> +        [GTK][MSE] Add dump of append pipeline

The whole patch looks very good. I would just rename the bug to "[MSE][GStreamer] ..." because it applies to all GStreamer ports (GTK, WPE or any other in the future).
Comment 3 Xabier Rodríguez Calvar 2017-10-09 03:23:41 PDT
Comment on attachment 323164 [details]
Patch

I agree with Enrique. This said, the patch is good enough and we can land it provided that you create a bug (and fix if if you can) to rework the way we subscribe and unsubscribe to signals in this and other pipelines.

First. we should subscribe with lambdas.

Second, we should unsubscribe with the by_data function instead of the by_func one. That way, as we always pass this as user_data, we'll always unsubscribe from stuff even when we haven't done it explicitly for all functions (and we won't need declared functions, lambdas will suffice).
Comment 4 Alicia Boya García 2017-10-09 03:32:32 PDT
Created attachment 323165 [details]
Patch
Comment 5 Xabier Rodríguez Calvar 2017-10-09 04:12:00 PDT
Comment on attachment 323165 [details]
Patch

By request of Alicia, we set cq- because she can't check the landing now.
Comment 6 WebKit Commit Bot 2017-10-11 02:33:49 PDT
Comment on attachment 323165 [details]
Patch

Clearing flags on attachment: 323165

Committed r223172: <http://trac.webkit.org/changeset/223172>
Comment 7 WebKit Commit Bot 2017-10-11 02:33:51 PDT
All reviewed patches have been landed.  Closing bug.