[Webkit-unassigned] [Bug 69835] [GStreamer] WebAudio AudioDestination

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=69835





--- Comment #25 from Philippe Normand <pnormand at igalia.com>  2011-11-21 08:24:24 PST ---
(From update of attachment 115118)
View in context: https://bugs.webkit.org/attachment.cgi?id=115118&action=review

>> Source/WebCore/platform/audio/gstreamer/AudioDestinationGStreamer.cpp:91
>> +void AudioDestinationGStreamer::plugSink(GstPad* pad)
> 
> I think this method deserves a more interesting name. plugSink is pretty generic. How about finishBuildingPipelineAfterWavParserPadReady or something similar?

Sure!

>> Source/WebCore/platform/audio/gstreamer/AudioDestinationGStreamer.cpp:95
>> +    GstElement* audiosink = gst_element_factory_make("autoaudiosink", 0);
> 
> RefPtr here?

Doesn't work :( I need to unref only if the element is not added to the bin.

>> Source/WebCore/platform/audio/gstreamer/AudioDestinationGStreamer.cpp:121

> 
> You are leaking sinkPad here because you didn't call adoptGRef.

:( Fixed!

>> Source/WebCore/platform/audio/gstreamer/WebKitWebAudioSourceGStreamer.cpp:57
>> +    GstTask* task;
> 
> GRefPtr here?

Ok

>> Source/WebCore/platform/audio/gstreamer/WebKitWebAudioSourceGStreamer.cpp:207
>> +    priv->wavEncoder = gst_element_factory_make("wavenc", 0);
> 
> Do these have floating references or full references? If full references you need to call adoptGRef here.

Well, full references afaik, so trying to use adoptGRef here, the destructor is called immediately :(

#0  WTF::derefGPtr<_GstElement> (ptr=0x8077260) at ../../Source/WebCore/platform/graphics/gstreamer/GRefPtrGStreamer.cpp:42
#1  0xb6af957d in WTF::GRefPtr<_GstElement>::~GRefPtr (this=0xbfffd75c, __in_chrg=<optimized out>)
    at ../../Source/JavaScriptCore/wtf/gobject/GRefPtr.h:72
#2  0xb6af851d in webKitWebAudioSrcConstructed (object=0x84c7150)
    at ../../Source/WebCore/platform/audio/gstreamer/WebKitWebAudioSourceGStreamer.cpp:206

Not sure what's going on here :( So the GstElement is unreffed and destructed even before we get a chance to add it to the bin...

>> Source/WebCore/platform/audio/gstreamer/WebKitWebAudioSourceGStreamer.cpp:243
>> +        GRefPtr<GstPad> sinkPad(gst_element_get_request_pad(priv->interleave.get(), "sink%d"));
> 
> You are leadking the pads here because of missing adoptGRef.

Right :/ Fixed

>> Source/WebCore/platform/audio/gstreamer/WebKitWebAudioSourceGStreamer.cpp:335

> 
> Hrm. Isn't there any way to have a GstBuffer point to some static memory that it won't free when it finishes? It seems this is possible if you don't use mallocdata. If so, you can avoid a memcpy here. Maybe it even makes sense to have one large buffer and create sub GstBuffers here. That would avoid fragmentation, at least.

A "large buffer"? How large would it be given that it's possible for the playback pipeline to run as long as your browser keeps a WebAudio-enabled page open?

I'm not sure it's possible to have a GstBuffer point to static memory. Another thing that's not possible here (afaik) is using gst_pad_alloc which requires srcpads while we need to chain to queue's sink pad. :/

>> Source/WebCore/platform/audio/gstreamer/WebKitWebAudioSourceGStreamer.cpp:336
>> +        GstCaps* monoCaps = getGStreamerMonoAudioCaps(priv->sampleRate);
> 
> This should be GRefPtr<GstCaps> monoCaps = adoptGRef(getGStreamerMonoAudioCaps(priv->samleRate));

I tried that already and it didn't work out, the gst_structure_set below failing.

>> Source/WebCore/platform/audio/gstreamer/WebKitWebAudioSourceGStreamer.h:26
>> +#define WEBKIT_WEB_AUDIO_SRC(obj) (G_TYPE_CHECK_INSTANCE_CAST((obj), WEBKIT_TYPE_WEB_AUDIO_SRC, WebKitWebAudioSrc))
> 
> Nit: Extra spaces  after macro names.

I'm sorry but I see nothing wrong here! :/ There's only one space between the macro name and its definition.

#define WEBKIT_WEB_AUDIO_SRC(obj)(G_TYPE_CHECK_INSTANCE_CAST((obj), WEBKIT_TYPE_WEB_AUDIO_SRC, WebKitWebAudioSrc))

obviously won't work.

>> Source/WebCore/platform/audio/gstreamer/WebKitWebAudioSourceGStreamer.h:28
>> +typedef struct _WebKitWebAudioSrc        WebKitWebAudioSrc;
> 
> no need to align this.

Right.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.