[webkit-reviews] review denied: [Bug 50382] [GStreamer] hasVideo/Audio return false until the pipeline reaches PAUSED : [Attachment 75377] proposed patch

[bugzilla-daemon at webkit.org]

Martin Robinson <mrobinson at webkit.org> has denied Philippe Normand
<pnormand at igalia.com>'s request for review:
Bug 50382: [GStreamer] hasVideo/Audio return false until the pipeline reaches
PAUSED
https://bugs.webkit.org/show_bug.cgi?id=50382

Attachment 75377: proposed patch
https://bugs.webkit.org/attachment.cgi?id=75377&action=review

------- Additional Comments from Martin Robinson <mrobinson at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=75377&action=review

Seems quite reasonable. I think it needs just a couple fixes.

> WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:208
> +void mediaPlayerPrivateVideoTagsChangedCallback(GObject* element, gint
streamId, gpointer data)

You can just make this void mediaPlayerPrivateVideoTagsChangedCallback(GObject*
element, gint streamId, MediaPlayerPrivateGStreamer* player)

> WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:214
> +void mediaPlayerPrivateAudioTagsChangedCallback(GObject* element, gint
streamId, gpointer data)

Same here.

> WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:346
> +    , m_hasAudio(false)

Shouldn't m_videoTagsTimerHandler and m_audioTagstimreHandler be initialized to
0 here. To guard touching junk data in the destructor?