Add AVC and HEVC codec mappings with BlockAdditionMapping #389
Conversation
JeromeMartinez
force-pushed
the
AVC_and_HEVC_BlockAddition
branch
from
8a91107 to
ed40ff8
Compare
| @@ -151,11 +151,11 @@ | |||
| <extension webm="1"/> | |||
| </element> | |||
| <element name="BlockAddID" path="\Segment\Cluster\BlockGroup\BlockAdditions\BlockMore\BlockAddID" id="0xEE" type="uinteger" range="not 0" default="1" minOccurs="1" maxOccurs="1"> | |||
| <documentation lang="en" purpose="definition">An ID to identify the BlockAdditional level. A value of 1 means the BlockAdditional data is interpreted as additional data passed to the codec with the Block data.</documentation> | |||
| <documentation lang="en" purpose="definition">An ID to identify the BlockAdditional level. If BlockAddIDType of the corresponding track is 0, for VP9, 0x01 is reserved and 0x04 indicates ITU T.35 metadata as defined by ITU-T T.35 terminal codes; otherwise the value refers to the BlockAddIDValue value used in the corresponding BlockAdditionMapping element.</documentation> | |||
There was a problem hiding this comment.
This is dangerously close to becoming very hard to read. Maybe break out the conditions to separate sections in other documents.
| @@ -318,11 +318,11 @@ | |||
| <extension webm="0"/> | |||
| </element> | |||
| <element name="BlockAdditionMapping" path="\Segment\Tracks\TrackEntry\BlockAdditionMapping" id="0x41E4" type="master" minver="4"> | |||
| <documentation lang="en" purpose="definition">Contains elements that describe each value of <a href="https://www.matroska.org/technical/specs/index.html#BlockAddID">BlockAddID</a> found in the Track.</documentation> | |||
| <documentation lang="en" purpose="definition">Contains elements that extend the track format, by adding content either to frames (with <a href="https://www.matroska.org/technical/specs/index.html#BlockAddID">BlockAddID</a>) or to the format definition</documentation> | |||
There was a problem hiding this comment.
"the format definition" is a very ambiguous term, one that I wouldn't understand as an implementer. The "format definition" is e.g. ISO 14496-12. We don't extend that definition. What we do extend is codec-specific data present in the track headers.
| <documentation lang="en" purpose="definition">This value indicates that BlockAddIDValue defines the content type, and a list of content type is in BlockAddID documentation.</documentation> | ||
| </enum> | ||
| <enum value="1" label="Codec specific content"> | ||
| <documentation lang="en" purpose="definition">This value indicates that BlockAddIDExtraData defines the content type, and this definition depends on the Codec ID used. A transcoder SHOULD discard this content if the addition is not supported</documentation> |
There was a problem hiding this comment.
Why place any restriction/recommendation on whether this data should be kept or discarded? Why should a demuxer/remuxer know whether such data is important? I'd leave that out, or always state that "data SHOULD be kept" (obviously data might be discarded on user request, but a generic remuxer should keep as much data as possible). Maybe the extension was registered in a later version of the specs and the remuxer doesn't know about it. Or maybe it's a generic demuxer that only passes data along to the codec, and it's up to the codec to decide if the data is actually relevant.
A specialized tool for enforcing format compatibility might make other choices, true, but this text is about the general case.
| <documentation lang="en" purpose="definition">This value indicates that BlockAddIDExtraData defines the content type, and this definition depends on the Codec ID used. A transcoder SHOULD discard this content if the addition is not supported</documentation> | ||
| </enum> | ||
| <enum value="2" label="General purpose content"> | ||
| <documentation lang="en" purpose="definition">This value indicates that BlockAddIDExtraData defines the content type, and this definition does not depend on the Codec ID used. A transcoder SHOULD NOT discard this content if the addition is not supported</documentation> |
There was a problem hiding this comment.
Same argument here. I'd rather the whole sentence about keeping/discarded din't exist.
| <documentation lang="en" purpose="definition">This value indicates that BlockAddIDExtraData defines the content type, and this definition does not depend on the Codec ID used. A transcoder SHOULD NOT discard this content if the addition is not supported</documentation> | ||
| </enum> | ||
| <enum value="3" label="ISOBMFF-like extension"> | ||
| <documentation lang="en" purpose="definition">This value indicates that BlockAddIDExtraData consists of a 32-bit type field and an arbitrary large content field. The values used for both fields are the same as used in header atoms in ISOBMFF files and its extensions such as the "mvcC" box in ISO 14496-15:2014. As it is not expected that ISOBMFF-like extension have frame related content, BlockAddIDValue SHOULD NOT be present.</documentation> |
There was a problem hiding this comment.
BlockAddIDValue SHOULD NOT be present
I'd change that to MUST NOT (yes, invalid file in that case). The question we've often heard from reviewers about "SHOULD (NOT)" is "when would it be OK to violate that rule? What should happen then?" and I see no situation in which we have anything useful to do with BlockAddIDValue in this case.
|
Should we close this PR as #390 seems the way to go ? |
This PR is an alternative proposal for #377.
Note that it was started before #388, I used some wording from this PR so this PR would replace both of them.
In my opinion
BlockAdditionMappingwas not clear enough, especially theBlockAddIDTypepart and its compatibility with WebM, so I expect to clarify the use ofBlockAddIDType.I use
BlockAddIDTypeof 0 (the default) for the compatibility with WebM, especially the fact thatBlockAddIDValuedefines the content in WebM if I well understand.I was expecting to use
BlockAdditionMappingfor Codec independent content, so I "reserve"BlockAddIDTypevalue 1 & 2 for specifying if theBlockAdditionMappingis dependent or not dependent of the Codec. For example a Codec dependent addition would be the extension of a codec transforming the main lossy data to lossless, with the same Codec (if the main content codec changes, this addition is no more usable, a transcoder must trash the addition), and a Codec independent addition would be the same except that the addition content format is specific and not dependent on the main content format (I think to my project RAWcooked here, I could convert main data from JPEG-2000 lossless to FFV1, the RAWcooked addition would not change, a transcoder should keep the addition; it could also be 608 Closed Caption or Bar Data).BlockAddIDTypevalue 3 would be for ISOBMFF-like stuff. I don't refer to "CodecPrivate extension" in my wording as ISOBMFF stuff does not rely on it, I prefer to keep it more generic for the future.