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

DTS Core IMAX detection #1832

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

RoyFunderburk
Copy link

@RoyFunderburk RoyFunderburk commented Jul 12, 2023

In File_Dts.cpp, .dtshd files already have detection for IMAX in the extension frames, this pull request adds IMAX detection for core-only .dtshd files. There is a flag indicating IMAX content in the Rev2 Auxiliary data chunk, parsing for this chunk is added. In order to parse this chunk, a field from further down in the core header (sub sub frame count) is needed, so the core chunk processing is expanded a bit also. I tried to keep the addition consistent with the existing IMAX detection.

New sample files "rev2aux-X.dts" test variations on the Rev2Aux chunk. imax.dtshd is a core-only file with the rev2aux chunk indicating IMAX content. 61.dtshd and 121.dtshd have IMAX in extension frames that test the pre-existing IMAX detection. in.dtshd is similar to imax.dtshd, but is normal, not IMAX.

Copy link
Member

@JeromeMartinez JeromeMartinez left a comment

Choose a reason for hiding this comment

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

Generally speaking, I don't get the meaning of the Type1CertifiedContent, it seems that it is only a flag without actual new content (only few bytes of metadata?) so just some marketing stuff, what is the technical difference? In other words, will a decoder not supporting this extension decode differently the stream compared to a decoder supporting this extension, and will a decoder supporting this extension decode differently a stream with this extension than a stream without this extension (e.g. if I replace 0x7004C070 by 0x12345678, is the decoded content same between the modified file and the unmodified file?)

With MediaInfo we do care about the difference between marketing and technical parts, so this does matter to know if a "legacy" decoder can decode in the same manner or not such stream.

Source/MediaInfo/Audio/File_Dts.cpp Outdated Show resolved Hide resolved
ABITBits+=SkipABITSelector(PrimaryChannels, 2, 3);
for (int i=5; i<10; i++)
ABITBits+=SkipABITSelector(PrimaryChannels, 3, 7);
Skip_S8(ABITBits, "Scale Factor Adjustment Indexes");
Copy link
Member

Choose a reason for hiding this comment

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

Use the more generic Skip_BS (because Skip_S8 is sometimes limited to 64 bits).

Source/MediaInfo/Audio/File_Dts.cpp Outdated Show resolved Hide resolved
Source/MediaInfo/Audio/File_Dts.cpp Outdated Show resolved Hide resolved
Skip_S1(Remain, "ByteAlign");
if (DRCMetadataPresent)
for (int i=0; i<SubSubFrameCount; i++)
Skip_S1(8, "subsubFrameDRC_Rev2AUX");
Copy link
Member

Choose a reason for hiding this comment

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

"DRCCoeff_Rev2"

Source/MediaInfo/Audio/File_Dts.cpp Outdated Show resolved Hide resolved
Source/MediaInfo/Audio/File_Dts.cpp Outdated Show resolved Hide resolved
@RoyFunderburk
Copy link
Author

if I replace 0x7004C070 by 0x12345678, is the decoded content same between the modified file and the unmodified file?)

All of the channels decode exactly the same data for these .dtshd imax files. The difference is the Ls/Rs channels are used as Lsr/Rsr. I had missed that detail in the pull request changes and I will update the channel display to reflect that situation.

@JeromeMartinez
Copy link
Member

JeromeMartinez commented Jul 13, 2023

The difference is the Ls/Rs channels are used as Lsr/Rsr

For 5.1? It seems a very small change, because in practice it does not change the user experience, Ls/rs and Lsr/Rsr being very similar for 5.1 config, and maybe same in practice.
In practice, with 6 speakers at home, what a decoder aware of Type1CertifiedContent will do a decoder not aware of Type1CertifiedContent won't do?

the format profiles part is intended for differences if a decoder does not support a feature, e.g. a 7.1 content is played as 5.1 (2 channels completely ignored) or a lossless content is not decoded as lossless, here a flag and no "visible" difference in practice makes me think that I'll move imax to the "commercial name" part only.

Copy link
Member

@JeromeMartinez JeromeMartinez left a comment

Choose a reason for hiding this comment

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

Generally speaking, I don't get the meaning of the Type1CertifiedContent, it seems that it is only a flag without actual new content (only few bytes of metadata?) so just some marketing stuff, what is the technical difference? In other words, will a decoder not supporting this extension decode differently the stream compared to a decoder supporting this extension, and will a decoder supporting this extension decode differently a stream with this extension than a stream without this extension (e.g. if I replace 0x7004C070 by 0x12345678, is the decoded content same between the modified file and the unmodified file?)

With MediaInfo we do care about the difference between marketing and technical parts, so this does matter to know if a "legacy" decoder can decode in the same manner or not such stream.

@RoyFunderburk
Copy link
Author

RoyFunderburk commented Jul 13, 2023

In practice, with 6 speakers at home, what a decoder aware of Type1CertifiedContent will do a decoder not aware of Type1CertifiedContent won't do?
it seems that it is only a flag without actual new content (only few bytes of metadata?)

The .dtshd decoder will output the same PCM data regardless of the t1cc setting, the intention was the audio that is used for the encode would be different.

Do you want me to undo the recent change where the channel layout reported is different for the t1cc flag?

@RoyFunderburk RoyFunderburk force-pushed the rf-sosm-5-1 branch 3 times, most recently from 497d38a to 286d12a Compare July 13, 2023 22:13
@RoyFunderburk
Copy link
Author

I modified the t1cc/imax detection to only report t1cc/imax in the "Commercial name" as you suggested. There are no changes to the format or any other report fields.

@JeromeMartinez
Copy link
Member

I tweaked a bit the output for doing the difference between the technical flag and the commercial name.

@JeromeMartinez
Copy link
Member

@RoyFunderburk I added a commit which fixes the bitstream parsing, due to:

if (nRev2AUXDataByteSize>4)
    bBroadcastMetadataPresent = (ExtractBits(1) == 1) ? TRUE : FALSE;
else
    bBroadcastMetadataPresent = FALSE;

Without this commit some files I have don't pass, they are like that:

000858  Rev2Aux (16 bytes)
000858   Sync Word:                            1879359600 (0x7004C070)
00085C   Rev2AUXDataByteSize:                  3 (0x3) - (7 bits)
00085C   ESMetaDataFlag:                       Yes
00085D   EmbESDownMixScaleIndex:               216 (0xD8) - (8 bits)
00085E   Rev2AUXCRC16:                         52973 (0xCEED)

by checking bBroadcastMetadataPresent it is out of specs and an extra bit not available in the content.

BUT: it breaks the parsing of your sample files.
Please hint about how to parse correctly your files while being compliant with specs (and previous files I have).

Copy link
Member

@JeromeMartinez JeromeMartinez left a comment

Choose a reason for hiding this comment

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

Mismatch between specs and provided code/files

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.

None yet

3 participants