arfbtwn / banshee

http://banshee.fm/
MIT License
6 stars 4 forks source link

RFC: Remove Banshee.GStreamerSharp backend #6

Open Carbenium opened 7 years ago

Carbenium commented 7 years ago

The gstreamer-sharp binding library is more or less unmaintained (last commit in Arpil 2016). Additionally it isn't shipped with any large distro.

Since we have our own binding + native library I'd suggest to remove the Banshee.GStreamerSharp backend.

Carbenium commented 7 years ago

From #2 Carbenium

Exactly. It's unmaintained, doesn't ship with any large distro and it couldn't play a track. So pretty useless in my opinion :smile:

arfbtwn

Agreed, did you get that behaviour even with their master branch? I remember experimentally trying it out - had to make some changes to Banshee to get it to compile, see bb1ac56, think I tried it out and it was working at the time.

arfbtwn commented 7 years ago

I just tried it and reproduced a crash, got the below just after/as the player died so we might be able to fix the problem in the bindings.

*** Error in `/usr/bin/mono': double free or corruption (out): 0x00007f030c0181d0 ***
======= Backtrace: =========
/lib64/libc.so.6(+0x6f273)[0x7f0360707273]
/lib64/libc.so.6(+0x748e6)[0x7f036070c8e6]
/lib64/libc.so.6(+0x750ee)[0x7f036070d0ee]
/usr/lib64/libgobject-2.0.so.0(g_value_unset+0x30)[0x7f0357de37a0]
/usr/lib/libgstreamer-1.0.so(gst_structure_free+0xac)[0x7f033f84ce4c]
/usr/lib/libgstreamer-1.0.so(+0xa0f3d)[0x7f033f854f3d]
/usr/lib64/libgobject-2.0.so.0(g_value_unset+0x30)[0x7f0357de37a0]
/usr/lib/libgstreamer-1.0.so(gst_structure_free+0xac)[0x7f033f84ce4c]
/usr/lib/libgstreamer-1.0.so(+0x68c1f)[0x7f033f81cc1f]
[0x40aafff2]

followed by, I think this bit's important:

  at <unknown> <0xffffffff>
  at (wrapper managed-to-native) Gst.MiniObject.gst_mini_object_unref (intptr) <IL 0x00009, 0x00071>
  at Gst.MiniObject/FinalizerInfo.Handler () [0x00007] in /tmp/portage/dev-dotnet/gstreamer-sharp-9999/work/gstreamer-sharp-9999/sources/generated/Gst/MiniObject.cs:233
  at GLib.Timeout/TimeoutProxy.Handler () [0x0000f] in /tmp/portage/dev-dotnet/gtk-sharp-9999/work/gtk-sharp-9999/glib/Timeout.cs:50
  at (wrapper native-to-managed) GLib.Timeout/TimeoutProxy.Handler () <IL 0x0002b, 0x000d1>
  at <unknown> <0xffffffff>
  at (wrapper managed-to-native) Gtk.Application.gtk_main () <IL 0x00008, 0x0006a>
  at Gtk.Application.Run () [0x00001] in /tmp/portage/dev-dotnet/gtk-sharp-9999/work/gtk-sharp-9999/gtk/Application.cs:133
  at Banshee.Gui.GtkBaseClient.Run () [0x0001e] in /home/nicholas/Source/Csharp/banshee/src/Core/Banshee.ThickClient/Banshee.Gui/GtkBaseClient.cs:222
  at Banshee.Gui.GtkBaseClient.Startup () [0x00010] in /home/nicholas/Source/Csharp/banshee/src/Core/Banshee.ThickClient/Banshee.Gui/GtkBaseClient.cs:79
  at Hyena.Gui.CleanRoomStartup.Startup (Hyena.Gui.CleanRoomStartup/StartupInvocationHandler) [0x00050] in /home/nicholas/Source/Csharp/banshee/src/Hyena/Hyena.Gui/Hyena.Gui/CleanRoomStartup.cs:54
  at Banshee.Gui.GtkBaseClient.Startup<T_REF> () [0x00049] in /home/nicholas/Source/Csharp/banshee/src/Core/Banshee.ThickClient/Banshee.Gui/GtkBaseClient.cs:74
  at Banshee.Gui.GtkBaseClient.Startup<T_REF> (string[]) [0x00021] in /home/nicholas/Source/Csharp/banshee/src/Core/Banshee.ThickClient/Banshee.Gui/GtkBaseClient.cs:64
  at Nereid.Client.Main (string[]) [0x00002] in /home/nicholas/Source/Csharp/banshee/src/Clients/Nereid/Nereid/Client.cs:54
  at (wrapper runtime-invoke) <Module>.runtime_invoke_void_object (object,intptr,intptr,intptr) <IL 0x00051, 0x000ef>
Carbenium commented 7 years ago

Yap. Thats exactly the stack trace I get, too. gstreamer-sharp is compiled from source (4af160).

I'm on the feature/lite branch since Banshee doesn't even start for me on master. Main window appears but without album browser and than it just hangs. Not, that it would be important for this issue.

Carbenium commented 7 years ago

Alright. I pinned it down to https://github.com/arfbtwn/banshee/blob/feature/lite/src/Backends/Banshee.GStreamerSharp/Banshee.GStreamerSharp/PlayerEngine.cs#L691

Remove that Dispose and everything works fine. I think those values are somehow destructed when the tag list is destroyed somewhere in the native code. By calling first Dispose this leads to the double-free.

A slightly more detailed part of the crash report:

Thread 1 (Thread 0x7f59250fd740 (LWP 20557)):
#0  0x00007f59245f2f7b in __waitpid (pid=20576, stat_loc=0x7ffd27f233bc, options=0) at ../sysdeps/unix/sysv/linux/waitpid.c:29
#1  0x00000000004ac659 in  ()
#2  0x00007f59245f3390 in <signal handler called> () at /lib/x86_64-linux-gnu/libpthread.so.0
#3  0x00007f5924038428 in __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:54
#4  0x00007f592403a02a in __GI_abort () at abort.c:89
#5  0x00007f592407a7ea in __libc_message (do_abort=do_abort@entry=2, fmt=fmt@entry=0x7f59241932e0 "*** Error in `%s': %s: 0x%s ***\n") at ../sysdeps/posix/libc_fatal.c:175
#6  0x00007f5924082e0a in _int_free (ar_ptr=<optimised out>, ptr=<optimised out>, str=0x7f59241933f0 "double free or corruption (out)", action=3) at malloc.c:5004
#7  0x00007f5924082e0a in _int_free (av=<optimised out>, p=<optimised out>, have_lock=0) at malloc.c:3865
#8  0x00007f592408698c in __GI___libc_free (mem=<optimised out>) at malloc.c:2966
#9  0x00007f5918dc4450 in g_value_unset (value=value@entry=0x7f58c800d818) at /build/glib2.0-prJhLS/glib2.0-2.48.2/./gobject/gvalue.c:275
#10 0x00007f591050f28c in gst_structure_free (structure=0x1b96cc0) at gststructure.c:383
#11 0x00007f59105174fd in __gst_tag_list_free (list=0x2330f20) at gsttaglist.c:715
#12 0x0000000040948ce0 in  ()
#13 0x0000000000000000 in  ()
arfbtwn commented 7 years ago

