Created attachment 130335 [details] [review]
Patch to fix incorrect casts and use of NULL.

Well done. I would add some more initialize in place such as this, in case gstreamer doesn't do it always:

GstState pending_state; //Choose an arbitary initial enum value - maybe GST_STATE_NULL.
gst_message_parse_state_changed(const_cast<GstMessage*>(gobj()), 0,

 

(In reply to comment #2)
> Well done. I would add some more initialize in place such as this, in case
> gstreamer doesn't do it always:
> 
> GstState pending_state; //Choose an arbitary initial enum value - maybe
> GST_STATE_NULL.
> gst_message_parse_state_changed(const_cast<GstMessage*>(gobj()), 0,

This is possible, although gst_message_parse_state_changed(), which is called by parse*(), checks each argument as below:

void gst_message_parse_state_changed (GstMessage * message, GstState * oldstate, GstState * newstate, GstState * pending)
{
  g_return_if_fail (GST_IS_MESSAGE (message));
  g_return_if_fail (GST_MESSAGE_TYPE (message) == GST_MESSAGE_STATE_CHANGED);

  if (oldstate)
    *oldstate =
        g_value_get_enum (gst_structure_id_get_value (message->structure,
            GST_QUARK (OLD_STATE)));
  /* and so on for other arguments... */
}

Is it therefore redundant to initialise GstState? If not, I will happily revise the patch.

You should never depend on the implementation. Tomorrow it might be different.

Created attachment 130341 [details] [review]
Additionally, initialises GstState to GST_STATE_NULL in parse*() functions.

Yes, I see what you mean about not relying on the implementation.

Thanks for catching this.  Are the int() casts inside the State() casts necessary?

Nevermind.  I've committed the patch w/o the int casts (I think they're not needed when converting between Gst::State and GstState).

Yes, when casting an enum type to another enum (which has the same range), there is no need to cast to an int explicitly. I had to go and read the C++ standard to check this unusual behaviour, though!

Its just that its not done through gstreamermm and for a minute I thought it might be necessary to go through the casts of enums to make sure they are right.  Thanks again.