[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-commits
Subject:    Re: [phonon-gstreamer] /: Enable build of the X11 renderer if X11 can be found
From:       Samuel Gaist <samuel.gaist () edeltech ! ch>
Date:       2015-10-04 21:52:51
Message-ID: D3E70B02-CC7C-4C7C-A8E7-85E03B9B6804 () edeltech ! ch
[Download RAW message or body]


On 4 oct. 2015, at 23:20, Christophe Giboudeaux <cgiboudeaux@gmx.com> wrote:

> Hi,
> 
> On dimanche 4 octobre 2015 23:13:50 CEST Samuel Gaist wrote:
>> On 28 sept. 2015, at 15:11, Christophe Giboudeaux <cgiboudeaux@gmx.com> 
> wrote:
>>> Hi,
>>> 
>>> On jeudi 24 septembre 2015 23:01:57 CEST Samuel Gaist wrote:
>>>> Git commit dca84a8c4eb4ded5db2348f5f62a1bed31f67e17 by Samuel Gaist.
>>>> Committed on 24/09/2015 at 22:55.
>>>> Pushed by sgaist into branch 'master'.
>>>> 
>>>> Enable build of the X11 renderer if X11 can be found
>>>> 
>>>> Rather than build on all non WIN32 OS, search for X11 with and enable
>>>> the build of the renderer if it's available.
>>>> 
>>>> REVIEW https://git.reviewboard.kde.org/r/124999/
>>>> 
>>>> M  +3    -1    CMakeLists.txt
>>>> M  +5    -2    gstreamer/CMakeLists.txt
>>>> M  +12   -7    gstreamer/devicemanager.cpp
>>>> 
>>>> http://commits.kde.org/phonon-gstreamer/dca84a8c4eb4ded5db2348f5f62a1bed3
>>>> 1f6 7e17
>>>> 
>>>> diff --git a/CMakeLists.txt b/CMakeLists.txt
>>>> index e601ec6..d988495 100644
>>>> --- a/CMakeLists.txt
>>>> +++ b/CMakeLists.txt
>>>> @@ -49,7 +49,9 @@ find_package(GObject REQUIRED)
>>>> find_package(LibXml2 REQUIRED)
>>>> macro_log_feature(LIBXML2_FOUND "LibXml2" "LibXml2 is required to compile
>>>> the gstreamer backend for Phonon" "http://xmlsoft.org/downloads.html"
>>>> TRUE)
>>>> 
>>>> -
>>>> +set(REQUIRED_QT_VERSION 5.2.0)
>>>> +macro_optional_find_package(Qt5X11Extras ${REQUIRED_QT_VERSION})
>>>> +macro_log_feature(Qt5X11Extras_FOUND "Qt5X11Extras" "Qt5X11Extras is
>>>> needed for the x11renderer"
>>>> "http://doc.qt.io/qt-5/qtx11extras-index.html" FALSE
>>>> "${REQUIRED_QT_VERSION}")
>>>> 
>>>> add_subdirectory(gstreamer)
>>>> 
>>>> diff --git a/gstreamer/CMakeLists.txt b/gstreamer/CMakeLists.txt
>>>> index 60bfcb9..beccb37 100644
>>>> --- a/gstreamer/CMakeLists.txt
>>>> +++ b/gstreamer/CMakeLists.txt
>>>> @@ -88,11 +88,14 @@ if (OPENGL_FOUND)
>>>> 
>>>>    list(APPEND phonon_gstreamer_SRCS glrenderer.cpp)
>>>> 
>>>> endif ()
>>>> 
>>>> -if(NOT WIN32)
>>>> +if(Qt5X11Extras_FOUND)
>>>> 
>>>>  set(phonon_gstreamer_SRCS
>>>> 
>>>>      ${phonon_gstreamer_SRCS}
>>>>      x11renderer.cpp)
>>>> 
>>>> -endif(NOT WIN32)
>>>> +  add_definitions(-DHAVE_X11)
>>>> +  qt5_use_modules(phonon_gstreamer X11Extras)
>>>> +  target_link_libraries(phonon_gstreamer Qt5::X11Extras)
>>>> +endif(Qt5X11Extras_FOUND)
>>> 
>>> This causes two issues :
>>> 
>>> phonon-gstreamer can be used with both Qt4 and 5. Looking for Qt5X11Extras
>>> unconditionnally will fail for Qt4 builds.
>>> 
>>> The build will also fail when using Qt5 and not having Qt5X11Extras for
>>> whatever reason with something looking like:
>>> 14:33:08 /home/jenkins/builds/phonon-gstreamer/latest-qt4/gstreamer/
>>> videowidget.cpp:83: undefined reference to `typeinfo for
>>> Phonon::Gstreamer::X11Renderer'
>>> 
>>> 14:33:08 /home/jenkins/builds/phonon-gstreamer/latest-qt4/gstreamer/
>>> videowidget.cpp:85: undefined reference to
>>> `Phonon::Gstreamer::X11Renderer::setOverlay()'
>>> 
>>> Christophe
>> 
>> Hi,
>> 
>> Sorry for the late reply.
>> 
>> If I've followed things correctly, your commit fixed the Qt 5 build so the
>> current problem would be Qt 4.
>> 
>> Is that correct ?
>> 
> 
> No.
> It fails when building with Qt4 [1] *and* when building with Qt5 but without 
> Qt5X11Extras (which is an optional build dependency) [2]
> 
> [1] https://build.kde.org/job/phonon-gstreamer%20master%20latest-qt4/
> [2] https://build.kde.org/job/phonon-gstreamer%20master%20kf5-qt5/
> 
> Christophe

Ok,

I see two options here:

1) Refactor the Q5X11Extras part using only usual X11 detection
2) Have one branch for Qt 4 and one for Qt 5.

No 1 should be compatible with both Qt series

What would be the preferred way ?

Samuel=
[prev in list] [next in list] [prev in thread] [next in thread]