Nice! I was wondering why the track started playing for a short period before the crash, I bet if we follow that call tree back up we'll see the library or Banshee dispose of the list. Are there any other instances in that file where we dispose of a GLib.Value from a TagList - I think we're at the point where we can confidently patch this if you want to pull request it?

Carbenium commented 7 years ago

No other references and there you go : #8 :)

arfbtwn commented 7 years ago

it's a shame GH doesn't use git-cherry to find out if pull requests are merged, it's nice to have the merged symbol on pages like this.

Carbenium commented 7 years ago

So I assume this means we keep the backend for now?

arfbtwn commented 7 years ago

Oops, there's still the maintenance issue, reopened :)

arfbtwn commented 6 years ago

@nicman23: there's an outstanding issue about maintenance but the thing with open source libraries is that they can seem that way through lack of activity and still work great.

As an example - in our native engine the last commit that did anything with its source was in August 2014 (adbcdb3dc323cbd155443475b445993a39042b83) and that looks like it was a bug fix rather than for any gstreamer changes.

Apart from that the managed engine was one of the newer features in Banshee development before people stopped working on the program, so that's the main reason why I'd like to keep it if possible.

Carbenium commented 6 years ago

As the NEWS file states

  Production-ready GStreamerSharp playback-engine backend, now default

        Our managed backend (that already allowed us to have playback working
        in the Windows platform) has been migrated to work with the revamped
        GStreamerSharp bindings (GObject-Introspection based, and capable of
        binding GStreamer 1.0 or newer) and is now the default one for all
        platforms.

        This paves the way to finish the long-standing goal of completely
        removing lower-level languages like C from our codebase, to lower
        the barrier of entry to new contributors, reduce complexity and
        increase stability.

        You can still use the optional unmanaged GStreamer backend, but
        before doing so we encourage you to provide us any feedback that
        you may have about the managed one (reporting bugs or writing to
        our forums or mailing lists), because we will deprecate this one very soon.

So I guess we should try really hard to keep the backend, rescinding my previous uneducated opinion.

TieSKey commented 6 years ago

A little bit offtopic but: Since this library doesn't ship with any large distro shouldn't it be somehow included in the project? Trying to compile gstreamer-sharp 0.99 in Kubuntu 17.10 throws a lot of "type or namespace xxx could not be found" which I can't possibly fix and the gstreamer-sharp project doesn't have an issues section or similar.

nicman23 commented 6 years ago

it is for windows mostly, also native is working fine for linux

TieSKey commented 6 years ago

Sorry again for going offtopic but could you point me to any doc on how to disable the dependency/backend so I can compile banshee on Kubuntu?

nicman23 commented 6 years ago

this works for arch linux, https://aur.archlinux.org/cgit/aur.git/tree/PKGBUILD?h=banshee-git