Comment 32 for bug 1203240

Revision history for this message
In , Cgarcia-f (cgarcia-f) wrote :

Comment on attachment 300398
Patch

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

> Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:40
> +#include <WebCore/FileSystem.h>

Don't use <> since you are in WebCore.

> Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:203
> + if (m_source && WEBKIT_IS_WEB_SRC(m_source.get()))

G_TYPE_CHECK_INSTANCE_TYPE already null checks the pointer.

> Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:204
> + g_signal_handlers_disconnect_by_func(GST_ELEMENT_PARENT(m_source.get()), reinterpret_cast<gpointer>(uriDecodeBinElementAddedCallback), this);

What if it has been disconnected before? I think it would be better to save the handler id and use g_signal_handler_disconnect() and invalidate the id.

> Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:1350
> + if (g_strcmp0(G_OBJECT_CLASS_NAME(G_OBJECT_GET_CLASS(G_OBJECT(element))), "GstDownloadBuffer"))

GST_IS_DOWNLOAD_BUFFER?

> Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:1395
> + // Transform "-XXXXXX" into "-??????" to get a file wildcard expression from the template.
> + for (char* p = &templateFile.get()[strlen(templateFile.get()) - 1]; p >= templateFile.get() && *p == 'X'; --p)
> + *p = '?';

I'm not sure I understand this, maybe you can simply do g_strdelimit (templateFile.get(), "X", '?');

Can't you pass something like "*-XXXXXX*" as pattern to listDirectory?

> Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:1397
> + for (auto filePath : listDirectory(templatePath.get(), templateFile.get())) {

auto&

> Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:1409
> + if (m_source && WEBKIT_IS_WEB_SRC(m_source.get()))

G_TYPE_CHECK_INSTANCE_TYPE already null checks the pointer.