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

Extend EncodedVideoChunkMetadata #636

Closed
wants to merge 7 commits into from
Closed

Extend EncodedVideoChunkMetadata #636

wants to merge 7 commits into from

Conversation

aboba
Copy link
Collaborator

@aboba aboba commented Feb 3, 2023

Fixes #619


💥 Error: 400 Bad Request 💥

PR Preview failed to build. (Last tried on Mar 16, 2023, 3:22 AM UTC).

More

PR Preview relies on a number of web services to run. There seems to be an issue with the following one:

🚨 CSS Spec Preprocessor - CSS Spec Preprocessor is the web service used to build Bikeshed specs.

🔗 Related URL

Error running preprocessor, returned code: 2.
FATAL ERROR: Quoted attribute was never closed
 ✘  Did not generate, due to fatal errors

If you don't have enough information above to solve the error by yourself (or to understand to which web service the error is related to, if any), please file an issue.

@aboba aboba requested a review from dalecurtis February 3, 2023 00:24
@aboba aboba requested a review from sandersdan February 3, 2023 00:35
@dalecurtis
Copy link
Contributor

@Djuffin to review.

@@ -1692,6 +1711,11 @@

dictionary SvcOutputMetadata {
unsigned long temporalLayerId;
unsigned long spatiallayerId;
unsigned short frameNumber;
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not frameId like in RTCEncodedVideoFrameMetadata ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

As noted in the Dependency Descriptor specification section A.8.3, frame_number is not the same as FrameId.

@@ -1692,6 +1711,11 @@

dictionary SvcOutputMetadata {
unsigned long temporalLayerId;
unsigned long spatiallayerId;
unsigned short frameNumber;
sequence<unsigned long> dependsOnIds;
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not dependencies like in RTCEncodedVideoFrameMetadata?

Copy link
Collaborator Author

@aboba aboba Feb 3, 2023

Choose a reason for hiding this comment

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

dependencies in RTCEncodedVideoFrameMetadata is not defined but I'm assuming it is a sequence of frameId values. dependsOnIds is a sequence of frameNumber values (though it seems odd that frameNumber is an unsigned short and dependsOnIds is a sequence of unsigned long values).

Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't it be names dependsOnFrameNumbers then?

Otherwise LGTM.

@aboba aboba closed this by deleting the head repository Mar 16, 2023
aboba added a commit that referenced this pull request Mar 16, 2023
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.

Inconsistent SVC metadata between WebCodecs and WebRTC Encoded Transform API
3 participants