This is basically the equivalent to gst_segment_to_position(), which does the same for a running time. I would rename gst_segment_to_position() to gst_segment_position_from_running_time() and deprecate the old one to prevent confusion and make it easier to find.
We could also call them gst_segment_running_time_to_position() (and s/running_time/stream_time/), not sure if I prefer "from" or "to" in the function name :)


Additionally all these functions need some unit tests. gst_segment_to_running_time() and gst_segment_to_position(), gst_segment_to_stream_time() and gst_segment_position_from_stream_time().
And especially gst_segment_to_running_time_full() needs some tests for the special cases added in 1.6.

The tests for the ->position conversion functions could check if position->(stream_time|running_time)->position gives the same values again.

So, basically one commit from the attached patch (?), one to deprecate gst_segment_to_position() (and replace it in a few places where it's used, basesink.c IIRC), and one to add unit tests for position_from_stream_time(), and then it can be pushed? :) Or what do you think?

Sounds like a plan :)

Created attachment 310511 [details] [review]
Patch to add unit tests

Created attachment 310513 [details] [review]
Replace gst_segment_to_position with gst_segment_position_from_running_time

Done, please see the new attachments :)

Review of attachment 310450 [details] [review]:

Thanks, looks mostly good. Just some cosmetic changes :)

::: gst/gstsegment.c
@@ +399,3 @@
     guint64 position)
 {
+  guint64 stream_time, start, stop, time;

Please put the change to the existing function where you rename this variable into a different patch. That's cleanup :)

@@ +463,3 @@
+ *
+ * Returns: the position in the segment for @stream_time. This function returns
+ * -1 when @stream_time is -1 or when it is not inside @segment.

Add "Since: 1.8" here

Review of attachment 310511 [details] [review]:

Looks good

Review of attachment 310513 [details] [review]:

And another minor cosmetic change

::: gst/gstsegment.c
@@ +750,1 @@
  * -1 when @running_time is -1 or when it is not inside @segment.

Add "Since: 1.8" here

Created attachment 312138 [details] [review]
Replace gst_segment_to_position with gst_segment_position_from_running_time

Created attachment 312139 [details] [review]
Added unit tests for gst_segment_position_from_stream_time

Created attachment 312140 [details] [review]
gst_segment_to_stream_time: Rename result to stream_time

Created attachment 312141 [details] [review]
Added gst_segment_position_from_stream_time

Done, new patches attached :)

commit 44ba1565d932952979959b8b17c3daf635db9bd6
Author: Vivia Nikolaidou <vivia@ahiru.eu>
Date:   Wed Sep 2 17:58:38 2015 +0300

    segment: Replaced gst_segment_to_position with gst_segment_position_from_running_time
    
    gst_segment_to_position might cause confusion, especially with the addition of
    gst_segment_position_from_stream_time . Deprecated gst_segment_to_position
    now, and replaced it with gst_segment_position_from_running_time.
    
    Also added unit tests.

commit 572c7c6b9a0d5d52062a8523ad0b26712b726994
Author: Vivia Nikolaidou <vivia@ahiru.eu>
Date:   Wed Sep 2 17:38:25 2015 +0300

    segment: Added unit tests for gst_segment_position_from_stream_time

commit 26ef44f17ce0a63907337eec8f34dc34a10a80b7
Author: Vivia Nikolaidou <vivia@toolsonair.com>
Date:   Fri Sep 25 15:57:16 2015 +0300

    segment: gst_segment_to_stream_time: Renamed 'result' to 'stream_time'
    
    Renamed the "result" variable to "stream_time" for better readability.

commit c0d4b1b64680bd8016ce6a69221b1564e392d59b
Author: Vivia Nikolaidou <vivia@toolsonair.com>
Date:   Fri Sep 25 15:56:45 2015 +0300

    segment: Added gst_segment_position_from_stream_time()
    
    gst_segment_position_from_stream_time() will convert stream time into a
    position in the segment so that gst_segment_to_stream_time() with that
    position returns the same stream time. It will return -1 if the stream time
    given is not inside the segment.