Bug 115355 – [gstreamer] Prevent race when pad caps is set on gstreamer player

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

[Andre Moreira Magalhaes]

Prevent race when pad caps is set on gstreamer player.

Patch to follow.

[Andre Moreira Magalhaes]

Created attachment 200012 [details]
Patch

[Philippe Normand]

Comment on attachment 200012 [details]
Patch

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

> Source/WebCore/ChangeLog:12
> +        No new tests (OOPS!).

Commit queue will complain here. Please remove this line. Also the entry format is wrong. Should be something like

bug title
bug link

Reviewed by...

Change description

[Andre Moreira Magalhaes]

Created attachment 202281 [details]
Patch

> > Source/WebCore/ChangeLog:12
> > +        No new tests (OOPS!).
> 
> Commit queue will complain here. Please remove this line. Also the entry format is wrong. Should be something like
> 
> bug title
> bug link
> 
> Reviewed by...
> 
> Change description
Done

[Andre Moreira Magalhaes]

Thanks all for the review, all patches are updated now with comments applied.

[Philippe Normand]

Comment on attachment 202281 [details]
Patch

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

> Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.h:30
> +#include <glib.h>

Too bad enums can't be forward-declared, it would have been good to avoid this include.

[WebKit Commit Bot]

Comment on attachment 202281 [details]
Patch

Rejecting attachment 202281 [details] from commit-queue.

Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.appspot.com', '--bot-id=webkit-cq-01', 'apply-attachment', '--no-update', '--non-interactive', 202281, '--port=mac']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit

Last 500 characters of output:
383 with fuzz 1 (offset 51 lines).
1 out of 4 hunks FAILED -- saving rejects to file Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp.rej
patching file Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.h
Hunk #1 succeeded at 27 with fuzz 1.
Hunk #2 succeeded at 123 (offset 14 lines).

Failed to run "[u'/Volumes/Data/EWS/WebKit/Tools/Scripts/svn-apply', '--force', '--reviewer', u'Philippe Normand']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit

Full output: http://webkit-queues.appspot.com/results/664222

[Philippe Normand]

Can you please rebase the patch with the "Reviewed by" field updated and mark it cq? ? Thanks :)

[Andre Moreira Magalhaes]

Created attachment 203709 [details]
Patch

(In reply to comment #7)
> Can you please rebase the patch with the "Reviewed by" field updated and mark it cq? ? Thanks :)

Done, rebased patch attached.

[WebKit Commit Bot]

Comment on attachment 203709 [details]
Patch

Clearing flags on attachment: 203709

Committed r151175: <http://trac.webkit.org/changeset/151175>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Csaba Osztrogonác]

It broke the build with gstreamer 0.1 (Qt WK2 perf bot and Qt MIPS bot):

/data/buildbot/mips-1/qt-linux-mipsel-mips32r2-release/build/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp: In member function 'virtual WebCore::IntSize WebCore::MediaPlayerPrivateGStreamerBase::naturalSize() const':
/data/buildbot/mips-1/qt-linux-mipsel-mips32r2-release/build/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp:171:32: error: invalid conversion from 'const GMutex* {aka const _GMutex*}' to 'GMutex* {aka _GMutex*}' [-fpermissive]


/usr/mipsel-linux-gnu/include/glib-2.0/glib/gthread.h:157:17: error:   initializing argument 1 of 'void g_mutex_lock(GMutex*)' [-fpermissive]
/data/buildbot/mips-1/qt-linux-mipsel-mips32r2-release/build/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp:173:34: error: invalid conversion from 'const GMutex* {aka const _GMutex*}' to 'GMutex* {aka _GMutex*}' [-fpermissive]


Isn't gstreamer 1.0 mandatory long time ago? If no, it needs buildfix.
If yes, Qt build system should be fixed to require gstreamer 1.0 installed.
( I mean removing the 0.1 fallback introduced in https://trac.webkit.org/changeset/144208 )

[Philippe Normand]

Qt should switch to gst 1.x, there's a bug about that, but this is not my call :)

Someone should indeed fix this build error, we need gst 0.10 support building. Why did that patch pass on the qt EWS?

[Csaba Osztrogonác]

(In reply to comment #12)
> Qt should switch to gst 1.x, there's a bug about that, but this is not my call :)
> 
> Someone should indeed fix this build error, we need gst 0.10 support building. Why did that patch pass on the qt EWS?

All Qt bots (EWS bots too) already use gst 1.0 (from the related 
thread on webkit-dev mailing list), but the MIPS and WK2 perf bot.

[Philippe Normand]

https://trac.webkit.org/r151203

[Balazs Kelemen]

Could you provide some information about why did you only fixed it for the old version? Thanks.