Bug 162898 - [GStreamer][MSE] SourceBufferPrivateGStreamer refactoring
Summary: [GStreamer][MSE] SourceBufferPrivateGStreamer refactoring
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Platform (show other bugs)
Version: WebKit Local Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Enrique Ocaña
URL:
Keywords:
Depends on:
Blocks: 157314
  Show dependency treegraph
 
Reported: 2016-10-04 04:06 PDT by Enrique Ocaña
Modified: 2016-10-26 01:43 PDT (History)
1 user (show)

See Also:


Attachments
Patch (21.61 KB, patch)
2016-10-04 04:13 PDT, Enrique Ocaña
no flags Details | Formatted Diff | Diff
Patch (21.55 KB, patch)
2016-10-16 12:05 PDT, Enrique Ocaña
no flags Details | Formatted Diff | Diff
Patch (21.25 KB, patch)
2016-10-21 14:25 PDT, Enrique Ocaña
no flags Details | Formatted Diff | Diff
Patch (21.10 KB, patch)
2016-10-25 11:03 PDT, Enrique Ocaña
no flags Details | Formatted Diff | Diff
Patch (21.10 KB, patch)
2016-10-26 01:20 PDT, Enrique Ocañ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 Enrique Ocaña 2016-10-04 04:06:51 PDT
Move SourceBufferPrivateGStreamer to an mse directory and add unimplemented features (abort, enqueueing, sample flow control).
Comment 1 Enrique Ocaña 2016-10-04 04:13:00 PDT
Created attachment 290590 [details]
Patch
Comment 2 Enrique Ocaña 2016-10-04 07:07:25 PDT
Comment on attachment 290590 [details]
Patch

Wait until all the patches in 157314 are ready.
Comment 3 Xabier Rodríguez Calvar 2016-10-08 04:44:33 PDT
Comment on attachment 290590 [details]
Patch

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

> Source/WebCore/platform/graphics/gstreamer/mse/SourceBufferPrivateGStreamer.h:89
> +    MediaSourceGStreamer* m_mediaSource;
> +    ContentType m_type;
> +    RefPtr<MediaSourceClientGStreamerMSE> m_client;
> +    SourceBufferPrivateClient* m_sourceBufferPrivateClient;

Let's add a comment here and create bug later to study making these plain pointers RefPtr or similar.
Comment 4 Xabier Rodríguez Calvar 2016-10-08 05:15:09 PDT
(In reply to comment #3)
> Let's add a comment here and create bug later to study making these plain
> pointers RefPtr or similar.

After talking to Quique (Enrique) in person, we saw that there are cross references in place and we don't want to mess this up.
Comment 5 Enrique Ocaña 2016-10-16 12:05:21 PDT
Created attachment 291761 [details]
Patch
Comment 6 Zan Dobersek 2016-10-21 00:06:06 PDT
Comment on attachment 291761 [details]
Patch

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

> Source/WebCore/platform/graphics/gstreamer/mse/SourceBufferPrivateGStreamer.cpp:60
> +    , m_client(client)

It seems to be assumed that m_client is non-null throughout the lifetime of SourceBufferPrivateGStreamer, at least how it's used in append() and because it's not nulled out anywhere.

If that's the case, the parameter to this constructor and the member variables should be Ref<>, and there's no need to null-check it anywhere.

> Source/WebCore/platform/graphics/gstreamer/mse/SourceBufferPrivateGStreamer.cpp:62
> +    , m_isReadyForMoreSamples(true)
> +    , m_notifyWhenReadyForMoreSamples(false)

These can be initialized in the constructor.

> Source/WebCore/platform/graphics/gstreamer/mse/SourceBufferPrivateGStreamer.cpp:68
> +SourceBufferPrivateGStreamer::~SourceBufferPrivateGStreamer()
> +{
> +}

Default it.
Comment 7 Enrique Ocaña 2016-10-21 14:25:32 PDT
Created attachment 292407 [details]
Patch
Comment 8 Enrique Ocaña 2016-10-25 11:03:52 PDT
Created attachment 292780 [details]
Patch
Comment 9 Enrique Ocaña 2016-10-26 01:20:08 PDT
Created attachment 292892 [details]
Patch
Comment 10 Enrique Ocaña 2016-10-26 01:43:27 PDT
Comment on attachment 292892 [details]
Patch

Clearing flags on attachment: 292892

Committed r207879: <http://trac.webkit.org/changeset/207879>
Comment 11 Enrique Ocaña 2016-10-26 01:43:35 PDT
All reviewed patches have been landed.  Closing bug.