Open ylatuya opened 3 years ago
The fix does not seams to be fully working. It needs some more work.
After checking, the fix works as expected. It actually raised an issue in the GLOverlayCompositor.AddCaps binding, caused by a lack of annotations of the gst_overlay_compositor_add_caps API.
@thiblahute can you take a look at this one?
@thiblahute is there a change you can look at this PR? Thanks
@sdroege Can you please review this?
The new Query object is owned and with an extra Ref, making it isWritable=false, while it should be !owned and with refcount of 1 to make it IsWritable=false
How does this fix prevent the unowned wrapper object to stay around longer than the C GstQuery*
?
The new Query object is owned and with an extra Ref, making it isWritable=false, while it should be !owned and with refcount of 1 to make it IsWritable=false
How does this fix prevent the unowned wrapper object to stay around longer than the C
GstQuery*
?
I think I now understand a bit more the different problems around the Opaque implementation.
"unowned" Opaque objects created with GetOpaque are currently copied to ensure that the wrapper object stays around longer than the C pointer. It's definitively a problem for uses cases like the OnQuery overrides were you need to modify the real GstQuery* and not a copy of it. It's working right now because there is a bug in the Copy that does not do any copy (I will expand in this later). Omitting this bug, this is the status:
The patch in this PR works because no real copy is being done, but it's not memory safe.
I would propose to handle the creation of unowned Opaque in different ways:
Back to the existing bugs, there are several issues that needs to be fixed:
@sdroege based on your experience with bindings, do you think the proposal makes sense? If so I will proceed to do the changes and write unit test for it.
GetOpaque is returning Opaque objects with owner set to the opposite value as the one requested. It seems like the existing fix was incorrectly applied to the wrong if branch.
This behaviour can be seen when overriding virtual methods like BaseSink.OnQuery that have as parameters Gst.MiniObject's and where GetOpaque is called with owned=false.
The new Query object is owned and with an extra Ref, making it isWritable=false, while it should be !owned and with refcount of 1 to make it IsWritable=false