Bug#785924: [Freewx-maint] Bug#785924: wxwidgets3.0: Please update to GStreamer 1.x

[Sebastian Dröge]

On Mo, 2016-02-22 at 00:49 +1300, Olly Betts wrote:
> 
> Sebastian Dröge  wrote:
> > How can I test the wxwidgets GStreamer integration?
> You need to test via an application which uses it - something which
> depends on the wx or wxpython media package.  I think I've used
> whyteboard for this before.
> Thanks for looking at this.

Ok, it all seems to work. I've uploaded this to DELAYED/1, just let me
know if you want to do the upload yourself instead :)

signature.asc
Description: This is a digitally signed message part

Bug#785924: [Freewx-maint] Bug#785924: wxwidgets3.0: Please update to GStreamer 1.x

[Olly Betts]

Sebastian Dröge  wrote:
> How can I test the wxwidgets GStreamer integration?

You need to test via an application which uses it - something which depends
on the wx or wxpython media package.  I think I've used whyteboard for this
before.

Thanks for looking at this.

Cheers,
Olly

Bug#785924: wxwidgets3.0: Please update to GStreamer 1.x

[Sebastian Dröge]

On So, 2016-02-21 at 12:29 +0200, Sebastian Dröge wrote:
> On So, 2016-02-21 at 11:09 +0200, Sebastian Dröge wrote:
> > On Di, 2015-11-10 at 20:28 +, Olly Betts wrote:
> > >  
> > > I'll try to find time to test it.  Also happy for someone else to
> > > test
> > > the amended patch and NMU if it works, but please keep me in the
> > > loop to
> > > avoid duplicating effort.
> > > 
> > > (If someone does NMU, you could usefully drop `--enable-sdl` in
> > > debian/rules to close #802770 - there's no corresponding BD so
> > > this
> > > doesn't affect the built result in a clean chroot).
> > 
> > I'm looking into this now btw, will provide an updated patch some
> > time later today or so. How can I test the wxwidgets GStreamer
> > integration?
> 
> Please see attached. This builds but I don't know how to test it.

I also provided that patch upstream with some further information, but
their bug tracker requires moderation of comments so it probably takes
a while until it shows up there.

signature.asc
Description: This is a digitally signed message part

Bug#785924: wxwidgets3.0: Please update to GStreamer 1.x

[Sebastian Dröge]

On So, 2016-02-21 at 11:09 +0200, Sebastian Dröge wrote:
> On Di, 2015-11-10 at 20:28 +, Olly Betts wrote:
> >  
> > I'll try to find time to test it.  Also happy for someone else to
> > test
> > the amended patch and NMU if it works, but please keep me in the
> > loop to
> > avoid duplicating effort.
> > 
> > (If someone does NMU, you could usefully drop `--enable-sdl` in
> > debian/rules to close #802770 - there's no corresponding BD so this
> > doesn't affect the built result in a clean chroot).
> 
> I'm looking into this now btw, will provide an updated patch some time
> later today or so. How can I test the wxwidgets GStreamer integration?

Please see attached. This builds but I don't know how to test it.diff -Nru wxwidgets3.0-3.0.2+dfsg/debian/changelog wxwidgets3.0-3.0.2+dfsg/debian/changelog
--- wxwidgets3.0-3.0.2+dfsg/debian/changelog	2015-08-03 13:55:58.0 +0300
+++ wxwidgets3.0-3.0.2+dfsg/debian/changelog	2016-02-21 12:26:54.0 +0200
@@ -1,3 +1,16 @@
+wxwidgets3.0 (3.0.2+dfsg-1.3) unstable; urgency=medium
+
+  * Non-maintainer upload.
+  * debian/control.in,
+debian/rules,
+debian/patches/gst1.0.patch:
++ Port to GStreamer 1.x (Closes: #785924), patch based on the one from
+  http://trac.wxwidgets.org/ticket/14976 with a lot of changes.
+  * debian/control.in:
++ Remove obsolete build-depends on ESD and GConf.
+
+ -- Sebastian Dröge   Sun, 21 Feb 2016 11:21:28 +0200
+
 wxwidgets3.0 (3.0.2+dfsg-1.2) unstable; urgency=medium
 
   * Non maintainer upload.
diff -Nru wxwidgets3.0-3.0.2+dfsg/debian/control wxwidgets3.0-3.0.2+dfsg/debian/control
--- wxwidgets3.0-3.0.2+dfsg/debian/control	2015-07-30 14:59:26.0 +0300
+++ wxwidgets3.0-3.0.2+dfsg/debian/control	2016-02-21 12:15:52.0 +0200
@@ -5,10 +5,10 @@
 Build-Depends: debhelper (>= 9), gettext, libgtk2.0-dev,
  g++ (>= 4:5.2),
  zlib1g-dev, libjpeg-dev, libpng-dev, libtiff5-dev, libsm-dev,
- libgl1-mesa-dev | libgl-dev, libglu1-mesa-dev | libglu-dev, libesd0-dev,
- autotools-dev, libexpat1-dev, dpkg-dev (>= 1.16.1~),
- libxt-dev, libgstreamer-plugins-base0.10-dev, libgconf2-dev, libwebkitgtk-dev,
- libnotify-dev
+ libgl1-mesa-dev | libgl-dev, libglu1-mesa-dev | libglu-dev,
+ autotools-dev, autoconf, libexpat1-dev, dpkg-dev (>= 1.16.1~), libxt-dev,
+ libgstreamer1.0-dev, libgstreamer-plugins-base1.0-dev,
+ libwebkitgtk-dev, libnotify-dev
 Maintainer: wxWidgets Maintainers 
 Uploaders: Olly Betts 
 Standards-Version: 3.9.6
@@ -129,7 +129,10 @@
 Package: libwxgtk-media3.0-0v5
 Architecture: any
 Pre-Depends: ${misc:Pre-Depends}
-Depends: ${shlibs:Depends}, ${misc:Depends}
+Depends: ${shlibs:Depends}, ${misc:Depends},
+  gstreamer1.0-plugins-base,
+  gstreamer1.0-plugins-good,
+  gstreamer1.0-x
 Multi-Arch: same
 Breaks: libwxgtk-media3.0-0
 Replaces: libwxgtk-media3.0-0
diff -Nru wxwidgets3.0-3.0.2+dfsg/debian/control.in wxwidgets3.0-3.0.2+dfsg/debian/control.in
--- wxwidgets3.0-3.0.2+dfsg/debian/control.in	2015-07-30 14:59:22.0 +0300
+++ wxwidgets3.0-3.0.2+dfsg/debian/control.in	2016-02-21 12:15:50.0 +0200
@@ -5,10 +5,10 @@
 Build-Depends: debhelper (>= 9), gettext, libgtk2.0-dev,
  g++ (>= 4:5.2),
  zlib1g-dev, libjpeg-dev, libpng-dev, libtiff5-dev, libsm-dev,
- libgl1-mesa-dev | libgl-dev, libglu1-mesa-dev | libglu-dev, libesd0-dev,
- autotools-dev, libexpat1-dev, dpkg-dev (>= 1.16.1~),
- libxt-dev, libgstreamer-plugins-base0.10-dev, libgconf2-dev, libwebkitgtk-dev,
- libnotify-dev
+ libgl1-mesa-dev | libgl-dev, libglu1-mesa-dev | libglu-dev,
+ autotools-dev, autoconf, libexpat1-dev, dpkg-dev (>= 1.16.1~), libxt-dev,
+ libgstreamer1.0-dev, libgstreamer-plugins-base1.0-dev,
+ libwebkitgtk-dev, libnotify-dev
 Maintainer: wxWidgets Maintainers 
 Uploaders: Olly Betts 
 Standards-Version: 3.9.6
@@ -129,7 +129,10 @@
 Package: libwxgtk-media=SOV
 Architecture: any
 Pre-Depends: ${misc:Pre-Depends}
-Depends: ${shlibs:Depends}, ${misc:Depends}
+Depends: ${shlibs:Depends}, ${misc:Depends},
+  gstreamer1.0-plugins-base,
+  gstreamer1.0-plugins-good,
+  gstreamer1.0-x
 Multi-Arch: same
 Breaks: libwxgtk-media3.0-0
 Replaces: libwxgtk-media3.0-0
diff -Nru wxwidgets3.0-3.0.2+dfsg/debian/patches/gst1.0.patch wxwidgets3.0-3.0.2+dfsg/debian/patches/gst1.0.patch
--- wxwidgets3.0-3.0.2+dfsg/debian/patches/gst1.0.patch	1970-01-01 02:00:00.0 +0200
+++ wxwidgets3.0-3.0.2+dfsg/debian/patches/gst1.0.patch	2016-02-21 11:48:12.0 +0200
@@ -0,0 +1,886 @@
+Index: wxwidgets3.0-3.0.2+dfsg/configure.in
+===
+--- wxwidgets3.0-3.0.2+dfsg.orig/configure.in
 wxwidgets3.0-3.0.2+dfsg/configure.in
+@@ -7543,43 +7543,22 @@ if test "$wxUSE_MEDIACTRL" = "yes" -o "$
+ wxUSE_GSTREAMER="no"
+ 
+ dnl ---
+-dnl Test for at least 0.8 

Bug#785924: wxwidgets3.0: Please update to GStreamer 1.x

[Sebastian Dröge]

On Di, 2015-11-10 at 20:28 +, Olly Betts wrote:
> 
> I'll try to find time to test it.  Also happy for someone else to test
> the amended patch and NMU if it works, but please keep me in the loop to
> avoid duplicating effort.
> 
> (If someone does NMU, you could usefully drop `--enable-sdl` in
> debian/rules to close #802770 - there's no corresponding BD so this
> doesn't affect the built result in a clean chroot).

I'm looking into this now btw, will provide an updated patch some time
later today or so. How can I test the wxwidgets GStreamer integration?

signature.asc
Description: This is a digitally signed message part

Bug#785924: wxwidgets3.0: Please update to GStreamer 1.x

[Olly Betts]

On Sun, Nov 01, 2015 at 11:14:55AM +0200, Sebastian Dröge wrote:
> On Sa, 2015-10-31 at 20:39 +, Olly Betts wrote:
> > Status is stalled, essentially.
> > 
> > I gather from what slomo said that the current patch isn't suitable
> > for applying.
> 
> The first remark is a minor memory leak, the second is just cleanup
> (and usage of the correct API) but at this time has no functional
> difference (but might in the future).

I'd be happier not to leave potential timebombs around, but you haven't
given enough information about this for me to address it...

> The leak you can fix by adding a simple gst_caps_unref() for the caps
> in that function.

OK.

> Does the patch work otherwise? If so, just get it in after adding that
> one line mentioned above :)

I haven't tried to test it yet - I've been short of time lately and it's
a non-trivial amount of work to do so, and my understanding was that the
patch wasn't suitable as-is.

I'll try to find time to test it.  Also happy for someone else to test
the amended patch and NMU if it works, but please keep me in the loop to
avoid duplicating effort.

(If someone does NMU, you could usefully drop `--enable-sdl` in
debian/rules to close #802770 - there's no corresponding BD so this
doesn't affect the built result in a clean chroot).

Cheers,
Olly

Bug#785924: wxwidgets3.0: Please update to GStreamer 1.x

[Sebastian Dröge]

On Sa, 2015-10-31 at 20:39 +, Olly Betts wrote:
> On Sat, Oct 31, 2015 at 02:16:28PM +0100, Moritz Muehlenhoff wrote:
> > On Thu, May 21, 2015 at 02:31:29PM +0300, Sebastian Dröge wrote:
> > > Hi Olly,
> > > 
> > > On Do, 2015-05-21 at 01:24 +0100, Olly Betts wrote:
> > > 
> > > > But there's an upstream ticket about switching to gstreamer 1.0
> > > > with a
> > > > recently added patch.  I'd appreciate a quick review from
> > > > someone who
> > > > has more idea about gstreamer's API than I do if you have a few
> > > > minutes
> > > > (it's not a huge patch):
> > > > 
> > > > http://trac.wxwidgets.org/attachment/ticket/14976/gst1.0.patch
> > > > 
> > > > I can tell the configure part needs improving, at least to make
> > > > it
> > > > upstream-able, which makes me wonder about its general quality.
> > > 
> > > wxGStreamerMediaBackend::QueryVideoSizeFromPad() probably leaks
> > > the caps
> > > now, you need to unref() them after usage.
> > > 
> > > !gst_structure_has_name (gst_message_get_structure(message),
> > > "prepare-window-handle"))
> > > should be using
> > > gst_is_video_overlay_prepare_window_handle_message()
> > > 
> > > Otherwise seems ok if that is really enough to make things work.
> > > Not
> > > sure if more changes are needed elsewhere.
> > 
> > what's the status? wxwidgets is now one of the two remaining
> > packages
> > blocking the removal of gstreamer 0.10 from stretch.
> 
> Status is stalled, essentially.
> 
> I gather from what slomo said that the current patch isn't suitable
> for applying.

The first remark is a minor memory leak, the second is just cleanup
(and usage of the correct API) but at this time has no functional
difference (but might in the future).

The leak you can fix by adding a simple gst_caps_unref() for the caps
in that function.


Does the patch work otherwise? If so, just get it in after adding that
one line mentioned above :)

signature.asc
Description: This is a digitally signed message part

Bug#785924: wxwidgets3.0: Please update to GStreamer 1.x

[Moritz Muehlenhoff]

On Thu, May 21, 2015 at 02:31:29PM +0300, Sebastian Dröge wrote:
> Hi Olly,
> 
> On Do, 2015-05-21 at 01:24 +0100, Olly Betts wrote:
> 
> > But there's an upstream ticket about switching to gstreamer 1.0 with a
> > recently added patch.  I'd appreciate a quick review from someone who
> > has more idea about gstreamer's API than I do if you have a few minutes
> > (it's not a huge patch):
> > 
> > http://trac.wxwidgets.org/attachment/ticket/14976/gst1.0.patch
> > 
> > I can tell the configure part needs improving, at least to make it
> > upstream-able, which makes me wonder about its general quality.
> 
> wxGStreamerMediaBackend::QueryVideoSizeFromPad() probably leaks the caps
> now, you need to unref() them after usage.
> 
> !gst_structure_has_name (gst_message_get_structure(message), 
> "prepare-window-handle"))
> should be using
> gst_is_video_overlay_prepare_window_handle_message()
> 
> Otherwise seems ok if that is really enough to make things work. Not
> sure if more changes are needed elsewhere.

Hi Olly,
what's the status? wxwidgets is now one of the two remaining packages blocking
the removal of gstreamer 0.10 from stretch.

Cheers,
Moritz

Bug#785924: wxwidgets3.0: Please update to GStreamer 1.x

[Olly Betts]

On Sat, Oct 31, 2015 at 02:16:28PM +0100, Moritz Muehlenhoff wrote:
> On Thu, May 21, 2015 at 02:31:29PM +0300, Sebastian Dröge wrote:
> > Hi Olly,
> > 
> > On Do, 2015-05-21 at 01:24 +0100, Olly Betts wrote:
> > 
> > > But there's an upstream ticket about switching to gstreamer 1.0 with a
> > > recently added patch.  I'd appreciate a quick review from someone who
> > > has more idea about gstreamer's API than I do if you have a few minutes
> > > (it's not a huge patch):
> > > 
> > > http://trac.wxwidgets.org/attachment/ticket/14976/gst1.0.patch
> > > 
> > > I can tell the configure part needs improving, at least to make it
> > > upstream-able, which makes me wonder about its general quality.
> > 
> > wxGStreamerMediaBackend::QueryVideoSizeFromPad() probably leaks the caps
> > now, you need to unref() them after usage.
> > 
> > !gst_structure_has_name (gst_message_get_structure(message), 
> > "prepare-window-handle"))
> > should be using
> > gst_is_video_overlay_prepare_window_handle_message()
> > 
> > Otherwise seems ok if that is really enough to make things work. Not
> > sure if more changes are needed elsewhere.
> 
> what's the status? wxwidgets is now one of the two remaining packages
> blocking the removal of gstreamer 0.10 from stretch.

Status is stalled, essentially.

I gather from what slomo said that the current patch isn't suitable
for applying.  But I don't know anything about the old or new gstreamer
APIs, and don't have the spare time to learn them, so the chances of me
successfully fixing up the patch in response to slomo's feedback are
pretty much non-existent.

There's been no response from upstream or the original patch author to
slomo's feedback on the patch so far.  I prodded them last week about
this, which hasn't had a response yet either.

My impression (from responses and lack of responses to previous bug
reports about the wxmedia stuff) is that it's not a feature which the
active upstream developers care much about, so I suspect any fixes will
need to come from outside.

Despite quite a lot of noise about enabling it, it only actually seems
to be used by whyteboard currently (at least if I can work "dak rm"),
and support for playing video there hardly seems an essential feature.
So a rather brutal way out might be to disable support again, and if
that upsets anyone, then ... well, improved patches welcome.

Cheers,
Olly