Bug 184498

Summary: [GStreamer] Use of playbin3 when USE_PLAYBIN3 environment variable is set
Product: WebKit Reporter: Thibault Saunier <tsaunier>
Component: New BugsAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, mcatanzaro, pnormand, tsaunier, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch none

Thibault Saunier
Reported 2018-04-11 07:30:11 PDT
[GStreamer] Use of playbin3 when USE_PLAYBIN3 environment variable is set
Attachments
Patch (18.34 KB, patch)
2018-04-11 07:31 PDT, Thibault Saunier 		no flags
DetailsFormatted DiffDiff
Patch (18.97 KB, patch)
2018-04-11 10:57 PDT, Thibault Saunier 		no flags
DetailsFormatted DiffDiff
Patch (18.97 KB, patch)
2018-04-11 10:59 PDT, Thibault Saunier 		no flags
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Thibault Saunier
Comment 1 2018-04-11 07:31:30 PDT
Created attachment 337699 [details] Patch
Philippe Normand
Comment 2 2018-04-11 07:45:03 PDT
Comment on attachment 337699 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=337699&action=review > Source/WebCore/platform/graphics/gstreamer/GRefPtrGStreamer.h:24 > +#include <gst/gst.h> This is for GST_CHECK_VERSION(), right? Then please remove the now useless forward typedefs :) > Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:2402 > +void MediaPlayerPrivateGStreamer::createGSTPlayBin(const gchar *playbin_name) Please use camelCase :P
Philippe Normand
Comment 3 2018-04-11 07:48:49 PDT
Comment on attachment 337699 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=337699&action=review >> Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:2402 >> +void MediaPlayerPrivateGStreamer::createGSTPlayBin(const gchar *playbin_name) > > Please use camelCase :P Ah also, we use char instead of gchar usually, and the star is mispositioned. Not sure why check-webkit-style didn't complain there...
Thibault Saunier
Comment 4 2018-04-11 10:57:49 PDT
Created attachment 337714 [details] Patch
Thibault Saunier
Comment 5 2018-04-11 10:59:10 PDT
Created attachment 337715 [details] Patch
Thibault Saunier
Comment 6 2018-04-11 11:01:03 PDT
> Ah also, we use char instead of gchar usually, and the star is mispositioned. Not sure why check-webkit-style didn't complain there... From my small checks, we do using g<ctypes> quite a lot all around :-) Looks like the star positioning is not enforce anywhere in the webkit style (clang-format doesn't seem to care, and the script doesn't either)
Thibault Saunier
Comment 7 2018-04-11 11:01:28 PDT
(I fixed it all anyway)
WebKit Commit Bot
Comment 8 2018-04-11 11:33:44 PDT
Comment on attachment 337715 [details] Patch Clearing flags on attachment: 337715 Committed r230540: <https://trac.webkit.org/changeset/230540>
WebKit Commit Bot
Comment 9 2018-04-11 11:33:45 PDT
All reviewed patches have been landed. Closing bug.
Radar WebKit Bug Importer
Comment 10 2018-04-11 11:34:21 PDT
<rdar://problem/39354946>
Michael Catanzaro
Comment 11 2019-04-16 07:36:35 PDT
Please, environment variables need to be documented at https://trac.webkit.org/wiki/EnvironmentVariables
Note You need to log in before you can comment on or make changes to this bug.