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

[WebCodecs codec registry] Policy for changes to existing entries #571

Closed
tidoust opened this issue Oct 3, 2022 · 5 comments
Closed

[WebCodecs codec registry] Policy for changes to existing entries #571

tidoust opened this issue Oct 3, 2022 · 5 comments

Comments

@tidoust
Copy link
Member

tidoust commented Oct 3, 2022

As mentioned in #559 (review), there is one requirement mandated by the Process Document for registry definitions that the current spec does not describe:

Define the policy for changes to existing entries, such as

  • whether entries can be deleted or deprecated
  • whether entries can be changed after being published, and what kinds of changes are allowed
  • whether previously-deleted unique identifiers can be re-used, or are reserved indefinitely

As a straw-man proposal, I would suggest:

  • entries can be deprecated but not deleted
  • The codec string may be completed, the public specification may be updated. All changes SHOULD be backward compatible. To be evaluated by the Media WG.
  • identifiers are reserved indefinitely

What do you think?

@dalecurtis
Copy link
Contributor

  • I don't think we'd ever want to deprecate or delete since it's non-normative. A codec wouldn't simply disappear; especially since we require additions to go through the Media WG. I suppose there could be non-technical reasons we have to remove a codec though...
  • Changes can be allowed. I would say the base codec string can't be deleted, but it can be extended. E.g., vp09 to vp09.aa.bb.cc.dd.ee.ff. I expect we'll also see things like configuration dictionaries added for more codecs than avc; though we'll have to think on the spec side how we want those feature detectable. @sandersdan
  • Since no deletion, no reuse.

@sandersdan
Copy link
Contributor

sandersdan commented Oct 3, 2022

we'll have to think on the spec side how we want those feature detectable.

There is only one proposed path for mutating metadata (a VideoFrame construction option), and it would be passively detectable in that metadata that doesn't match a known key would not appear on the constructed VideoFrame.

The primary alternative would be to add an interface that we return from metadata() rather than re-using the dictionary type; that way there would be a JS object available to be introspected.

Edit: for encoder/decoder configurations, isConfigSupported() returns the configuration, this allows apps to check which configuration options were understood.

tidoust added a commit that referenced this issue Oct 5, 2022
This attempts to formalize the policy for changes proposed in:
#571 (comment)
@dalecurtis
Copy link
Contributor

@aboba @padenot did you want to weigh in before we merge @tidoust 's proposal?

@chrisn
Copy link
Member

chrisn commented Oct 5, 2022

The proposal from @tidoust looks good, taking into account #571 (comment), although, as I understand it, adding configuration options would affect the codec-specific registrations rather than the registry (i.e., the mapping document) itself.

The part of @sandersdan's comment on returning an interface or dictionary type from metadata() applies to #559, do we want to change the return type?

@aboba
Copy link
Collaborator

aboba commented Oct 6, 2022

I have reviewed PR #572 (it looks fine).

agonza1 added a commit to agonza1/webcodecs that referenced this issue Jul 16, 2023
* Make VideoFrame.timestamp non-nullable.

Current spec always defines a timestamp for user constructed
VideoFrames. Setting a real timestamp is especially important for frames
that sent to VideoEncoder, as the timestamp is used for rate control.

With no way for users to generate a null timestamp, it makes sense to
make the type non-nullable. This also aligns with AudioData.timestamp.

This patch also addresses the one lingering opporutnity for null
timestamps: ImageDecoder used null as a default for output VideoFrames.
This patch updates that default to be zero. While it could be argued
that null is technically more correct (we don't often present images on
timeline), this is outweighed by the practical impact of using zero
which allows us to define a clear contract: VideoFrame always has a
timestamp.

* Add AudioDecoder 'dequeue' event

* Add notes clarifying VideoFrame timestamp

* [WebCodecs] Fix markup for notes within lists

A `<div>` cannot appear within a `<dl>`. Bikeshed used to automatically tie
notes with the previous `<dd>` in a list but no longer does that, and generated
HTML is now invalid.

* [Webcodecs] Fix markdown typos for definition lists

Trailing `</dt>` has nothing to do here and now makes Bikeshed generate invalid
HTML markup.

* Describe that WG consensus is needed to add registry entries

