Opened 4 years ago

Closed 5 months ago

#14976 closed enhancement (fixed)

Update wxMediaCtrl for GStreamer 1.0

Reported by: eheintzmann Owned by:
Priority: normal Milestone: 3.2.0
Component: wxGTK Version:
Keywords: gstreamer work-needed Cc: leio, olly
Blocked By: Blocking:
Patch: no

Description

With GNOME 3.0 (april 2011), the GNOME Project decided to discontinue GConf.
It won't be maintained anymore: bugs and security holes will not be fixed.
Thus, wxWidgets shouldn't depend on GConf any longer.
Please, migrate to GSettings.
GStettings is the official GNOME replacement for GConf, and the GNOME project
is porting all of its applications to it:
https://live.gnome.org/GnomeGoals/GSettingsMigration

You will find the official from GConf to GSettings porting guide at:
http://developer.gnome.org/gio/stable/ch31.html

Attachments (2)

gst1.0.patch download (7.6 KB) - added by crisb 15 months ago.
Patch against wxWidgets 3.0.2
gst1.0.2.patch download (36.6 KB) - added by slomo 5 months ago.
gst1.0.patch (updated)

Download all attachments as: .zip

Change History (31)

comment:1 follow-up: Changed 4 years ago by vadz

  • Component changed from base to wxGTK
  • Resolution set to invalid
  • Status changed from new to closed

Where exactly are we using GConf? I didn't think we did and I can't find any references to it anywhere, so AFAICS we're not affected by this change. Please reopen if I'm missing something here.

comment:2 in reply to: ↑ 1 Changed 4 years ago by eheintzmann

Replying to vadz:

Where exactly are we using GConf? I didn't think we did and I can't find any references to it anywhere, so AFAICS we're not affected by this change. Please reopen if I'm missing something here.

Package amule-gnome-support depends on gconf.
When I reported this bug to aMule bugzilla, they answered that the problem was in wxWidgets.
This is why I report this bug here.

http://amule.forumer.com/port-from-gconf-to-gsettings-t2300205.html

comment:3 Changed 4 years ago by RedTide

I'm not sure, but maybe it could be related to wx media, see http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=493090
(Maybe amule use it for some video preview for downloaded files?)

I'm not a gtk/gstreamer expert but it seems there are still gconf dependencies also in 2.9 (see src/unix/mediactrl.cpp).

Other than that I never heard anything else depending to gconf in wx.

comment:4 Changed 4 years ago by eheintzmann

Yes, gst-gconf 0.10 depends on GConf. And we'll have to wait for the new 1.0 version.
In the meaning time, I suggest to reopen this bug

comment:5 Changed 4 years ago by vadz

  • Resolution invalid deleted
  • Status changed from closed to reopened
  • Summary changed from Migrate from GConf to GSettings to Use GSettings instead of GConf in wxMediaCtrl and wxLaunchDefaultBrowser()

Thanks, gstreamer indeed depends on gconf. And there is an invocation of "gconftool-2" in source:wxWidgets/trunk/src/unix/utilsx11.cpp too. The latter shouldn't be difficult to fix (although I couldn't find what was the equivalent of gconftool with GSettings yet) but we probably need to wait for a next version of gstreamer to fix the former.

comment:6 Changed 4 years ago by eheintzmann

although I couldn't find what was the equivalent of gconftool with GSettings yet

http://developer.gnome.org/gio/unstable/gsettings-tool.html

comment:7 Changed 4 years ago by pcor

  • Priority changed from normal to low
  • Summary changed from Use GSettings instead of GConf in wxMediaCtrl and wxLaunchDefaultBrowser() to Update wxMediaCtrl for GStreamer 1.0

The use of gconftool-2 in wxDoLaunchDefaultBrowser() is to support older systems, should not be removed, and does not need to be updated to use GSettings. It will not be run on newer systems, because the earlier call to xdg-open will have succeeded.

wxMediaCtrl uses GStreamer, so any system using GStreamer 0.10 or earlier will depend on GConf. Patches for support of GStreamer 1.0 in wxMediaCtrl would be welcome.

comment:8 Changed 4 years ago by eheintzmann

