After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 697808 - sdp: add boxed type for GstSDPMessage
sdp: add boxed type for GstSDPMessage
Product: GStreamer
Classification: Platform
Component: gst-plugins-base
Other Linux
: Normal enhancement
: 1.1.1
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Reported: 2013-04-11 15:18 UTC by Jose Antonio Santos Cadenas
Modified: 2017-02-04 16:22 UTC
See Also:
GNOME target: ---
GNOME version: ---

sdp: Make GstSDPMessage a boxed type (9.22 KB, patch)
2013-04-11 15:18 UTC, Jose Antonio Santos Cadenas
needs-work Details | Review
patch (14.44 KB, patch)
2013-04-11 22:55 UTC, Jose Antonio Santos Cadenas
needs-work Details | Review
sdp: add boxed type for GstSDPMessage (14.12 KB, patch)
2013-04-12 07:39 UTC, Jose Antonio Santos Cadenas
committed Details | Review

Description Jose Antonio Santos Cadenas 2013-04-11 15:18:13 UTC
Created attachment 241262 [details] [review]
sdp: Make GstSDPMessage a boxed type

Attached patch converts GstSDPMessage type into a boxed type, this way it is easier to use it as an attribute or in signals.

Patch can be applied over 1.0 and master branches.

Patch includes tests for this new functionality.
Comment 1 Tim-Philipp Müller 2013-04-11 22:45:33 UTC
Comment on attachment 241262 [details] [review]
sdp: Make GstSDPMessage a boxed type

It looks like you forgot to 'git add' the new tests/check/libs/sdp.c file? (while at it, make sure to add the binary to .gitignore).
Comment 2 Jose Antonio Santos Cadenas 2013-04-11 22:55:38 UTC
Created attachment 241311 [details] [review]

Sorry, here is the new patch
Comment 3 Tim-Philipp Müller 2013-04-11 23:05:07 UTC
Comment on attachment 241311 [details] [review]

>+++ b/gst-libs/gst/sdp/gstsdpmessage.h
>@@ -251,6 +251,7 @@ typedef struct {
>  * The contents of the SDP message.
>  */
> typedef struct {
>+  GType            type;
>   gchar            *version;
>   GstSDPOrigin      origin;
>   gchar            *session_name;

This breaks ABI I'm afraid. I think we'll have to do without the GST_IS_SDP_MESSAGE() for now.
Comment 4 Jose Antonio Santos Cadenas 2013-04-12 07:39:58 UTC
Created attachment 241321 [details] [review]
sdp: add boxed type for GstSDPMessage

Sorry I didn't think about the ABI.

Additionally type attribute wasn't initialized, so there is no problem removing it.
Comment 5 Tim-Philipp Müller 2013-04-12 11:44:36 UTC
These new public _copy() functions are a bit weird IMHO.

I think either they should return the newly-allocated copied structure directly as return argument, or they should copy into an-already allocated struct. The former seems more straight-forward. Do we need to expose these functions at all? Maybe we should just keep them internal for now?
Comment 6 Jose Antonio Santos Cadenas 2013-04-12 12:08:35 UTC
I also prefer to return the newly-allocated structure, but I did this way to be consistent with the _new functions, that I think that are a little bit weird too. I don't have any problem changing them.

About keeping them public, I find them useful in some cases (ie: generating and answer for an offer) and I prefer to keep them public if it's OK for you.
Comment 7 Jose Antonio Santos Cadenas 2013-04-12 12:52:02 UTC
Ok, let's remove them from public, I'll send a patch with this changes.
Comment 8 Tim-Philipp Müller 2013-04-12 12:58:38 UTC
I don't mind - if you have a need for them it's fine. I didn't know if you did or not.

Your consistency argument also makes sense.

I'll just wait to see if Wim has an opinion.
Comment 9 Jose Antonio Santos Cadenas 2013-04-12 13:12:28 UTC
I need for them, as well as a need for removing functions (ie, "gst_sdp_media_remove_connection (media, id)" and similars). I'm thinking about filing a new bug for this enhancements if this one is OK at the end and you think that they are useful. Otherwise I can implement them in my project.
Comment 10 Wim Taymans 2013-04-15 12:29:08 UTC
commit 83a877daf6c2fb457b7364558fe55c3feb95cef3
Author: Wim Taymans <>
Date:   Mon Apr 15 14:25:16 2013 +0200

    sdp: add Since markers

commit ba1e6938536b02b4af705ce45f2c89822a6abc62
Author: Jose Antonio Santos Cadenas <>
Date:   Fri Apr 12 09:35:34 2013 +0200

    sdp: add boxed type for GstSDPMessage
    Also added some tests of this improvement.
Comment 11 Jose Antonio Santos Cadenas 2013-04-15 12:50:50 UTC
Sorry Wim, but the Since markers shouldn't indicate 1.1 version?

Or stable versions has always an even number?
Comment 12 Tim-Philipp Müller 2013-04-15 13:18:32 UTC
We add Since markers for the upcoming stable version.
Comment 13 Tim-Philipp Müller 2017-02-04 11:43:31 UTC
Jose, Miguel, this patch added a small amount of code that is GPLv3 to gst-plugins-base, taken from gst-plugins-kurento it seems. As your name is on the copyright header for that - would you be willing to relicense this to LGPLv2.1+ (as the rest of GStreamer)?
Comment 14 Jose Antonio Santos Cadenas 2017-02-04 14:48:54 UTC
Hi Tim,

I agree changing the license, actually it should be LGPL from the beginig. I can send a patch changing the license if you feel it's better.

Comment 15 Miguel París Díaz 2017-02-04 15:15:29 UTC
Of course, let's change the license ;).
Comment 16 Tim-Philipp Müller 2017-02-04 16:22:37 UTC
If you could send a patch that'd be great, thanks!