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

Confusing names for KEY and KEY_SHIFT structures #36

Closed
DanilChapovalov opened this issue Jul 6, 2020 · 10 comments
Closed

Confusing names for KEY and KEY_SHIFT structures #36

DanilChapovalov opened this issue Jul 6, 2020 · 10 comments

Comments

@DanilChapovalov
Copy link
Contributor

L4T7_KEY name suggests it is using 4 spatial and 7 temporal layers.
That is not entirely true. It is using 4 spatial ids and 7 temporal ids to describes 3 spatial and 3 temporal layers.
Extra ids are needed to overcome limitations of the av1 operating_point_idcs.
Though webrtc-svc spec mentions av1, it is not av1 specific.
e.g. VP9 describes same structure using 3 spatial ids and 3 temporal ids.

Suggestion: Rename them or add alternative names that better match described number of layers, i.e.
instead of L4T7_KEY use L3T3_KEY and L4T7_KEY_AV1
instead of L3T3_KEY use L2T3_KEY and L3T3_KEY_AV1
(same for all other KEY and KEY_SHIFT structures)

@aboba
Copy link
Contributor

aboba commented Jul 6, 2020

The entries in the Table in Section 6 are taken from Section 6.7.5 of the AV1 Bitstream Specification. Is there a better reference for mode names and dependency graphs?

@DanilChapovalov
Copy link
Contributor Author

Ksvc with shift structure for 3 spatial and 3 temporal layers is also exampled in the av1 rtp spec as L3T3: https://aomediacodec.github.io/av1-rtp-spec/#a64-l3t3-k-svc-with-temporal-shift
It is the same structure that in AV1 was named L4T7_KEY_SHIFT

I'm not aware of any other spec that reference these structures. webrtc-svc might be the best one to describe and name them.
alternatively it is possible to example all KEY and KEY_SHIFT structures in the annex of the av1 rtp spec.

@Philipel-WebRTC
Copy link

I would prefer to rename and to skip the _AV1 names altogether.

@aboba
Copy link
Contributor

aboba commented Jul 9, 2020

If we want to remove the reference to the AV1 bitstream specification, then we'll need to provide dependency pictures for all the modes (in SVG format) along with the names. Do we have all the SVG images?

@DanilChapovalov
Copy link
Contributor Author

Now we do have all the svg images:
I've created a script and generated svg images for all the structures:
https://github.com/DanilChapovalov/webrtc-svc/tree/add-images/images
not sure how to see all images at once, but each individual can be seen via link like this one:
https://raw.githubusercontent.com/DanilChapovalov/webrtc-svc/add-images/images/L3T3_KEY_SHIFT.svg

@aboba
Copy link
Contributor

aboba commented Jul 10, 2020

@DanilChapovalov Can you submit a PR with the structures you generated? Once we have those in the repo, we can work on including them in the document and updating the table.

aboba added a commit that referenced this issue Jul 10, 2020
Partial fix for Issue #36
@aboba
Copy link
Contributor

aboba commented Jul 10, 2020

@DanilChapovalov I have added the diagrams in Section 9, but some of them are not rendering correctly.

@DanilChapovalov
Copy link
Contributor Author

It seems I set the height incorrectly, and Time axis is cut as the result. Are there other issues with the images?
which boxes are opaque? (I do not see or understand that issue)

btw, L2T4_KEY should likely be removed because it is isomorphic to L2T3_KEY structure (same for L2T4_KEY_SHIFT)

aboba added a commit that referenced this issue Jul 11, 2020
Fix for Issue #36
aboba added a commit that referenced this issue Jul 11, 2020
Fix for Issue #36
aboba added a commit that referenced this issue Jul 13, 2020
@aboba
Copy link
Contributor

aboba commented Jul 14, 2020

@DanilChapovalov @mhoro We now have the diagrams and table entries linking to them, as well as a column in the table providing the corresponding AV1 scalability mode. How does this all look?

@DanilChapovalov
Copy link
Contributor Author

I think it looks good and resolves the confusion. Thank you!

@aboba aboba closed this as completed Jul 18, 2020
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

3 participants