* fix getAvccBox

* Revise processing model, begin transitioning AudioDecoder

* Use coded size in mp4-decode sample

Fixes w3c#495

* Fix typo in processing loop

* Explicitly define codec saturation

* Remove ImageTrack.onchange event.

We decided not to do this, but forgot to remove it from the spec. The fact that
decode() calls stall until the next frame is available and the availability of the
`completed` promise make it largely unnecessary.

Clients which don't want to wait for for `completed` can instead handle a
RangeError exception during decode() if no more frames remain. While not
ideal it's inefficient to implement something like onchange since common
formats like GIF would require us to attempt decode in order to update the
frame count.

* Add HEVC registration

* Fix H.264 biblio title

* Apply suggestions from code review

Co-authored-by: Chris Needham <chrisn@users.noreply.github.com>

* Finish transitioning AudioDecoder to new model

* update HTTP RFC

* Add missing AudoiDecoder slot init

* Transition VideoDecoder to new model

* Transition AudioEncoder to new model

* Transition VideoEncoder to new model

* Transition VideoFrame to new model

* Transition ImageDecoder to new model

* line wrap fix

* Schedule the dequeue event to coalesce events and avoid spam

* Add dequeue event to VideoDecoder

* Add dequeue event to VideoEncoder

* Add dequeue event to AudioEncoder

* Fix missing removal from ImageTrack change event.

Overlooked in initial PR.

* Add missing biblio entry to HEVC registration

* Revert "Fix the problem that getAvccBox cannot get the value when the first track of the video is not AVC"

* Revert "Revert "Fix the problem that getAvccBox cannot get the value when the first track of the video is not AVC""

* Add audio-video-player sample

Authored in collaboration w/ padenot@

Originally hosted here:
https://github.com/chcunningham/wc-talk

This is mostly a copy/paste of that, but I've moved demuxing and
decoding to a dedicated "media worker" and tried to clean things up a
bit.

* Cleanup for audio_video_player sample

* Rearrange audio_video_player.html

* Add audio_video_player.html to samples index

* Move/rename mp4-decode sample

* Rewview feedback

* Update index links for player sample to use netlify

* Update hevc_codec_registration.src.html

* Remove unused property from example code

* Move Netlify _headers to top level

* Move Netlify _headers back to samples folder

Since we're only interested in publishing samples to Netlify, let's only publish
the `samples` folder with Netlify instead of the contents of the `gh-pages`
branch.

* Codec registry: escape "*" in codec strings

As the spec uses Markdown, some of the `*` characters were interpreted by
Bikeshed as a request to turn the string in italics, whereas we actually want
to render the asterisk characters. For instance, `hev1.*, hvc1.*` was rendered
as `hev1. <em>, hvc1.<em>`.

This update escapes the asterisk character in codec strings.

* Refactor

* Address PR comments.

* Support decoding of hevc.

Added the parser part. Actual hevc content is not uploaded unless
this is neccessary.

* Fix link to the registry in the README.

This fixes w3c#540.

* Move common samples files to library/

Preparing to reuse these files in a new sample.

* Fix visible rect comparison to use strict equality to allow for visible rect of the same size as coded size

This fixes w3c#515.

* Add HEVC registry entry generated output to .gitignore

* Split the privacy and security considerations sections in the registry entries and the registry document

This fixes w3c#544.

* Add new pull demuxer interface. Inject demuxers.

Rather than have the renderer create a specific demuxer, let's inject a
demuxer that uses a common interface. This allows us to re-use the same
renderer with the a variety of demuxers (including the forthcoming
FFmpeg based demuxer).

* Add Acknowledgements - long overdue!

* Add frameDuration attribute to OpusEncoderConfig

* [Publication job] Add HEVC WebCodecs registration

Add HEVC (H.265) WebCodecs Registration to the auto-publish script so that the
specs gets published to the `gh-pages` branch (and to /TR later on when a first
draft note will have been published)

* fix a typo in 8.2.1

* change type of frameDuration in OpusEncoderConfig to unsigned long long, and meaning of it to microseconds as well

* Introduce VideoFrame metadata

* Copy metadata from HTMLVideoElement if init metadata is absent.
Fix some additional nits.

* Additional nit

