Clock.getCalibration sets the calibration, doesn't retrieve it.
Gamebuster19901 opened this issue · 4 comments
First, if this is intentional the javadoc is wrong. None of the parameters can be null because they're primitives.
I believe the method is supposed to call GSTCLOCK_API.gst_clock_get_calibration
instead of GSTCLOCK_API.gst_clock_set_calibration
because this is the getter not the setter. This would allow null to be passed as the parameters would be arrays instead of primitives...
I believe this method is intended to modify arrays that a developer passes into the method so they can retrieve the calibration.
gst1-java-core/src/org/freedesktop/gstreamer/Clock.java
Lines 183 to 196 in 37e7555
Er, yes, that's definitely not right! Inherited from https://github.com/gstreamer-java/gstreamer-java/blob/master/gstreamer-java/src/org/gstreamer/Clock.java#L195 (ClockTime to long change makes no difference here). Need to deprecate that and work out how we can actually best support the required functionality.
OK, looks like best option is to just provide a Clock.Calibration getCalibration()
method and ignore the ability to pass in nulls entirely? Good shout for conversion to record in future.
Honestly, I'd recommend adding the following methods
getInternalTime()
getExternalTime()
getRate()
setInternalTime()
setExternalTime() //if this actually makes sense, I don't actually know the difference between external and internal time.
setRate()
If this was implemented in Java I'd probably agree. But we generally try to mirror upstream method names as much as possible. I'm also unsure of usage of this method. How often is someone likely to only need one of the values? Crossing the Java<>Native boundary only once is probably preferable. Looking at other bindings, C# has out parameters, so no use(!), and Rust has tuple return types, and also ignores ability to pass in null. So a small data class (in future a record) would be my preferred way to map this.