-
Notifications
You must be signed in to change notification settings - Fork 178
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
base: master
Are you sure you want to change the base?
DTS Core IMAX detection #1832
Conversation
There was a problem hiding this 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
ABITBits+=SkipABITSelector(PrimaryChannels, 2, 3); | ||
for (int i=5; i<10; i++) | ||
ABITBits+=SkipABITSelector(PrimaryChannels, 3, 7); | ||
Skip_S8(ABITBits, "Scale Factor Adjustment Indexes"); |
There was a problem hiding this comment.
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
Skip_S1(Remain, "ByteAlign"); | ||
if (DRCMetadataPresent) | ||
for (int i=0; i<SubSubFrameCount; i++) | ||
Skip_S1(8, "subsubFrameDRC_Rev2AUX"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"DRCCoeff_Rev2"
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. |
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. 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. |
There was a problem hiding this 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.
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? |
497d38a
to
286d12a
Compare
286d12a
to
b2f6d6b
Compare
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. |
I tweaked a bit the output for doing the difference between the technical flag and the commercial name. |
09fe71d
to
9157368
Compare
@RoyFunderburk I added a commit which fixes the bitstream parsing, due to:
Without this commit some files I have don't pass, they are like that:
by checking BUT: it breaks the parsing of your sample files. |
There was a problem hiding this 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
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.