* Remove ImageDecoderInit.premultiplyAlpha

Fixes w3c#508

* Fix build error and rename getMetadata to metadata

* Throw when calling metadata() in case VideoFrame is closed

* Address review comments on ImageDecoder spec change.

Fixes issues addressed in review.

* Fix indent for paragraphs.

Fixes preview build error.

* Fix reference to mime type RFC.

Fixes build error.

* change the type of frameDuration in OpusEncoderConfig to unsigned long; add validation checking steps to it

* Fixup nullability.

* Remove escape.

* Remove unrelated change.

* Introduce metadata registry

* fix wrong reference of RFC7587 in opus_codec_registration

* wrap ptime with backticks in opus_codec_registration.src.html

* Add WCG color spaces.

* Add spec text for serializable encoded chunks.

Chromium launched with this support, but apparently the spec text
was never updated. Sorry about that!

Fixes: w3c#289

* Change library to lib

* Fix typos

* Remove EnforceRange to framrate definition since EnforceRange is only for integer types

* Address some of Chris's feedback

* Update video_frame_metadata_registry.src.html

* Add policy for changes to existing registry entries

This attempts to formalize the policy for changes proposed in:
w3c#571 (comment)

* Update .github/workflows/auto-publish.yml

Co-authored-by: François Daoust <fd@tidoust.net>

* merge fix

* Add AllowShared to EncodedVideoChunkInit data

* Address Chris comments

* Make VideoDecoderConfig.description AllowShared

