xapi-videojs icon indicating copy to clipboard operation
xapi-videojs copied to clipboard

playedSegments - wrong start?

Open github-canni opened this issue 2 years ago • 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;
   }
}

github-canni avatar Apr 22 '22 22:04 github-canni

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.

liveaspankaj avatar Apr 23 '22 17:04 liveaspankaj

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")

github-canni avatar Apr 25 '22 09:04 github-canni

So, probably we are looking at different things.

  • 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"

I was referring to the difference between the ending part of one segment and starting time of the next segment. That is being reduced to 0 difference and that is what that part of code might be doing ( I haven't re-looked at it)

However, the ending being smaller than starting, is certainly a bug. Probably due to forward adjustment in the starting point.

Going back would start a new segment. Rewind/Reverse play was not a use case for the specification, and is not supported by the specification.

Regards, Pankaj Agrawal

On Mon, Apr 25, 2022 at 3:02 PM github-canni @.***> wrote:

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 player quick rewind function (+/-10s).

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.

— Reply to this email directly, view it on GitHub https://github.com/jhaag75/xapi-videojs/issues/30#issuecomment-1108323823, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAN73GKBNP4BYNRU4MJ7PD3VGZRBTANCNFSM5UDRKIJQ . You are receiving this because you commented.Message ID: @.***>

liveaspankaj avatar Apr 25 '22 18:04 liveaspankaj

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.

github-canni avatar Apr 26 '22 06:04 github-canni