I think it would be better to use gnome-open:
http://www.unix.com/man-page/all/1/gnome-open/

comment:9 Changed 4 years ago by eheintzmann

if (desktop == wxT("GNOME"))

{

gnome-open directly opens the given URL

if (wxExecute(wxT("gnome-open") + url))

return true;

}

comment:10 Changed 4 years ago by vadz

Isn't this Solaris only? And xdg-open should be perfectly fine, really.

comment:11 Changed 4 years ago by eheintzmann

I mean replacing gconftool by gnome-open

	bool wxDoLaunchDefaultBrowser(const wxString& url, int flags)
888	{
889	    wxUnusedVar(flags);
890	
891	    // Our best best is to use xdg-open from freedesktop.org cross-desktop
892	    // compatibility suite xdg-utils
893	    // (see http://portland.freedesktop.org/wiki/) -- this is installed on
894	    // most modern distributions and may be tweaked by them to handle
895	    // distribution specifics. Only if that fails, try to find the right
896	    // browser ourselves.
897	    wxString path, xdg_open;
898	    if ( wxGetEnv("PATH", &path) &&
899	         wxFindFileInPath(&xdg_open, path, "xdg-open") )
900	    {
901	        if ( wxExecute(xdg_open + " " + url) )
902	            return true;
903	    }
904	
905	    wxString desktop = wxTheApp->GetTraits()->GetDesktopEnvironment();
906	
907	    // GNOME and KDE desktops have some applications which should be always installed
908	    // together with their main parts, which give us the
909	    if (desktop == wxT("GNOME"))
            {
+              // gnome-open directly opens the given URL
+             if (wxExecute(wxT("gnome-open") + url))
+               return true;    }
925	    else if (desktop == wxT("KDE"))
926	    {
927	        // kfmclient directly opens the given URL
928	        if (wxExecute(wxT("kfmclient openURL ") + url))
929	            return true;
930	    }
931	
932	    return false;
933	}
Last edited 15 months ago by vadz (previous) (diff)

comment:12 Changed 4 years ago by vadz

Yes, and my comment about xdg-open, used just above, always working on any system recent enough to deprecate gconftool, still stands. There is really no problem here.

comment:13 Changed 22 months ago by leio

  • Priority changed from low to normal

wxGTK should simply use gtk_show_uri if gtk ver >=2.14. Though I suppose you guys still insist on supporting older, so that xdg-open business is for older than that still, and wxX11/wxUniv.

Is there any ongoing work on gstreamer 1.0 support somewhere? This 0.10 dep is one blocker for removing gst0.10 from Gentoo and I might have to step up here. And given how long 0.10 has been completely unmaintained upstream and how most distros want to get rid of it rather sooner than later, I am adjusting priority to higher (not sure anyone tracks that though).

comment:14 Changed 22 months ago by leio

  • Cc leio added

comment:15 Changed 22 months ago by vadz

Any changes using gtk_show_uri() would be welcome, it just needs to be inside compile and run-time version check (see any existing occurrences of gtk_check_version() in the sources).

As for gstreamer 1.0, I have really no idea what would be involved in upgrading to it, are there (m)any incompatible changes?

In any case, if you could help with this, it would, of course, be great. TIA!

Changed 15 months ago by crisb

Patch against wxWidgets 3.0.2

comment:16 Changed 15 months ago by crisb

i've attached a patch which builds against 3.0.2, and makes wxgtk use gstreamer 1.0.

i've tested the mediaplayer sample which seems working.

crisb (openmandriva)

comment:17 Changed 15 months ago by ojwb

  • Cc olly added

Debian are also aiming to migrate away from gstreamer 0.10.

comment:18 Changed 15 months ago by vadz

  • Keywords gstreamer work-needed added
  • Milestone set to 3.2.0
  • Status changed from reopened to confirmed
  • Type changed from defect to enhancement

Thanks for the patch!

I don't think we can change this in 3.0, but we should try to do it for 3.2.

From a quick look at the patch, it seems like it doesn't update the messages in configure nor comments in the sources which still speak about 0.8 and 0.10 that are not supported any longer. It would be nice if it could be done too.

comment:19 Changed 13 months ago by ojwb