* Link to netlify-powered URL for samples (w3c#585)

* Move note on meaningful timestamp to VideoFrame constructor

it really is about the timestamp in VideoFrameInit, not the attribute of VideoFrame

* When serializing some WebCodecs objects with forStorage=true, throw DataCloneError instead of TypeError

This applies to EncodedVideoChunk, EncodedAudioChunk, AudioData and
VideoFrame, and fixes issue w3c#589.

* Add null defaults to VideoColorSpaceInit members

No need to do in prose what Web IDL can achieve.

* Add VideoFrame Metadata Registry to local biblio

The entry is needed until the registry gets published and can be added to
official databases of specs.

* Add OpusEncoderConfig parameters

* Add defaults to booleans

* Move VideoFrameMetadata dfn to WebCodecs

A registry cannot contain normative IDL, so the base definition of the
`VideoFrameMetadata` dictionary should remain in WebCodecs.

Also note that while the registry will be published as a "W3C Draft Registry",
the document in the repo remains an "Editor's Draft". Updating the status
accordingly.

* Add more defaults, notes

* Remove text description of defaults

* Change frameDuration to microseconds

* Linkify to Opus RFC 6716

* Expand acronyms

* Address minor comment

* Change type to long long

* Introduce VideoFrame metadata

* Copy metadata from HTMLVideoElement if init metadata is absent.
Fix some additional nits.

* Additional nit

* Fix build error and rename getMetadata to metadata

* Throw when calling metadata() in case VideoFrame is closed

* Introduce metadata registry

* Address some of Chris's feedback

* Update video_frame_metadata_registry.src.html

* Update .github/workflows/auto-publish.yml

Co-authored-by: François Daoust <fd@tidoust.net>

* Address Chris comments

* Add VideoFrame Metadata Registry to local biblio

The entry is needed until the registry gets published and can be added to
official databases of specs.

* Add null defaults to VideoColorSpaceInit members

No need to do in prose what Web IDL can achieve.

* Move note on meaningful timestamp to VideoFrame constructor

it really is about the timestamp in VideoFrameInit, not the attribute of VideoFrame

* Move VideoFrameMetadata dfn to WebCodecs

A registry cannot contain normative IDL, so the base definition of the
`VideoFrameMetadata` dictionary should remain in WebCodecs.

Also note that while the registry will be published as a "W3C Draft Registry",
the document in the repo remains an "Editor's Draft". Updating the status
accordingly.

* When serializing some WebCodecs objects with forStorage=true, throw DataCloneError instead of TypeError

This applies to EncodedVideoChunk, EncodedAudioChunk, AudioData and
VideoFrame, and fixes issue w3c#589.

* Add OpusEncoderConfig parameters

* Add defaults to booleans

* Add more defaults, notes

* Remove text description of defaults

* Linkify to Opus RFC 6716

* Expand acronyms

* Address minor comment

* Add requirements to metadata registry definition

Covers adding, changing, and deprecating registry entries

* Clarify metadata entry requirements

* Clarify metadata entry requirements

* Mark Chris as a former editor of WebCodecs specs.

* Update WGSL of renderer_webgpu.js

This `textureSampleLevel()` overload was moved to a new `textureSampleBaseClampToEdge()` builtin

* Add a way of specifying blockSize and compressLevel to FLAC encoder

* This fixes w3c#595. Add a way of specifying frame duration to FLAC encoder config

* Fix memory layout of NV12 image example

According to the text:
- The coded width and coded height must be even. So image cannot be 16x9. This
update makes it 16x10
- There should be one Y byte per pixel, so 16x10 Y bytes (example only had 14
bytes per row)
- The U and V components are sub-sampled horizontally and vertically by a factor
of 2, and interleaved, so there should 16x10/4 = 40 UV blocks (example only
sub-sampled horizontally).

This update also fixes occurrences of "horizontaly".

* Drop local custom definitions where possible

Bikeshed now contains cross-reference anchors from Webref, which means it now
knows about a number of "new" specifications, including WebCodecs, Media Capture
specs, the Infra standard, etc. See announcement at:
https://lists.w3.org/Archives/Public/spec-prod/2023JanMar/0004.html

In turn, this means that WebCodecs specs no longer need to list and maintain a
local database of cross-references, except in rare cases (such as when a term
is not exported by the referenced spec).

This update drops local definitions where ever possible, adjusting references
in the prose as needed. A few definitions remain here and there to list external
terms that are not exported, along with a couple of linking defaults in the
WebCodecs spec.

One temporary hiccup: the markup definitions of `CanvasImageSource` and
`drawImage()` are slightly incorrect. Hence the need to re-define them locally.
To be removed once the HTML spec is fixed.

In codec registration specs, note that the form
`{{VideoDecoderConfig/description|VideoDecoderConfig.description}}` is used to
output `VideoDecoderConfig.description` (with the right link to the WebCodecs
spec) when merely having `description` would be ambiguous.

* Drop local definition of now fixed HTML anchors

Definitions fixed upstream so local definitions are no longer needed:
whatwg/html#8736

* Add VideoFrame Metadata Registry to the Readme

* Editorial: fix typos, add missing links

* fix several typoes

* restore 3 wrong fixes according to the comments of @padenot

* Add WebCodecs in Worker sample

This is a sample demonstrating WebCodecs Encode and Decode in a worker.  A live demo is here:
https://webrtc.internaut.com/wc/wcWorker/

* Cleanup debug messages

* Add more granular error messages

* Add CSS, update codec configuration

* Fix HEVC codec string

* Fix HEVC codec string

* Remove remaining "var" usage

* Renove hard coded HEVC error

* Update encode/decode and queue depth statistics

* Remove encoder and decoder metrics

* Restore queue size metrics, change default bitrate and GoP size

* Fix cfg.svc bug

* Add bitrate mode selection

* Adjust default bitrate

* Add WebCodecs in Worker link on samples page

Add a link to the WebCodecs in Worker sample, once merged.

Related: w3c#583

* Add nixxquality's fixes

See: w3c#332 (comment)

* Fix close algorithm for non-null VideoFrame timestamps.

Timestamp shouldn't be null anymore, so remove language around setting it to null.

* Fix a link URL

Fix the link to the "WebCodecs in Worker" sample

* Annotate encoder/decoders with EventTarget on ondequeue events.

Seems to be an oversight from when ondequeue events were added.
Allows the idlharness.js test to start passing.

* Fix usage of {Audio/Video}Decoder.isConfigSupported() in example

* Introducing a new 'quantizer' mode for VideoEncoder

A codec specific quantizer is specified for each video frame for constant
quality or fine tuned external rate control.

* Use is not origin-clean concept in VideoFrame constructor

* support specify audio encode bitrateMode interface

* Move the attribute of bitrateMode from to OpusEncoderConfigAudioEncoderConfig

* Per frame quantizer option for Av1.

This value is used by VideoEncoder.encode() if VideoEncoder is configured
with "quantizer" bitrate mode.

* Enable configuration of AV1 screen content coding tools

Fixes w3c#646

Rebase of w3c#652

* Add screen content coding tools as an encoder configuration option

* Update the abstract

* fix 2 grammatical errors

* Add per-frame QP support to AVC

This value is used by VideoEncoder.encode() if VideoEncoder is configured with "quantizer" bitrate mode. 

Followup for: w3c#270

* Av1 -> Avc

* Clone configuration should perform a deep copy

This fixes w3c#485.

* Address review comment.

* Define videoSource before videoSelect.onchange to avoid ReferenceError.

* Add VideoFrameBufferInit.transfer

* Editorial: account for SharedArrayBuffer change in Web IDL

See whatwg/webidl#1311 for context.

* Fix usage of RFC2119 words in privacy and security section

This fixes w3c#644.

* Add AudioDataInit.transfer

* Remaining normative language issues

Fixes w3c#689

* Replace RFC2119 wording in Security Connsiderations

* Add to codec registry requirements

Changes:

- Adds a requirement for a public specification that is stable
- The WG may consult external expertise as part of its review
- Fixes the numbering in the source document

* Fix HEVC registration text

* Add ImageDecoderInit.transfer

* Fix typo

multiply -> mutliply

Also trigger spec publishing

* Add H.265/VP8/VP9/AV1 samples to video decoding page.

This should be helpful for developer to test the browser implement
or compare the performance of different codecs on different platforms.

Note that the previous version of mp4box is not able to parse AV1
video codec string correctly, so to support AV1, we also need to
upgrade mp4box to version 0.5.2.

Safari doesn't support `gl.createShader` so change the default
renderer to `2d`.

All sample videos are encoded from `bbb-1920x1080-cfg06.mp4` using
ffmpeg and shaka packager.

* Remove stable from codec registry requirements

* Revert unwanted change

* Add Eugene Zemtsov to the list of editors

---------

Co-authored-by: Chris Cunningham <chcunningham@chromium.org>
Co-authored-by: Francois Daoust <fd@tidoust.net>
Co-authored-by: Chris Needham <chris.needham@bbc.co.uk>
Co-authored-by: Vinlic <vinlic@vinlic.com>
Co-authored-by: Dan Sanders <sandersd@google.com>
Co-authored-by: Dale Curtis <dalecurtis@chromium.org>
Co-authored-by: Chris Needham <chrisn@users.noreply.github.com>
Co-authored-by: Christoph Guttandin <chrisguttandin@media-codings.com>
Co-authored-by: Dan Sanders <sandersd@chromium.org>
Co-authored-by: Qiu Jianlin <jianlin.qiu@intel.com>
Co-authored-by: Paul Adenot <paul@paul.cx>
Co-authored-by: bdrtc <webrtc@bytedance.com>
Co-authored-by: Youenn Fablet <youennf@gmail.com>
Co-authored-by: 何超 <hechao.0520@bytedance.com>
Co-authored-by: youennf <youennf@users.noreply.github.com>
Co-authored-by: Thomas Guilbert <52718142+tguilbert-google@users.noreply.github.com>
Co-authored-by: Dominique Hazael-Massieux <dom@w3.org>
Co-authored-by: Thomas Guilbert <tguilbert@chromium.org>
Co-authored-by: Bernard Aboba <bernard.aboba@gmail.com>
Co-authored-by: mark a. foltz <mfoltz@chromium.org>
Co-authored-by: Ben Clayton <bclayton@google.com>
Co-authored-by: josephrocca <1167575+josephrocca@users.noreply.github.com>
Co-authored-by: Chris James <etcet@users.noreply.github.com>
Co-authored-by: Eugene Zemtsov <eugene@chromium.org>
Co-authored-by: Chunbo Hua <chunbo.hua@intel.com>
Co-authored-by: Anne van Kesteren <annevk@annevk.nl>
Co-authored-by: Sida Zhu <zhusida@bytedance.com>
Co-authored-by: Eugene Zemtsov <e.zemtsov@gmail.com>
Co-authored-by: StaZhu <zhusidayoyo@hotmail.com>
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

No branches or pull requests

5 participants