Bug 190045 – [GStreamer][MSE] Use GObject for GST_TRACE_OBJECT

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKitGTK

[Alicia Boya García]

Passing a non-GObject object to GST_TRACE_OBJECT() can be
theoretically misunderstood by the GStreamer logging function, so this
patch avoids that.

[Alicia Boya García]

Created attachment 350977 [details]
Patch

[Thibault Saunier]

Comment on attachment 350977 [details]
Patch

Should you also give the pipeline a proper name?

[Alicia Boya García]

(In reply to Thibault Saunier from comment #2)
> Comment on attachment 350977 [details]
> Patch
> 
> Should you also give the pipeline a proper name?

Any suggestion for a useful name?

[Philippe Normand]

mse-append-pipeline-n where n is a monotically increasing number?

[Alicia Boya García]

(In reply to Philippe Normand from comment #4)
> mse-append-pipeline-n where n is a monotically increasing number?

I've chosen to use "append-pipeline-<type>-<container>-<number>", e.g.: append-pipeline-audio-mp4-0; as it's usually useful to know which pipeline handles what thing.

[Alicia Boya García]

Created attachment 351067 [details]
Patch

[Thibault Saunier]

Comment on attachment 351067 [details]
Patch

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

lgtm otherwise.

> Source/WebCore/platform/graphics/gstreamer/mse/AppendPipeline.cpp:144
> +        m_sourceBufferPrivate->type().containerType().replace("/", "-").utf8().data(), appendPipelineCount++);

Increment should probably be atomic.

[Alicia Boya García]

Comment on attachment 351067 [details]
Patch

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

>> Source/WebCore/platform/graphics/gstreamer/mse/AppendPipeline.cpp:144
>> +        m_sourceBufferPrivate->type().containerType().replace("/", "-").utf8().data(), appendPipelineCount++);
> 
> Increment should probably be atomic.

AppendPipeline can only be constructed in the main thread. There is even an assertion for that. No need for atomics here.

[Thibault Saunier]

(In reply to Alicia Boya García from comment #8)
> Comment on attachment 351067 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=351067&action=review
> 
> >> Source/WebCore/platform/graphics/gstreamer/mse/AppendPipeline.cpp:144
> >> +        m_sourceBufferPrivate->type().containerType().replace("/", "-").utf8().data(), appendPipelineCount++);
> > 
> > Increment should probably be atomic.
> 
> AppendPipeline can only be constructed in the main thread. There is even an
> assertion for that. No need for atomics here.

OK, informal r+ here then.

[WebKit Commit Bot]

Comment on attachment 351067 [details]
Patch

Clearing flags on attachment: 351067

Committed r236640: <https://trac.webkit.org/changeset/236640>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.