Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

More changes for generating gstreamer bindings #159

Draft
wants to merge 7 commits into
base: 4
Choose a base branch
from

Conversation

RSWilli
Copy link
Contributor

@RSWilli RSWilli commented Jan 31, 2025

I got some closure name collisions for the generated Connect callbacks. The collisions only happened on action signals which are only to be emitted AFAIK (please verify).

I also needed to preprocess enum member names so I added a set name.

@RSWilli
Copy link
Contributor Author

RSWilli commented Feb 2, 2025

There seems to be a diff now. I checked the docs for the GSignalFlags:

G_SIGNAL_ACTION: Action signals are signals that may freely be emitted on alive objects from user code via g_signal_emit() and friends, without the need of being embedded into extra code that performs pre or post emission adjustments on the object. They can also be thought of as object methods which can be called generically by third-party code.

I don't think that is is necessary to generate methods for those. The diff in the actions pipeline confirms that IMHO. Type safe emit calls would be very nice though.

@RSWilli
Copy link
Contributor Author

RSWilli commented Feb 4, 2025

The borrowing creates code like this: https://github.com/go-gst/go-gst/blob/generated_bindings/pkg/gst/gst.go#L35155

or search for "borrow" in the generated packages.

@@ -445,6 +445,53 @@ func (conv *Converter) cgoConverter(value *ValueConverted) bool {
}
return true

case "Gst.MiniObject", "Gst.Structure", "Gst.Caps", "Gst.Buffer", "Gst.BufferList", "Gst.Memory", "Gst.Message", "Gst.Query", "Gst.Sample":
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI I'm likely not going to accept this PR as-is. This diff is a good study for me to add additional APIs into the GIR generator that would permit this for external generators, but adding edge cases from external generators into gotk4 directly is not sustainable in the long term.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please keep this diff around so I can go back and reference it, but I would strongly recommend moving this to a draft PR so that it doesn't block this PR from being merged.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm completely on your side. I'd like to move the cases for the special types into the go-gst repo, and add some functionality to this PR that allows me to hook into this function.

I'm not quite sure how to do this though, because this is fairly deep into the call stack and quite far from the genmain.Data. If you have a suggestion please let me know.

@RSWilli RSWilli marked this pull request as draft February 5, 2025 08:59
wbartel and others added 2 commits February 5, 2025 10:51
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants