Re: [gst-devel] gst_element_class_set_details() gone?

[Stefan K. <en...@ho...>]

Felipe Contreras wrote:
> 2010/4/24 Sebastian Dröge <seb...@co...>:
>   
>> 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.
>
>