RFC - ClockTime: don't limit Rust type system expressiveness

Current Status

Purpose

ClockTime provides an explicit and compile-time checked time value.

Optional ClockTime

ClockTime is defined as:

#[derive(...)]
pub struct ClockTime(pub Option<u64>);

This allows representing an undefined ClockTime (which value in C is GST_CLOCK_TIME_NONE and can be checked using GST_CLOCK_TIME_IS_VALID). In Rust, we can write:

fn compute_delay(&mut self) -> bool {
    let mut pts = ClockTime::none();

    // Compute `pts`
    [...]

    if pts.is_none() {
        // not available yet
        return false;
    }

    [...]
    // self.delay: Clocktime
    self.delay = pts - now;

    true
}

The delay computation behaves as expected. This is made possible thanks to the operators being defined as part of the GenericFormattedValue::Time definition.

More examples of supported arithmetic operators:

let lhv = ClockTime::from_mseconds(20);
let rhv = ClockTime::from_mseconds(10);
assert_eq!(lhv - rhv, ClockTime::from_mseconds(10));

let lhv = ClockTime::from_mseconds(20);
let rhv = ClockTime::none();
assert_eq!(lhv - rhv, ClockTime::none());

let lhv = ClockTime::from_mseconds(10);
let rhv = ClockTime::from_mseconds(20);
assert_eq!(lhv - rhv, ClockTime::none());

Error Handling

In Rust, we could be inclined to write the above compute_delay function like this:

fn compute_delay(&mut self) -> Result<(), DelayError> {
    let mut pts = ClockTime::none();

    // Compute `pts`
    [...]

    let pts = pts.ok_or(DelayError::PtsNotAvailable)?;

    [...]
    // self.delay: Clocktime
    // Warning: this doesn't work!
    self.delay = pts - now;
    
    Ok(())
}

... but then pts becomes a u64 so we loose the ClockTime type. The solution would be to re-build the ClockTime from the value, which should be zero-cost, but not very user-friendly:

    self.delay = ClockTime::from_nseconds(pts) - now;

Mandatory Argument

Some functions which use a ClockTime as an argument require the argument to be mandatory. For instance:

pub fn new_gap<'a>(timestamp: ::ClockTime, duration: ::ClockTime) -> GapBuilder<'a>;

The timestamp argument is mandatory here, but the caller can pass ClockTime::none() which causes gst_event_new_gap to return NULL. Ex:

let gap_evt = Event::new_gap(ClockTime::none(), ClockTime::none()).build();

... panics with assertion failed: !ptr.is_null().

Since Rust type system allows specifying such constraints, it would be valuable to differentiate between a mandatory and an optional ClockTime.

Return Value

When receiving a Gap event, the user might expect the timestamp value to be defined:

match event.view() {
    EventView::Gap(e) => {
        let (timestamp, duration) = e.get();
        assert!(timestamp.is_some());
        [...]
    }
    _ => (),
}

It should be possible to use timestamp immediately without checking the value's validity. The gst_event_new_gap C function guarantees that the timestamp is valid. Unfortunately they are other functions such as gst_event_new_latency which I suspect should do the same but don't.

Proposal

In order to accommodate the use cases identified in previous paragraph, I propose to:

• Change ClockTime definition to ClockTime(u64). The getters would return the actual u64 value, after scaling.
• Use Option<ClockTime> in arguments when appropriate, i.e. when the value can be undefined.
• Use ClockTime in arguments when appropriate, i.e. when the value must be defined.
• Use Option<ClockTime> in return values when appropriate or necessary, i.e. when the value can be undefined (see also below).
• Use ClockTime in return values when appropriate & possible, i.e. when the C code actually checks that the mandatory value is defined.
• Implement arithmetic operators for (Option<ClockTime>, ClockTime) and (ClockTime, Option<ClockTime>). Due to the orphan rule, we can't implement the operators for (Option<ClockTime>, Option<ClockTime>). The user could either check that at least one of the member is defined beforehand or use the Option::map function.
• Implement ClockTime::from_* as const fn so that users can define constants, e.g. for default setting values.

Impact on Other Projects

These changes require modifications in the following projects.

glib

We need to implement the following traits on Option<ClockTime>:

• ToGlib
• FromGlib<gst_sys::GstClockTime>
• glib::value::SetValue

These traits can't be implemented in gstreamer-rs because of the orphan rule. It should be possible to define new traits in glib that would be implemented on ClockTime to allow auto-implementing the traits in the list.

gir

ClockTime appears in many auto-generated files. Gir files should be updated, and possibly gir itself. There might be an increase in manually generated files.

Other gstreamer-rs members and gst-plugins-rs

Updates required to use the modified API.

GStreamer

We should probably audit the C code to make sure the GstClockTime arguments are always checked for validity when they are mandatory.