On closer look...

void
gst_element_set_clock (GstElement * element, GstClock * clock)
{
  GstElementClass *oclass;
                                                                                
  g_return_if_fail (GST_IS_ELEMENT (element));
                                                                                
  oclass = GST_ELEMENT_GET_CLASS (element);
                                                                                
  if (oclass->set_clock)
    oclass->set_clock (element, clock);
                                                                                
  gst_object_replace ((GstObject **) & element->clock, (GstObject *) clock);
}

In gstelement.c. I think the code is correct. We don't need to call the parent
in videosink.c.

Ronald,

depends on what the "object contract" is.

In general object orientation practice the pact is that if a method is
overridden then the base class method implementation is not invoked. If it needs
to be invoked, then this is explitely stated and is the responsibility of the
overriding implementation.

Current use of GstVideoSink by the plugins seem to assume this pact is valid and 
no call to the superclass method is necessary.

In other words, the plugins, internally, change the clock calling the
GstVideoSink clock setter, not calling gst_element_set_clock. And this setter
does not call gst_element_set_clock either.

We are dealing with an object oriented paradigm implemented in C, and C is not
natively and OO language.

The code above is really doing two different things. It's a convenient wrapper
to the call of a method of GstElement and it's the implementation of that method
for the GstElement class. 

Nice. But not done according to the ordinary object paradigm. The method is
overridable. There's no guarantee (unless explicitelly defined in the docs) that
this is ever called. Also there's no clear way to call the GstElement
implementation as this is actually mixed with the wrapper.

The net result is that the clock setting is correct if we call
gst_element_se_clock, but not if we call the GstVideoSink implementatin directly
(and that is what video sinks do and it's correct according to OO paradigm)
because the gst_object_replace is not called and the "outer" clock member is not
set. Therefore the clock getter (and that's a macro) does not return the clock.

You should never call the videosink implementation directly. It is intended to
be a virtual function. Compare other functions that are virutally overrideable.
They all are called class->this_name, the calling function (what apps call) is
gst_something_this_name, the default implementation (if any) is called
gst_something_this_name_func and subclass implementations are called
gst_subclass_this_name. This may be a bit weird, but we've done it for years and
it makes a lot of sense if yuo think about it.

at best, it's badly documented, and I'll do better in 0.9. Also, the actions in
gst_element_set_clock() should indeed be done in the default virtual function
implementation. Those are all imperfections, which I will fix in 0.9 (where we
rewrite all this), but not now. Hence the 'NOBUG' mark.