-
Notifications
You must be signed in to change notification settings - Fork 483
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
Add AC3 Atom support to MP4Info and MP4Dump #122
base: master
Are you sure you want to change the base?
Conversation
Thanks @GnaphronG! This would be really helpful 👍 |
@@ -711,6 +712,13 @@ AP4_AtomFactory::CreateAtomFromStream(AP4_ByteStream& stream, | |||
atom = AP4_Dec3Atom::Create(size_32, stream); | |||
} | |||
break; | |||
case AP4_ATOM_TYPE_DAC3: | |||
if (atom_is_large) return AP4_ERROR_INVALID_FORMAT; | |||
if (GetContext() == AP4_ATOM_TYPE_AC_3) { |
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.
If you want to support encrypted audio tracks, you could do so by changing this line to the following:
if (GetContext() == AP4_ATOM_TYPE_AC_3 || GetContext() == AP4_ATOM_TYPE_ENCA) {
This fix is included in the current master branch on GitHub.
…On Wed, Aug 16, 2017 at 8:58 AM, essolsoj ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In Source/C++/Core/Ap4AtomFactory.cpp
<#122 (comment)>
:
> @@ -711,6 +712,13 @@ AP4_AtomFactory::CreateAtomFromStream(AP4_ByteStream& stream,
atom = AP4_Dec3Atom::Create(size_32, stream);
}
break;
+ case AP4_ATOM_TYPE_DAC3:
+ if (atom_is_large) return AP4_ERROR_INVALID_FORMAT;
+ if (GetContext() == AP4_ATOM_TYPE_AC_3) {
If you want to support encrypted audio tracks, you could do so by changing
this line to the following:
if (GetContext() == AP4_ATOM_TYPE_AC_3 || GetContext() ==
AP4_ATOM_TYPE_ENCA) {
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#122 (review)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ACWJjTOOA7wANN5qFxJytwk2lJ2N8AI6ks5sYxGfgaJpZM4Le_9j>
.
|
m_RawBytes.SetData(payload, payload_size); | ||
|
||
// sanity check | ||
if (payload_size < 2) return; |
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.
Wouldn't we want a payload of at least 3 bytes (as we read payload[2]
line 86) ?
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.
You're right, this should be < 3
// parse the payload | ||
m_Fscod = (payload[0]>>6) & 0x3; | ||
m_Bsid = (payload[0]>>1) & 0x1F; | ||
m_Bsmod = (payload[0]<<2 | payload[1]>>6) & 0x3; |
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.
payload[0] << 2
is meaningless as the line ends with & 0x3
. Perhaps we want & 0x7
?
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.
Good catch. This should be masked with 0x7, not 0x3, as the field is 3 bits wide. Thanks for spotting that.
MP4Info doesn't return the info of AC-3 tracks:
For example using the AC-3 file for the HLS Advanced stream example https://developer.apple.com/streaming/examples/:
https://tungsten.aaplimg.com/VOD/bipbop_adv_fmp4_example/a2/main.mp4
The patch returns results similar to EC-3 tracks for AC-3 tracks