Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
rfc(feature): Video replay envelope #129
rfc(feature): Video replay envelope #129
Changes from 16 commits
8ccece4
985c32c
7691ba5
26ee3cc
b4d3723
ae41a9d
a653d99
5574b37
14f517b
3c60f03
75aba68
46c72b8
ab3db0d
8843512
c2e004e
e22a091
cb938e5
e9488b1
78987fb
79ef51f
1e20ad4
2424b31
70e1b05
6260eaa
d4dd02a
fc78a2b
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@billyvg How would the player like to be notified that it should download video data? A
type
value on the replay? The video events in the RRWeb? Infer it from the replay's platform? Something else?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@cmanallen Thinking of using the rrweb video event
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@cmanallen just to confirm: this
segment_id
and the ones in theReplay
item andReplayRecording
's payload should be the same, correct? Shall we also document it here for transparency?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
They should be the same but if you sent a segment 2 video with a segment 3 rrweb event nothing would break. So long as the rrweb video event on segment 2 accurately reflects the segment 2 video. The events are not dependent on one another for processing. They are just associated to the same "segment_id" metadata.