jhaag75/xapi-videojs

playedSegments - wrong start?

github-canni opened this issue · 4 comments

Currently in the reference implementation the function fixed_play_time(time) returns played_segments_segment_end if Math.abs(played_segments_segment_end - time) < 1

example:

  • play segment: 16.268 _ 16.873
  • seek to 15.973
  • play to 16.183
  • pause
    results in: https://w3id.org/xapi/video/extensions/played-segments: "16.268[.]16.873[,]16.873[.]16.183"
    expected: https://w3id.org/xapi/video/extensions/played-segments: "16.268[.]16.873[,]15.973[.]16.183"

probable reason: 16.873 - 15.973 = 0.9 < 1

What is the reason for condition Math.abs(played_segments_segment_end - time) >= 1?

Probable fix?

function fixed_play_time(time) {
   if (played_segments_segment_end == null) {
        return time;
   }
   var diff = time - played_segments_segment_end;
   if( diff <= 1 && diff > 0) {
        return played_segments_segment_end;
    } else {
        return time;
   }
}

It was a conscious choice to ignore negligible gaps in played segments. Little gaps like these can happen due to player inaccuracies too, like if you pause and play at the same time, time might be slightly different. However, minor gaps will end up causing the total time to be be less than video time by a fraction of second and completion might not be marked.

Hi, i get that small gaps could/should be ignored (which seems pretty reasonable to me)

What I rather meant was, that the logged segments (in my point of view) could be misleading due to that construction. If a user is seeking too close to the left of current time, the logged segment of reference implementation would semantically mean "watching the video in rewind/reverse-mode" / "watching that specific segment backwards". Seeking too close could easily be done due to some players quick rewind function.

The previous code then constructs a played segment where the right point of the interval is smaller than the left (which means in my interpretation that the user is watching a segment "backwards" (which is not the case), like from the example above:
16.873[.]16.183"

Or did I get the data model concept wrong and played segments should be recorded or are allowed to be recorded like that?

That would probably allow logging "reverse"-played segments of videos (like an itentional reverse-play of media which probably is a rather special use case). But then the implementation would conflict as you cannot tell the difference between a "reverse"-play and the "regular-play" as the logged played segment from the example above.

In terms for "completion" it should always be better to use currentTime() as played_segments_segment_start for that case here, as long as currentTime() is smaller than the played_segments_segment_end. For gaps on the right side the last played_segments_segment_end could be used as played_segments_segment_start (as previously intended and which is still the case with the suggested "fix")

Thanks for getting back. Generally speaking your reference from the end and start point being reduced to 0 makes perfectly sense for me (as long as the play "continues" somewhat after that starting point and if end > start of segment).

The bug mentioned can be reproduced in the demo

If video navigation is as described in this ticket, the starting point of the recorded played segments is wrong.

Example of a recorded statement:
"0[.]1.551[,]16.432[.]16.815[,]16.815[.]16.344"
The actual viewers last segment in this case were: 16.208[.]16.344 but being recorded as 16.815[.]16.344

This could be reviewed in the LRS here.