Sebastian Dröge is driving the gstreamer 1.0 transition in Debian - I asked him to look over gst1.0.patch (https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=785924#17) and he kindly did and commented:

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.

comment:20 Changed 9 months ago by ojwb

crisb: Did you have a chance to review slomo's comments on your patch (see comment:19)?

comment:21 Changed 5 months ago by slomo

I attached a new version of the patch which should work fine, but I don't know how to test this. Is there some application using this API that I can use?

Also I removed the GStreamer 0.8 (!) and 0.10 support, as otherwise even more #ifdefs are needed and both were broken with the previous patch already anyway. Nobody should be using any of those older versions anymore at this point.

Independent of that, if there's interest I could write a second GStreamer backend that uses the new GstPlayer API from 1.7.2: https://gstreamer.freedesktop.org/data/doc/gstreamer/head/gst-plugins-bad-libs/html/gst-plugins-bad-libs-gstplayer.html

This maps more or less directly to the wxMediaBackendCommonBase interface, should solve lots of problems with the current GStreamer backend code and should make it ~100 lines instead of ~1400.

comment:22 Changed 5 months ago by vadz

Sorry, attaching the new version didn't work because of a name clash with the existing attachment, could you please redo it? But I'm also afraid that the patch itself needs to be redone because 0.8 support has already been removed recently, so there are probably going to be conflicts applying this one.

I am not sure which systems use which versions of GStreamer exactly (0.10, 1.x, 1.7.2, ...), so I don't know if the proposed new backend could replace the existing one. But it would definitely be nice to have it at least as an alternative implementation -- wxMediaCtrl supports this easily and we could just build it for the systems which have new enough GStreamer while continuing to build the old/current one for all of them.

So this would be definitely welcome in any case and especially so if it's so much simpler.

TIA!

Changed 5 months ago by slomo

gst1.0.patch (updated)

comment:23 Changed 5 months ago by slomo

I would remove 0.10 support, except for very few enterprise/LTS distros nobody is shipping that as the default anymore and it's completely unmaintained by the GStreamer project since 3 years.

I'll update the patch against latest GIT if it's acceptable to remove 0.10 support for you. Otherwise I'll have to find some more time to make it work with both versions, it's going to require a few new #ifdefs :)

Edit: And I'll submit it on GitHub as a proper pull request then, probably makes your life a bit easier.

Last edited 5 months ago by slomo (previous) (diff)

comment:24 Changed 5 months ago by vadz

3 years doesn't seem that long, to be honest, there are still plenty of people using 10 years old wxWidgets 2.8, unfortunately.

I thought that perhaps we could skip 1.0 entirely and keep using the current 0.10 for anything less than 1.7.2 for which we would use your newer and simpler backend, but I'm not sure about how realistic is this.

The safest is to support both 0.10 and 1.0 with ifdefs, but if it's too complicated supporting only 1.0 is better than supporting 0.10, of course.

comment:25 Changed 5 months ago by slomo

It's not really "too complicated", just makes the code more #ifdef spaghetti than it already is :) But I guess it's better after the 0.8 support was removed, I'll take a look.

For the 1.7.2+ based, simpler backend, I'll take a look at that too afterwards. However 1.7.2 was just released this Friday, so it will probably be a good idea to also get the existing code ported in any case.

comment:26 Changed 5 months ago by leio

Maybe it would be possible to have 0.10, 1.0 and 1.8 as separate wxMediaCtrl backends in separate code directories, controlled by configure time and runtime choices? Or was the backend stuff only working across different ports (wxMSW vs wxGTK etc)?
For distributions we need to clean up and remove any gst0.10 code, so 1.0 support we'll need to backport to 3.0 as well, unless wx3.2 is coming out very soon.

comment:27 Changed 5 months ago by slomo

leio, 0.10/1.0 can be the same codebase really, just with #ifdefs... and then another backend that is for 1.8.

You can use the patch I attached for 3.0 btw, that's what it was based on.

comment:28 Changed 5 months ago by slomo

Updated patch against master can be found here https://github.com/wxWidgets/wxWidgets/pull/225

comment:29 Changed 5 months ago by vadz

  • Resolution set to fixed
  • Status changed from confirmed to closed
Note: See TracTickets for help on using tickets.