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

[Wim Taymans]

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.

> 
> 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.

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
-- 
Wim Taymans <wim at fluendo.com>