[Webkit-unassigned] [Bug 78883] [GStreamer] AudioSourceProvider support in the MediaPlayer

[bugzilla-daemon at webkit.org]

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





--- Comment #32 from Philippe Normand <pnormand at igalia.com>  2013-01-26 04:26:44 PST ---
(In reply to comment #31)
> (From update of attachment 183670 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=183670&action=review
> 
> Seems sane to me, though to be honest, I don't understand this code very well. Does this allow us to unskip any tests?
> 

There is no WAV test for this feature yet, only a test that checks a mediaelementsourcenode can be created.
I'll attach a png dump of the pipeline and comment the code a bit more.

> > Source/WebCore/platform/audio/gstreamer/AudioSourceProviderGStreamer.cpp:35
> > +#include <wtf/UnusedParam.h>
> 
> This should go with the first block of includes.
> 

Like I said in comment 26, the style checker says no :)

> > Source/WebCore/platform/audio/gstreamer/AudioSourceProviderGStreamer.cpp:179
> > +    GRefPtr<GstCaps> caps = gst_buffer_get_caps(buffer);
> 
> Does gst_buffer_get_caps return a floating reference? If not, I think this might be a memory leak unless you use adoptGPtr here.
> 

It doesn't return a floating ref, I'll add adopt!

> > Source/WebCore/platform/audio/gstreamer/AudioSourceProviderGStreamer.cpp:194
> > +    case GST_AUDIO_CHANNEL_POSITION_FRONT_LEFT:
> > +        gst_buffer_list_iterator_add(m_frontLeftBuffersIterator, buffer);
> > +        break;
> > +    case GST_AUDIO_CHANNEL_POSITION_FRONT_RIGHT:
> > +        gst_buffer_list_iterator_add(m_frontRightBuffersIterator, buffer);
> > +        break;
> > +    default:
> > +        gst_buffer_unref(buffer);
> > +        break;
> 
> Is it correct that you only unref the buffer if you get a channel that isn't left/right, or is this supposed to be a fallthrough? If you are always supposed to unref it, perhaps this should just be a GRefPtr. :)
> 

It's a fallthrough, gst_buffer_list_iterator_add takes ownership of the buffer, so no need to unref it in those cases.

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