Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Playout with seek #2482

Conversation

ricardo-salgado-tekever
Copy link
Contributor

This pull request adds the capability to seek to a particular timestamp in the playout.
The seek capability works by:

  1. changing the play message, which supports an optional start_at property for the timestamp to start the video at;
  2. adding the seek message, which support a single mandatory timestamp property, for the timestamp to which the video should jump to.

For both cases, timestamp is expressed in microsseconds since the start of the recording files. After seeking, all timestamps and sequence numbers are recalculated, so the webRTC client does not perceive the seek as frame repetitions or jumps.

The sample demo was not changed, so to test just use this command to seek to 5.0sec of video on the javascript console:
recordplay.send({ message : { "request" : "seek", "timestamp" : 5000000 }}, (response) => { console.log(response);})

@lminiero
Copy link
Member

lminiero commented Dec 7, 2020

Can you rebase it on #2481, since there are other changes to the Record&Play plugin there that will probably merged before this?

@ricardo-salgado-tekever
Copy link
Contributor Author

Rebased on #2481.

@lminiero
Copy link
Member

lminiero commented Dec 7, 2020

You need to also change the base in the PR page, as it still lists master.

@ricardo-salgado-tekever ricardo-salgado-tekever changed the base branch from master to record-binarydata December 7, 2020 17:36
@lminiero
Copy link
Member

lminiero commented Dec 7, 2020

Thanks! I'll have a look in the next few days.

Quick question, though: are you taking into account keyframes when seeking? "Brutally" seeking to X seconds will not work with video, since if you jump to 23s and keyframes arrived at seconds 18 and 30, you won't get working video for 7 seconds (that is, until the next keyframe is available in the recording). We did implement seeking on mjr files for a customer, and this is something we had to implement exactly because of that: basically having keyframe times act as "magnets" when you try to seek (so in the example above, turn seek to 23 to either seek to 18 or seek to 30, depending on the policy).

@ricardo-salgado-tekever
Copy link
Contributor Author

ricardo-salgado-tekever commented Dec 7, 2020

I did not account for keyframes. The examples I've been using have enough keyframes that this is atually not a problem.
But I'll look into this.

@ricardo-salgado-tekever
Copy link
Contributor Author

ricardo-salgado-tekever commented Dec 7, 2020

Now accounts for keyframes.
Starts searching at seek location, looking for next available keyframe. It would be easy enough to invert search (search backward) or to make search both ways to see which ever keyframe is closest....

@lminiero lminiero closed this Dec 11, 2020
@lminiero lminiero deleted the branch meetecho:record-binarydata December 11, 2020 11:36
@ricardo-salgado-tekever
Copy link
Contributor Author

This pull request was closed without merging it seems - after record-binarydata was closed.
Am I seeing this wrong?

@lminiero
Copy link
Member

Oh that's probably my fault: I merged the binary recordings branch, and then deleted that branch. Since your PR was based on that, github closed it automatically too, apparently. Apologies for that! I'll reopen, and I'll see if I can rebase it on master from here.

@lminiero
Copy link
Member

Mh, it won't let me reopen it since the branch is gone, and I can't change the base myself apparently: can you see if the option is there for you?

@ricardo-salgado-tekever
Copy link
Contributor Author

Nope. Worse is that I now get a nasty "pre-recieve hook declined" when I push this branch

@lminiero
Copy link
Member

Darn, sorry about that... can you create a new PR from the existing code from a separate branch? I promise I'll be more careful next time 😁

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants