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

Update https://w3c.github.io/mediacapture-record/#dom-blobeventinit-timecode. #223

Merged
merged 2 commits into from
Oct 31, 2024

Conversation

handellm
Copy link
Contributor

@handellm handellm commented Sep 5, 2024

The spec text was unclear. From offline discussion with the original spec author it's believed that the initial intent of the spec language was meant to convey that the first produced timecode would contain an absolute timestamp. However, it's not clear what use case is unlocked by that, as the user can construct their own absolute timestamp with performance.now() on event reception. This ignores the fact that the event handler can execute delayed, but since it's not clear what problem this fixes, this PR simply makes the timestamp sequence start with zero instead.

Fixes #222.


Preview | Diff

…imecode.

The spec text was unclear. From offline discussion with the original spec author
it's believed that the intent of the spec language was meant to convey
that the first produced timecode would contain an absolute timestamp. However,
it's not clear what usecase is unlocked by that, as the user can construct
their own absolute timestamp with performance.now() on event reception. This
ignores the fact that the event handler can execute delayed, but since it's
not clear what problem this fixes, this PR simply makes the timestamp sequence
start with zero instead.

Fixes w3c#222.
@bradisbell
Copy link

From offline discussion with the original spec author it's believed that the initial intent of the spec language was meant to convey that the first produced timecode would contain an absolute timestamp.

@handellm Actually, that makes sense... that way, the user can know when recording started.

We know from MediaRecorder's start event roughly what browser time the recorder started, but we don't know the browser timestamp of the first recorded audio/video data.

So, if a user clicks record and we start recording a MediaStream with a MediaRecorder, with a timeslice of 5_000...

  • 2024-09-05 15:19:32.500Z We ask MediaRecorder to start
  • 2024-09-05 15:19:33.000Z MediaRecorder start fires
  • 2024-09-05 15:19:33.100Z The next video frame comes in... this is the first frame since the MediaRecorder has started, and suppose it's a screen recording so frames are random and sparse.
  • ... more frames here ...
  • 2024-09-05 15:19:38.222Z MediaRecorder dataavailable fires. This is the first chunk.

Now, in this sequence we know when we asked MediaRecorder to start, but this isn't a good timestamp to use for the actual start time because it can take quite a bit of time for MediaRecorder to get moving, especially with hardware codecs involved that need to be woken up and initialized. MediaRecorder tells us when it starts, but that isn't a good timestamp either because it isn't relative to the MediaStream. There could be several seconds of nothing before the next frame. Only when dataavailable fires do we know what was captured and when.

I might just be hopeful here, but if the original intent in the spec was to give a timestamp for that first recorded data... maybe we should find a way to make that clearer rather than abandon it? Or, if this is an expansion of the API and not how browsers work today, then maybe this is worthy of a future addition?

Copy link
Member

@jan-ivar jan-ivar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 with nit

Co-authored-by: Harald Alvestrand <[email protected]>
@handellm
Copy link
Contributor Author

@bradisbell - if the time of arrival of the first frame is to be timestamped, there are better ways of observing this such as using MediaStreamTrackProcessor/generator. I'm going to merge this PR now and follow up with WPTs and Chromium change.

@alvestrand
Copy link
Contributor

Added "editors can integrate" label to PR too (it was wrongly applied to just the issue).

chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Oct 1, 2024
…BlobEvent.

spec, https://w3c.github.io/mediacapture-record/#dom-blobevent-timecode, said that the timecode is time delta from the first BlobEvent of this recorder.

Here are issues and its discussion.
w3c/mediacapture-record#222
w3c/mediacapture-record#223

Bug: 40247788
Change-Id: Ic9c74299230b9839578ca317fff056e7a1b26dc6
aarongable pushed a commit to chromium/chromium that referenced this pull request Oct 1, 2024
…BlobEvent.

spec, https://w3c.github.io/mediacapture-record/#dom-blobevent-timecode, said that the timecode is time delta from the first BlobEvent of this recorder.

Here are issues and its discussion.
w3c/mediacapture-record#222
w3c/mediacapture-record#223

Bug: 40247788
Change-Id: Ic9c74299230b9839578ca317fff056e7a1b26dc6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5631009
Commit-Queue: Sunggook Chue <[email protected]>
Reviewed-by: Markus Handell <[email protected]>
Reviewed-by: Gabriel Brito <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1362716}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Oct 1, 2024
…BlobEvent.

spec, https://w3c.github.io/mediacapture-record/#dom-blobevent-timecode, said that the timecode is time delta from the first BlobEvent of this recorder.

Here are issues and its discussion.
w3c/mediacapture-record#222
w3c/mediacapture-record#223

Bug: 40247788
Change-Id: Ic9c74299230b9839578ca317fff056e7a1b26dc6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5631009
Commit-Queue: Sunggook Chue <[email protected]>
Reviewed-by: Markus Handell <[email protected]>
Reviewed-by: Gabriel Brito <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1362716}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Oct 2, 2024
…BlobEvent.

spec, https://w3c.github.io/mediacapture-record/#dom-blobevent-timecode, said that the timecode is time delta from the first BlobEvent of this recorder.

Here are issues and its discussion.
w3c/mediacapture-record#222
w3c/mediacapture-record#223

Bug: 40247788
Change-Id: Ic9c74299230b9839578ca317fff056e7a1b26dc6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5631009
Commit-Queue: Sunggook Chue <[email protected]>
Reviewed-by: Markus Handell <[email protected]>
Reviewed-by: Gabriel Brito <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1362716}
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request Oct 10, 2024
… from the first chunk of the BlobEvent., a=testonly

