[gst-devel] flag macro inconsistence of gstobjec and gstminiobject

[Stefan Kost]

Wim Taymans wrote:
>On Sat, 2005-10-08 at 15:54 +0200, Stefan Kost wrote:
>  
>>hi,
>>
>>was adding more macro docs here and there and notices that
>>
>>gstobject treats flags as bit-numbers:
>>
>>#define GST_FLAGS(obj)                  (GST_OBJECT_CAST (obj)->flags)
>>/* for the flags we double-not to make them comparable to TRUE and FALSE */
>>#define GST_FLAG_IS_SET(obj,flag)       (!!(GST_FLAGS (obj) & (1<<(flag))))
>>#define GST_FLAG_SET(obj,flag)          (GST_FLAGS (obj) |= (1<<(flag)))
>>#define GST_FLAG_UNSET(obj,flag)        (GST_FLAGS (obj) &= ~(1<<(flag)))
>>
>>gstminiobject treats them as masks:
>>#define GST_MINI_OBJECT_FLAGS(obj)  (GST_MINI_OBJECT(obj)->flags)
>>#define GST_MINI_OBJECT_FLAG_IS_SET(obj,flag)       
>>(GST_MINI_OBJECT_FLAGS(obj) & (flag))
>>#define GST_MINI_OBJECT_FLAG_SET(obj,flag)          
>>(GST_MINI_OBJECT_FLAGS (obj) |= (flag))
>>#define GST_MINI_OBJECT_FLAG_UNSET(obj,flag)        
>>(GST_MINI_OBJECT_FLAGS (obj) &= ~(flag))
>>
>>A quick look at gtk shows that they have:
>>#define GTK_WIDGET_FLAGS(wid)          (GTK_OBJECT_FLAGS (wid))
>>#define GTK_WIDGET_SET_FLAGS(wid,flag)      G_STMT_START{
>>(GTK_WIDGET_FLAGS (wid) |= (flag)); }G_STMT_END
>>#define GTK_WIDGET_UNSET_FLAGS(wid,flag)  G_STMT_START{
>>(GTK_WIDGET_FLAGS (wid) &= ~(flag)); }G_STMT_END
>>    
>
>The reason for the bit-numbers is so we can say:
>
>typedef enum
>{
>  GST_OBJECT_DISPOSING   = 0,
>  GST_OBJECT_FLOATING,
>
>  GST_OBJECT_FLAG_LAST   = 4
>} GstObjectFlags;
>
>and then in GstElement.h, since it subclasses GstObject:
>
>typedef enum
>{
>  GST_ELEMENT_LOCKED_STATE      = GST_OBJECT_FLAG_LAST,
>  GST_ELEMENT_IS_SINK,
>  GST_ELEMENT_UNPARENTING,
>  GST_ELEMENT_FLAG_LAST         = GST_OBJECT_FLAG_LAST + 16
>} GstElementFlags;
>
>Changing the number of flags in GstObject.h automatically changes the
>flag
>numbers in all subclasses too that way. It avoids constructs like:
>
>typedef enum
>{
>  GST_OBJECT_DISPOSING   = (1 << 0),
>  GST_OBJECT_FLOATING    = (1 << 1),
>
>  GST_OBJECT_FLAG_LAST   = 4
>} GstObjectFlags;
>
>typedef enum
>{
>  GST_ELEMENT_LOCKED_STATE      = (1 << (GST_OBJECT_FLAG_LAST + 0)),
>  GST_ELEMENT_IS_SINK           = (1 << (GST_OBJECT_FLAG_LAST + 1)),
>  GST_ELEMENT_UNPARENTING       = (1 << (GST_OBJECT_FLAG_LAST + 2)),
>
>  GST_ELEMENT_FLAG_LAST         = GST_OBJECT_FLAG_LAST + 16
>} GstElementFlags;
>
>Where the _FLAG_LAST enum is not really a flag. Could be fixed with a
>#define for the _LAST number.
>
>Or, as wingo suggested:
>
>typedef enum
>{
>  GST_OBJECT_DISPOSING   = (1 << 0),
>  GST_OBJECT_FLOATING    = (1 << 1),
>
>  GST_OBJECT_FLAG_LAST   = (1 << 4)
>} GstObjectFlags;
>
>typedef enum
>{
>  GST_ELEMENT_LOCKED_STATE      = (GST_OBJECT_FLAG_LAST << 0),
>  GST_ELEMENT_IS_SINK           = (GST_OBJECT_FLAG_LAST << 1),
>  GST_ELEMENT_UNPARENTING       = (GST_OBJECT_FLAG_LAST << 2),
>
>  GST_ELEMENT_FLAG_LAST         = (GST_OBJECT_FLAG_LAST << 16)
>} GstElementFlags;
> 
>Which does not seem too ugly. If you really care about this issue you
>can change it to this last suggestion.
>
>  
The last version is how GstMiniObject, GstBuffer and do on do it. I
don't prefer either way. So if others have a preference vote and I will
refactor it one or the other way.

>>Furthermore if an api break can still be done I vote to rename the
>>macros in gstobject from
>>GST_FLAG_* to GST_OBJECT_FLAG
>>
>>    
>
>Agreed.
>  
Okay. So i'll do that change together with the rest when the decission
has been cast.

Stefan
>Wim
>
>  
>>Any conclusion?
>>
>>Stefan
>>
>>
>>
>>-------------------------------------------------------
>>This SF.Net email is sponsored by:
>>Power Architecture Resource Center: Free content, downloads, discussions,
>>and more. http://solutions.newsforge.com/ibmarch.tmpl
>>_______________________________________________
>>gstreamer-devel mailing list
>>gstreamer-devel at lists.sourceforge.net
>>https://lists.sourceforge.net/lists/listinfo/gstreamer-devel
>>