Passing a non-GObject object to GST_TRACE_OBJECT() can be theoretically misunderstood by the GStreamer logging function, so this patch avoids that.
Created attachment 350977 [details] Patch
Comment on attachment 350977 [details] Patch Should you also give the pipeline a proper name?
(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?
mse-append-pipeline-n where n is a monotically increasing number?
(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.
Created attachment 351067 [details] Patch
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.
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.
(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.
Comment on attachment 351067 [details] Patch Clearing flags on attachment: 351067 Committed r236640: <https://trac.webkit.org/changeset/236640>
All reviewed patches have been landed. Closing bug.