Felipe Contreras wrote:
> 2010/4/24 Sebastian Dröge <
[hidden email]>:
>
>> On Sat, 2010-04-24 at 13:36 +0300, Felipe Contreras wrote:
>>
>>> About this commit:
>>>
http://cgit.freedesktop.org/gstreamer/gstreamer/commit/?id=3b4aa3f76a0252560151186bbdfb6be4e28880af>>>
>>> The commit message is very bad; it's missing key information: why?
>>>
>>> According to Stefan; gst_element_class_set_details is causing a extra
>>> reloc and pointer copying. While that's probably true, the overhead is
>>> almost nothing, and only happens once, when the class is being
>>> initialized, right? It's ABI breakage for no reason.
>>>
>>> If a new API is in the works, that's cool, but since there's no better
>>> API right now, IMO the commit is doing more damage than good.
>>>
>>> FWIW, I'll be removing GST_DISABLE_DEPRECATED instead.
>>>
>> The replacement for gst_element_class_set_details() is
>> gst_element_class_set_details_simple(), which does exactly the same
>> thing but more efficient.
>>
>
> How more efficient? I'm looking at the resulting assembly, and this
> code actually looks more efficient:
>
> gst_element_class_set_details(element_class, &(const GstElementDetails) {
> .longname = "basic video decoder",
> .klass = "Codec/Decoder/Video",
> .description = "Decodes video",
> .author = "John Doe",
> });
>
> If that looks too confusing, this is exactly the same:
>
> const GstElementDetails details = {
> .longname = "basic video decoder",
> .klass = "Codec/Decoder/Video",
> .description = "Decodes video",
> .author = "John Doe",
> };
>
> gst_element_class_set_details(element_class, &details);
>
> Personally, I find both versions to be more readable than the
> gst_element_class_set_details_simple() alternative.
>
See attached demo:
$ ls -al setdetails?
-rwxr-xr-x 1 ensonic ensonic 8389 2010-04-26 18:01 setdetails0
-rwxr-xr-x 1 ensonic ensonic 8372 2010-04-26 18:01 setdetails1
-rwxr-xr-x 1 ensonic ensonic 8372 2010-04-26 18:01 setdetails2
Your version is bigger.
Stefan
>
>> gst_element_class_set_details() is still there unless you define
>> GST_REMOVE_DEPRECATED but will be hidden from the headers if you define
>> GST_DISABLE_DEPRECATED.
>>
>
> I'm aware of that.
>
>
>> That's the same as was done to many other functions in the past too,
>> e.g. gst_element_get_pad() or gst_atomic_int_set().
>>
>
> Indeed, but there was a good reason for those. I don't see any for this.
>
> Cheers.
>
>
/* test code size and relocs
*
* gcc `pkg-config --cflags --libs glib-2.0` -O2 -DV0 setdetails.c -o setdetails0
* gcc `pkg-config --cflags --libs glib-2.0` -O2 -DV1 setdetails.c -o setdetails1
* gcc `pkg-config --cflags --libs glib-2.0` -O2 -DV2 setdetails.c -o setdetails2
*
* ls -al setdetails?
*
* objdump -p setdetails? | grep REL
*/
#include <glib.h>
typedef struct {
gchar *longname,*klass,*description,*author;
} GstElementDetails;
typedef struct {
GstElementDetails *details;
} GstElementClass;
#ifdef V0
const GstElementDetails details = {
.longname = "basic video decoder",
.klass = "Codec/Decoder/Video",
.description = "Decodes video",
.author = "John Doe",
};
void
gst_element_class_set_details(GstElementClass *c, const GstElementDetails *details)
{
g_free(c->details->longname);
g_free(c->details->klass);
g_free(c->details->description);
g_free(c->details->author);
c->details->longname = g_strdup(details->longname);
c->details->klass = g_strdup(details->klass);
c->details->description = g_strdup(details->description);
c->details->author = g_strdup(details->author);
}
gint
main (gint argc, gchar *argv[])
{
GstElementClass c = {NULL,};
GstElementDetails cd;
c.details=&cd;
gst_element_class_set_details(&c, &details);
return 0;
}
#endif
#ifdef V1
void
gst_element_class_set_details_simple(GstElementClass *c, const gchar *longname,
const gchar *klass, const gchar *description, const gchar *author)
{
const GstElementDetails details = {
(gchar *) longname, (gchar *) klass, (gchar *) description, (gchar *) author
};
g_free(c->details->longname);
g_free(c->details->klass);
g_free(c->details->description);
g_free(c->details->author);
c->details->longname = g_strdup(details.longname);
c->details->klass = g_strdup(details.klass);
c->details->description = g_strdup(details.description);
c->details->author = g_strdup(details.author);
}
gint
main (gint argc, gchar *argv[])
{
GstElementClass c = {NULL,};
GstElementDetails cd;
c.details=&cd;
gst_element_class_set_details_simple(&c, "basic video decoder", "Codec/Decoder/Video", "Decodes video", "John Doe");
return 0;
}
#endif
#ifdef V2
void
gst_element_class_set_details_simple(GstElementClass *c, const gchar *longname,
const gchar *klass, const gchar *description, const gchar *author)
{
g_free(c->details->longname);
g_free(c->details->klass);
g_free(c->details->description);
g_free(c->details->author);
c->details->longname = g_strdup(longname);
c->details->klass = g_strdup(klass);
c->details->description = g_strdup(description);
c->details->author = g_strdup(author);
}
gint
main (gint argc, gchar *argv[])
{
GstElementClass c = {NULL,};
GstElementDetails cd;
c.details=&cd;
gst_element_class_set_details_simple(&c, "basic video decoder", "Codec/Decoder/Video", "Decodes video", "John Doe");
return 0;
}
#endif
------------------------------------------------------------------------------
_______________________________________________
gstreamer-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/gstreamer-devel
Locked
signature.asc (205 bytes)