Enable accurate mime type assignment for DTS-HD in TS#3184
Conversation
3cc2fe6 to
39fe5ef
Compare
Read more of the DTS-HD header in order to find out extension substream type, to get correct mime type which is relevant for buffer size decision logic (as DTS Express has way lower maximum bit rate than DTS-HD). Issue: androidx#2487 Issue: androidx#3147
There was a problem hiding this comment.
Hi @rohitjoins,
I don't understand why this is needed: according to my reading of Table 7-5/7-6/7-7 you don't need to check this and can just assume embedded stereo/6ch flags as false if static fields aren't present.
There was a problem hiding this comment.
Thank you for looking into these commits and providing feedback.
I took a closer look at the ETSI TS 102 114 V1.6.1 specification to verify this, and unfortunately, assuming those flags are false when bStaticFieldsPresent == 0 violates the spec and will cause a parser desync.
According to Section 7.5.2 (Page 98), the definition of bStaticFieldsPresent states: If the bStaticFieldsPresent is false, the metadata fields that are static over the duration of an encoded stream are omitted from the extension substream header.
Because they are static over the duration of an encoded stream, they are simply omitted to save bandwidth, they do not become false? They retain whatever value was established in the initial keyframe.
Since DtsUtil.parseDtsHdHeader is a stateless utility, it has no memory of the stream's true bEmbeddedStereoFlag. If we blindly assume the flag is false on an intermediate frame, but the stream actually has embedded stereo, our parser will fail to skip those 8 bits and instantly desync. By the time it tries to read the coding mode for the MIME type, it will be reading misaligned garbage data.
Therefore, because we cannot reliably parse the Dynamic Metadata statelessly, safely confining the parsing to if (staticFieldsPresent) and using the sampleMimeType fallback in DtsReader is required to prevent crashes on intermediate frames?
Please let me know if my interpretation of the spec is correct, and if I should send this PR at its current state for internal review. Thanks!
There was a problem hiding this comment.
Thanks for explaining, you're right :)
There was a problem hiding this comment.
@nift4 Is your next PR going to modify the DTS format detection of MKV files?
Moves the parsing of the Asset Descriptor's Dynamic Metadata and Decoder Navigation Data inside the `staticFieldsPresent` block. Because `DtsUtil.parseDtsHdHeader` is a stateless utility, it cannot safely skip dynamic metadata on intermediate frames where static fields are absent, as the bit layout relies on flags (like `embeddedStereo` and `enableMixMetadata`) established in the static block. To handle intermediate frames safely: - Extracts the complex ETSI Table 7-6 and 7-7 parsing into dedicated helper methods for readability. - Makes `DtsHeader.mimeType` `@Nullable` so it can be safely omitted when static fields are skipped. - Updates `DtsReader` to gracefully fall back to the existing format's `sampleMimeType` when the parsed header yields a null MIME type, preventing unnecessary format recreations.
|
Thanks @rohitjoins, I've uploaded #3226 as next step |
Read more of the DTS-HD header in order to find out extension substream type, to get correct mime type which is relevant for buffer size decision logic (as DTS Express has way lower maximum bit rate than DTS-HD).
Issue: #2487
Issue: #3147