Bug 113880 – [Gstreamer] Use gst_buffer_extract() in copyGstreamerBuffersToAudioChannel()

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Chris Dumez]

copyGstreamerBuffersToAudioChannel() currently maps the GstBuffer content to memcpy it the the AudioChannel buffer. We could leverage gst_buffer_extract() API function to simplify the code and avoid issues with error handling.

[Chris Dumez]

Created attachment 196362 [details]
Patch

[Philippe Normand]

Comment on attachment 196362 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=196362&action=review

Looks cool indeed!

> Source/WebCore/platform/audio/gstreamer/AudioFileReaderGStreamer.cpp:105
> +    guint bufferCount = gst_buffer_list_length(buffers);
> +    for (guint i = 0; i < bufferCount; ++i) {

bufferCount is not needed I think. Also please use unsigned instead of guint if possible, we try to give preference to non-glib types when possible.

[Chris Dumez]

Comment on attachment 196362 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=196362&action=review

>> Source/WebCore/platform/audio/gstreamer/AudioFileReaderGStreamer.cpp:105
>> +    for (guint i = 0; i < bufferCount; ++i) {
> 
> bufferCount is not needed I think. Also please use unsigned instead of guint if possible, we try to give preference to non-glib types when possible.

Caching the container size before the loop is commonly done in WebKit and it avoids calling gst_buffer_list_length() for each iteration. It's not a big deal here because gst_buffer_list_length() is inexpensive but I still think it is good practice.

I'll replace guint by unsigned, I wasn't sure what the policy was about this.

[Chris Dumez]

Created attachment 196374 [details]
Patch

Using unsigned instead of guint. I kept the container size caching for now.

[WebKit Review Bot]

Comment on attachment 196374 [details]
Patch

Clearing flags on attachment: 196374

Committed r147567: <http://trac.webkit.org/changeset/147567>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.