Closed PhilippMDoerner closed 8 months ago
For freeing the MediaStream
you want to do something like Pixbuf
/TextBuffer
: https://github.com/can-lehmann/owlkettle/blob/main/owlkettle/widgets.nim#L464
Hmm, so define a destructor got GtkMediaStream and when Video gets destroyed and garbage collected it inevitably also triggers the GtkMediaStream destructor, causing a call to g_object_unref which free's the GtkMediaStream
?
I'll try it.
Should be relatively easy though, basically just defining a destructor and a container type
For freeing the
MediaStream
you want to do something likePixbuf
/TextBuffer
: https://github.com/can-lehmann/owlkettle/blob/main/owlkettle/widgets.nim#L464
Okay, did that. I've:
GtkMediaStream
in MediaStreamObj
whose ref-type alias is MediaStream
MediaStreamObj
MediaStream
(GFile and Filename can be used)The mechanism appears to work as intended. There still seems to be very much a memory leak, but I'm not quite sure where it's coming from. At least in my testing, flipping between 2 .webm files via the filechooser easily balloons the memory consumption from 60mb to > 130mb
ElegantBeef mentioned that maybe implementing sink and move hooks could help here? I'm not entirely convinced but that's basically all I've got as a hint on what to do.
Naturally if that solves anything it'd make sense to also implement the hooks for the other types that we implemented hooks for so far.
A draft PR for #87 .
I think this is mostly finished. It provides a Video widget with setters so you can provide a fileName (string) or a file (GFile) to play in it. It... I don't think it's free of memory leaks, I see the memory consumption of the example application grow as I play around with the filename, but sometimes it also drastically jumps down again ? Honestly not sure if that means we're free of leaks or not.
I free the old media stream if a new media stream gets set via property hook at least.
This is draft because I don't yet have a strategy on how to free
GtkMediaStream
memory when the Video widget gets destroyed. Until that is cleared up I do not consider this mergeable. A possible strategy is outlined in my last comment on #87 but I'd like a sanity check for what I think could work there.