[Webkit-unassigned] [Bug 178076] [MSE][GStreamer] Insert parser elements in AppendPipeline when demuxing opus or Vorbis
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Fri Oct 13 03:39:16 PDT 2017
https://bugs.webkit.org/show_bug.cgi?id=178076
--- Comment #4 from Alicia Boya GarcĂa <aboya at igalia.com> ---
Comment on attachment 323400
--> https://bugs.webkit.org/attachment.cgi?id=323400
Patch
View in context: https://bugs.webkit.org/attachment.cgi?id=323400&action=review
>> Source/WebCore/platform/graphics/gstreamer/mse/AppendPipeline.cpp:955
>> + GRefPtr<GstCaps> padCaps = adoptGRef(gst_pad_get_current_caps(GST_PAD(demuxerSrcPad)));
>
> Is this GST_PAD() macro actually needed?
Not really, I should remove it.
>> Source/WebCore/platform/graphics/gstreamer/mse/AppendPipeline.cpp:965
>> + return opusparse;
>
> Just a comment for future reference without any action required on your side: opusparse is a floating reference. The return type is GRefPtr. This means that there's going to be a GRefPtr(opusparse) call under the hood which will call g_object_ref_sink() and convert the floating ref into a regular ref owned by the GRefPtr.
That is intended in this case, though I could be more explicit, right.
>> Source/WebCore/platform/graphics/gstreamer/mse/AppendPipeline.cpp:1034
>> + // This is known to be an issue with YouTube webm files containing Opus audio as of YT2018.
>
> YouTube/tv (isn't the same as YouTube)
Regular YouTube also uses Opus, I would be surprised if the files were different in regard to durations in the container.
>> Source/WebCore/platform/graphics/gstreamer/mse/AppendPipeline.cpp:-1094
>> - // Must be done in the thread we were called from (usually streaming thread).
>
> What happened to the black hole probe?
The condition in the `if` block was always false because by the time `pad-removed` is emitted the pad is already unlinked.
I'm not completely sure about probes lifecycle but I think they are removed automatically with the pad when the pad refcount hits zero (which should be soon after the `pad-removed` handler finishes unless it's re-linked there).
>> Source/WebCore/platform/graphics/gstreamer/mse/AppendPipeline.cpp:-1111
>> - gst_element_unlink(m_demux.get(), m_decryptor.get());
>
> No unlinking now? Why?
Removing the elements from the bin automatically unlinks them. It's easier to remove a series of optional elements than to remove links between a chain of optional elements then also removing the elements.
--
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20171013/ecae5ed1/attachment.html>
More information about the webkit-unassigned
mailing list