w3c / webrtc-svc

W3C Scalable Video Coding (SVC) Extension for WebRTC
https://w3c.github.io/webrtc-svc/
Other
39 stars 14 forks source link

Confusing names for KEY and KEY_SHIFT structures #36

Closed DanilChapovalov closed 4 years ago

DanilChapovalov commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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

aboba commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

@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 commented 4 years ago

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

DanilChapovalov commented 4 years ago

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 commented 4 years ago

@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 commented 4 years ago

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