[GStreamer][MSE] Add GstFlowCombiner to handle non-linked inactive branches
Created attachment 344930 [details] Patch
This patch looks good to me. It's going to be very helpful to support more than one audio streams.
Comment on attachment 344930 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=344930&action=review > Source/WebCore/platform/graphics/gstreamer/GUniquePtrGStreamer.h:36 > +// GstFlowCombiner becomes reference counted in 1.12. What does this comment mean? It's confusing, if it's ref counted, why we define a GUniquePtr? > Source/WebCore/platform/graphics/gstreamer/mse/WebKitMediaSourceGStreamer.cpp:255 > + return gst_flow_combiner_update_pad_flow(self.get()->priv->flowCombiner.get(), pad, gst_proxy_pad_chain_default(pad, GST_OBJECT(self.get()), buffer)); self.get()->priv -> self->priv > Source/WebCore/platform/graphics/gstreamer/mse/WebKitMediaSourceGStreamer.cpp:482 > - GUniquePtr<gchar> padName(g_strdup_printf("src_%u", padId)); > - GstPad* ghostpad = WebCore::webkitGstGhostPadFromStaticTemplate(&srcTemplate, padName.get(), sourcePad); > + auto padName = String::format("src_%u", padId); > + GstPad* ghostPad = WebCore::webkitGstGhostPadFromStaticTemplate(&srcTemplate, padName.utf8().data(), sourcePad); Why did you change this? We are now doing encoding conversions.
(In reply to Carlos Garcia Campos from comment #3) > > Source/WebCore/platform/graphics/gstreamer/GUniquePtrGStreamer.h:36 > > +// GstFlowCombiner becomes reference counted in 1.12. > > What does this comment mean? It's confusing, if it's ref counted, why we > define a GUniquePtr? It became refcounted in 1.12 so from that moment on can can ref and unref but with previous versions of GStreamer, we have _new and _free. Since we don't need it to be refcounted for now, I thought the simplest way was implementing the non-refcounted use case. > > Source/WebCore/platform/graphics/gstreamer/mse/WebKitMediaSourceGStreamer.cpp:255 > > + return gst_flow_combiner_update_pad_flow(self.get()->priv->flowCombiner.get(), pad, gst_proxy_pad_chain_default(pad, GST_OBJECT(self.get()), buffer)); > > self.get()->priv -> self->priv Yes (monkey coding). > > Source/WebCore/platform/graphics/gstreamer/mse/WebKitMediaSourceGStreamer.cpp:482 > > - GUniquePtr<gchar> padName(g_strdup_printf("src_%u", padId)); > > - GstPad* ghostpad = WebCore::webkitGstGhostPadFromStaticTemplate(&srcTemplate, padName.get(), sourcePad); > > + auto padName = String::format("src_%u", padId); > > + GstPad* ghostPad = WebCore::webkitGstGhostPadFromStaticTemplate(&srcTemplate, padName.utf8().data(), sourcePad); > > Why did you change this? We are now doing encoding conversions. Cause I think it was better to use WebKit API rather than GLib's if possible. I think it is better. Do you object to this?
(In reply to Xabier Rodríguez Calvar from comment #4) > (In reply to Carlos Garcia Campos from comment #3) > > > Source/WebCore/platform/graphics/gstreamer/GUniquePtrGStreamer.h:36 > > > +// GstFlowCombiner becomes reference counted in 1.12. > > > > What does this comment mean? It's confusing, if it's ref counted, why we > > define a GUniquePtr? > > It became refcounted in 1.12 so from that moment on can can ref and unref > but with previous versions of GStreamer, we have _new and _free. Since we > don't need it to be refcounted for now, I thought the simplest way was > implementing the non-refcounted use case. Remove the comment then, it's confusing. > > > Source/WebCore/platform/graphics/gstreamer/mse/WebKitMediaSourceGStreamer.cpp:255 > > > + return gst_flow_combiner_update_pad_flow(self.get()->priv->flowCombiner.get(), pad, gst_proxy_pad_chain_default(pad, GST_OBJECT(self.get()), buffer)); > > > > self.get()->priv -> self->priv > > Yes (monkey coding). > > > > Source/WebCore/platform/graphics/gstreamer/mse/WebKitMediaSourceGStreamer.cpp:482 > > > - GUniquePtr<gchar> padName(g_strdup_printf("src_%u", padId)); > > > - GstPad* ghostpad = WebCore::webkitGstGhostPadFromStaticTemplate(&srcTemplate, padName.get(), sourcePad); > > > + auto padName = String::format("src_%u", padId); > > > + GstPad* ghostPad = WebCore::webkitGstGhostPadFromStaticTemplate(&srcTemplate, padName.utf8().data(), sourcePad); > > > > Why did you change this? We are now doing encoding conversions. > > Cause I think it was better to use WebKit API rather than GLib's if > possible. I think it is better. Do you object to this? Yes, WTF is preferred but only when it makes sense, and it doesn't in this case. The change is unrelated to this bug too.
Created attachment 344933 [details] Patch
Comment on attachment 344933 [details] Patch Clearing flags on attachment: 344933 Committed r233797: <https://trac.webkit.org/changeset/233797>
All reviewed patches have been landed. Closing bug.
<rdar://problem/42160243>