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 744093 - [API] message: expose a details structure
[API] message: expose a details structure
Status: RESOLVED DUPLICATE of bug 756806
Product: GStreamer
Classification: Platform
Component: gstreamer (core)
unspecified
Other All
: Normal enhancement
: NONE
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2015-02-06 12:59 UTC by Mathieu Duponchelle
Modified: 2017-07-14 15:27 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
message: expose a details structure. (4.78 KB, patch)
2015-02-06 12:59 UTC, Mathieu Duponchelle
reviewed Details | Review
message: expose an error details structure. (5.23 KB, patch)
2015-02-13 17:01 UTC, Mathieu Duponchelle
none Details | Review
message: expose error details. (9.39 KB, patch)
2015-02-16 13:12 UTC, Mathieu Duponchelle
none Details | Review

Description Mathieu Duponchelle 2015-02-06 12:59:50 UTC
+ [API] gst_message_get_details, returning a copy
+ [API] gst_message_set_details, wrapping gst_structure_set
Comment 1 Mathieu Duponchelle 2015-02-06 12:59:53 UTC
Created attachment 296269 [details] [review]
message: expose a details structure.
Comment 2 Mathieu Duponchelle 2015-02-06 13:00:34 UTC
As discussed on https://bugzilla.gnome.org/show_bug.cgi?id=743703.
Comment 3 Tim-Philipp Müller 2015-02-06 13:22:24 UTC
Comment on attachment 296269 [details] [review]
message: expose a details structure.

The locking is not needed here (mini object writability).

This would also require bindings-friendly API then.

Why make it generic though? I think it should be message-specific. And if we do it like this, we may just as well re-use the GstStructure from gst_message_get_structure().
Comment 4 Mathieu Duponchelle 2015-02-06 15:27:46 UTC
(In reply to comment #3)
> (From update of attachment 296269 [details] [review])
> The locking is not needed here (mini object writability).

True

> This would also require bindings-friendly API then.

Yep, was thinking we could add it as needed, as long as reading is possible through the bindings.

> Why make it generic though? I think it should be message-specific.

Not sure what you mean here?

> And if we do
> it like this, we may just as well re-use the GstStructure from
> gst_message_get_structure().

The advantage of this approach is to guarantee "standard" fields won't get overwritten.
Comment 5 Mathieu Duponchelle 2015-02-10 16:36:03 UTC
As discussed on IRC, there are concerns over the exact API that we should expose.

On one hand, Sebastian said he wasn't comfortable with exposing implementation details such as GstStructure directly.

On the other hand, if we hide away the GstStructure, then it means we have to expose a number of bindings-friendly methods, basically duplicating GstStructure's API.

The other concern was that we might not want to make it generic, and only allow setting and getting details on error messages.

My personal opinion is that we already expose the GstStructure of the message, so exposing details as a GstStructure would be consistent with this.

Regarding genericity, as thiago said we can start with only error messages, then if need be make the API generic.

I think we all agree that this would be a valuable addition, and it kind of blocks my work to let souphttpsrc pass more information about failure codes around, it would be great if we could go ahead and decide on something, implementation itself shouldn't be much work.
Comment 6 Mathieu Duponchelle 2015-02-13 17:01:35 UTC
Created attachment 296788 [details] [review]
message: expose an error details structure.

[API]: gst_message_set_error_details
[API]: gst_message_get_error_details

This API works with GstStructures directly, as Sebastian advised me to
go ahead with this for now.
Comment 7 Mathieu Duponchelle 2015-02-16 13:12:02 UTC
Created attachment 296921 [details] [review]
message: expose error details.

[API] : gst_message_get_error_details
[API] : gst_message_set_error_details
[API] : gst_message_get_error_detail
[API] : gst_message_set_error_detail

+ Add a test
Comment 8 Tim-Philipp Müller 2017-07-14 15:27:29 UTC

*** This bug has been marked as a duplicate of bug 756806 ***