On IRC it was mentioned that the clock waiting and "clock not ready" stuff should be merged into GstClock instead, as we have the same problem with other clocks and something generic to handle this would be nice.

Proposed API:
flag:   GST_CLOCK_FLAG_CAN_BE_NOT_READY = (1<<6)
signal: void ready-state-changed(GstClock *clock, gboolean ready);
method: gboolean gst_clock_wait_ready(GstClock *clock, GstClockTime timeout);
        gboolean gst_clock_is_ready(GstClock *clock);


Comments?

I think the flag and 'readiness' is the same as the network clock - at startup time, the clock doesn't know what time to report until it has some observations of the master clock.

Maybe with that in mind, maybe something like

GST_CLOCK_FLAG_NEEDS_STARTUP_SYNC and gst_clock_wait_for_sync() gst_clock_is_synced()

?

I like that, let's go for that :)

Created attachment 304485 [details] [review]
clock: Add GST_CLOCK_FLAG_NEEDS_STARTUP_SYNC and related API

gst_clock_wait_for_sync(), gst_clock_is_synced() and gst_clock_set_synced()
plus a signal to asynchronously wait for the clock to be synced.

This can be used by clocks to signal that they need initial synchronization
before they can report any time, and that this synchronization can also get
completely lost at some point. Network clocks, like the GStreamer
netclientclock, NTP or PTP clocks are examples for clocks where this is useful
to have as they can't report any time at all before they're synced.

Review of attachment 304485 [details] [review]:

Works for me!

Comment on attachment 304485 [details] [review]
clock: Add GST_CLOCK_FLAG_NEEDS_STARTUP_SYNC and related API

Looks good to me. Some docs nitpicks:

>+  /**
>+   * GstElement::synced:

GstClock::synced ?

Might be good to mention explicitly that this signal will be emitted from a non-application thread.


>+/**
>+ * gst_clock_wait_for_sync:

Would be good to mention in the docs the 'async' variant (i.e. the signal).

I initially was not so fond of the synced=true/false boolean, but a SyncStatus enum seems to be a bit over the top as well, especially if it only has two values, so I guess it's all good.

Created attachment 304495 [details] [review]
clock: Add GST_CLOCK_FLAG_NEEDS_STARTUP_SYNC and related API

gst_clock_wait_for_sync(), gst_clock_is_synced() and gst_clock_set_synced()
plus a signal to asynchronously wait for the clock to be synced.

This can be used by clocks to signal that they need initial synchronization
before they can report any time, and that this synchronization can also get
completely lost at some point. Network clocks, like the GStreamer
netclientclock, NTP or PTP clocks are examples for clocks where this is useful
to have as they can't report any time at all before they're synced.

All merged, thanks for the reviews :)

Review of attachment 304485 [details] [review]:

Looks good.

One addendum: gst_clock_wait_ready() is a good idea, but should be interruptible. Imagine writing a sink bin that internally sets up a netclientclock or a PTP clock, and waits for a long time (because of a large timeout). It should still be possible then to abort the wait, for example during a PAUSED->READY state change, with a call like gst_clock_abort_wait().

(In reply to Carlos Rafael Giani from comment #10)
> with a call like gst_clock_abort_wait().

That is probably a good case for a GCancellable