Automatic update from web-platform-tests
Make BlobEvent::timecode as a difference from the first chunk of the BlobEvent.

spec, https://w3c.github.io/mediacapture-record/#dom-blobevent-timecode, said that the timecode is time delta from the first BlobEvent of this recorder.

Here are issues and its discussion.
w3c/mediacapture-record#222
w3c/mediacapture-record#223

Bug: 40247788
Change-Id: Ic9c74299230b9839578ca317fff056e7a1b26dc6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5631009
Commit-Queue: Sunggook Chue <[email protected]>
Reviewed-by: Markus Handell <[email protected]>
Reviewed-by: Gabriel Brito <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1362716}

--

wpt-commits: 832a0a52922181cc6855e912ffe37f5837ddd08c
wpt-pr: 48415
i3roly pushed a commit to i3roly/firefox-dynasty that referenced this pull request Oct 12, 2024
… from the first chunk of the BlobEvent., a=testonly

Automatic update from web-platform-tests
Make BlobEvent::timecode as a difference from the first chunk of the BlobEvent.

spec, https://w3c.github.io/mediacapture-record/#dom-blobevent-timecode, said that the timecode is time delta from the first BlobEvent of this recorder.

Here are issues and its discussion.
w3c/mediacapture-record#222
w3c/mediacapture-record#223

Bug: 40247788
Change-Id: Ic9c74299230b9839578ca317fff056e7a1b26dc6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5631009
Commit-Queue: Sunggook Chue <[email protected]>
Reviewed-by: Markus Handell <[email protected]>
Reviewed-by: Gabriel Brito <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1362716}

--

wpt-commits: 832a0a52922181cc6855e912ffe37f5837ddd08c
wpt-pr: 48415
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this pull request Oct 15, 2024
… from the first chunk of the BlobEvent., a=testonly

Automatic update from web-platform-tests
Make BlobEvent::timecode as a difference from the first chunk of the BlobEvent.

spec, https://w3c.github.io/mediacapture-record/#dom-blobevent-timecode, said that the timecode is time delta from the first BlobEvent of this recorder.

Here are issues and its discussion.
w3c/mediacapture-record#222
w3c/mediacapture-record#223

Bug: 40247788
Change-Id: Ic9c74299230b9839578ca317fff056e7a1b26dc6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5631009
Commit-Queue: Sunggook Chue <sunggchmicrosoft.com>
Reviewed-by: Markus Handell <handellmgoogle.com>
Reviewed-by: Gabriel Brito <gabrielbritomicrosoft.com>
Cr-Commit-Position: refs/heads/main{#1362716}

--

wpt-commits: 832a0a52922181cc6855e912ffe37f5837ddd08c
wpt-pr: 48415

UltraBlame original commit: bf2a71f0b8e8ed32d72fdb4add025cb198a9e6dc
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this pull request Oct 15, 2024
… from the first chunk of the BlobEvent., a=testonly

Automatic update from web-platform-tests
Make BlobEvent::timecode as a difference from the first chunk of the BlobEvent.

spec, https://w3c.github.io/mediacapture-record/#dom-blobevent-timecode, said that the timecode is time delta from the first BlobEvent of this recorder.

Here are issues and its discussion.
w3c/mediacapture-record#222
w3c/mediacapture-record#223

Bug: 40247788
Change-Id: Ic9c74299230b9839578ca317fff056e7a1b26dc6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5631009
Commit-Queue: Sunggook Chue <sunggchmicrosoft.com>
Reviewed-by: Markus Handell <handellmgoogle.com>
Reviewed-by: Gabriel Brito <gabrielbritomicrosoft.com>
Cr-Commit-Position: refs/heads/main{#1362716}

--

wpt-commits: 832a0a52922181cc6855e912ffe37f5837ddd08c
wpt-pr: 48415

UltraBlame original commit: bf2a71f0b8e8ed32d72fdb4add025cb198a9e6dc
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this pull request Oct 15, 2024
… from the first chunk of the BlobEvent., a=testonly

Automatic update from web-platform-tests
Make BlobEvent::timecode as a difference from the first chunk of the BlobEvent.

spec, https://w3c.github.io/mediacapture-record/#dom-blobevent-timecode, said that the timecode is time delta from the first BlobEvent of this recorder.

Here are issues and its discussion.
w3c/mediacapture-record#222
w3c/mediacapture-record#223

Bug: 40247788
Change-Id: Ic9c74299230b9839578ca317fff056e7a1b26dc6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5631009
Commit-Queue: Sunggook Chue <sunggchmicrosoft.com>
Reviewed-by: Markus Handell <handellmgoogle.com>
Reviewed-by: Gabriel Brito <gabrielbritomicrosoft.com>
Cr-Commit-Position: refs/heads/main{#1362716}

--

wpt-commits: 832a0a52922181cc6855e912ffe37f5837ddd08c
wpt-pr: 48415

UltraBlame original commit: bf2a71f0b8e8ed32d72fdb4add025cb198a9e6dc
@youennf youennf merged commit dbe375f into w3c:main Oct 31, 2024
2 checks passed
github-actions bot added a commit that referenced this pull request Oct 31, 2024
SHA: dbe375f
Reason: push, by youennf

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

MediaRecorder BlobEvent timecode clarification
6 participants