Bug 142537 - [GStreamer][GTK] Minimum required version for gst-plugins-base is actually 1.1
Summary: [GStreamer][GTK] Minimum required version for gst-plugins-base is actually 1.1
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Sebastian Dröge (slomo)
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-03-10 11:46 PDT by Carlos Alberto Lopez Perez
Modified: 2015-03-12 12:32 PDT (History)
5 users (show)

See Also:


Attachments
Patch (5.79 KB, patch)
2015-03-12 06:16 PDT, Sebastian Dröge (slomo)
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Carlos Alberto Lopez Perez 2015-03-10 11:46:50 PDT
When finding the GStreamer libraries at Source/cmake/FindGStreamer.cmake the minimum required version for gst-plugins-base is 1.0.

However the file Source/WebCore/platform/audio/gstreamer/WebKitWebAudioSourceGStreamer.cpp includes <gst/app/app.h> which is only available on gst-plugins-base >= 1.1

This include was added on r177085 <http://trac.webkit.org/r177085>

So I guess we either need to raise the minimum version required or put some ifdefs.
Comment 1 Sebastian Dröge (slomo) 2015-03-10 16:33:26 PDT
I'll provide a patch tomorrow to make it work again with 1.0
Comment 2 Sebastian Dröge (slomo) 2015-03-11 09:18:08 PDT
Actually no, we include 1.2+ headers in various places unconditionally. I would recommend to just do it consistently and then require GStreamer >= 1.2.0.

Opinions?

I can prepare a patch for cleaning up the includes and the dependency check.
Comment 3 Carlos Garcia Campos 2015-03-11 09:57:45 PDT
(In reply to comment #2)
> Actually no, we include 1.2+ headers in various places unconditionally. I
> would recommend to just do it consistently and then require GStreamer >=
> 1.2.0.
> 
> Opinions?
> 
> I can prepare a patch for cleaning up the includes and the dependency check.

We have a policy of not bumping minimum requirements unless it's really needed (or until we break the ABI), so we should make sure it build with 1.0.3, see http://trac.webkit.org/wiki/WebKitGTK/Dependencies
Comment 4 Sebastian Dröge (slomo) 2015-03-11 10:05:56 PDT
Ok, I can make a patch but I can't really test it. I have no 1.0.x anywhere anymore. So better if someone else does that
Comment 5 Carlos Alberto Lopez Perez 2015-03-11 18:11:01 PDT
(In reply to comment #4)
> Ok, I can make a patch but I can't really test it. I have no 1.0.x anywhere
> anymore. So better if someone else does that

I have a development environment with 1.0.9, I can test the patch with that version.
Comment 6 Sebastian Dröge (slomo) 2015-03-12 02:40:54 PDT
Awesome, I'll prepare one later today then and make sure it builds with 1.4 at least :) Thanks!
Comment 7 Sebastian Dröge (slomo) 2015-03-12 06:16:45 PDT
Created attachment 248512 [details]
Patch
Comment 8 Sebastian Dröge (slomo) 2015-03-12 06:17:55 PDT
Please check if this indeed compiles with 1.0 or if anything else is missing. If it fails please paste the compiler errors here :)

It builds at least on 1.4.5 with these changes.
Comment 9 Carlos Alberto Lopez Perez 2015-03-12 11:30:12 PDT
I tested it, and it builds fine with 1.0.9. Thanks!
Comment 10 Philippe Normand 2015-03-12 11:44:30 PDT
Comment on attachment 248512 [details]
Patch

Ok let's land this then :)
Comment 11 WebKit Commit Bot 2015-03-12 12:32:07 PDT
Comment on attachment 248512 [details]
Patch

Clearing flags on attachment: 248512

Committed r181449: <http://trac.webkit.org/changeset/181449>
Comment 12 WebKit Commit Bot 2015-03-12 12:32:12 PDT
All reviewed patches have been landed.  Closing bug.