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

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Jan 24 18:35:24 PST 2013


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





--- Comment #31 from Martin Robinson <mrobinson at webkit.org>  2013-01-24 18:37:15 PST ---
(From update of attachment 183670)
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?

> Source/WebCore/platform/audio/gstreamer/AudioSourceProviderGStreamer.cpp:35
> +#include <wtf/UnusedParam.h>

This should go with the first block of includes.

> 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.

> 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. :)

> Source/WebCore/platform/audio/gstreamer/AudioSourceProviderGStreamer.cpp:309
> +    GstCaps* caps = getGstAudioCaps(1, gSampleBitRate);
> +    gst_app_sink_set_caps(GST_APP_SINK(sink), caps);
> +    gst_caps_unref(caps);

Looks like this could be:

GRefPtr<GstCaps> caps = adoptGRef(getGstAudioCaps(1, gSampleBitRate));
gst_app_sink_set_caps(GST_APP_SINK(sink), caps.get());

> Source/WebCore/platform/audio/gstreamer/AudioSourceProviderGStreamer.h:26
> +#include "AudioSourceProvider.h"
> +
> +#include "GRefPtrGStreamer.h"

Extra newline here.

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



More information about the webkit-unassigned mailing list