-
Notifications
You must be signed in to change notification settings - Fork 31
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
improvement: Add support for keys and numbered ilst items BoxTypes. #159
Conversation
@dtrejod I have a suggestion. The value of
We can implement to resolve Apple metadata box by following code instead: func (boxType BoxType) getBoxDef(ctx Context) *boxDef {
boxDefs := boxMap[boxType]
for i := len(boxDefs) - 1; i >= 0; i-- {
boxDef := &boxDefs[i]
if boxDef.isTarget == nil || boxDef.isTarget(ctx) {
return boxDef
}
}
if ctx.UnderIlst {
typeID := /* TODO: convert boxType to uint32 */
if typeID >= 1 && typeID <= ctx.QuickTimeKeysMetaEntryCount {
return &boxDef {
/* TODO */
}
}
}
return nil
} For this approach, we need to add reference:Android libstagefright status_t MPEG4Extractor::parseQTMetaVal(
int32_t keyId, off64_t offset, size_t size) {
ssize_t index = mMetaKeyMap.indexOfKey(keyId);
if (index < 0) {
// corresponding key is not present, ignore
return ERROR_MALFORMED;
} |
@sunfish-shogi Thank you for the feedback. Agree that approach is much more sensible. I pushed a commit with your suggestion. UPDATE: After some further testing I identified the approach here does not work however. The latest commit demonstrates the number items under I'll revisit this PR when I have time and a better understanding of this repository. |
For example, https://github.com/abema/go-mp4/blob/v1.1.1/read.go#L53-L65 And it propagate the flag to following same level boxes. https://github.com/abema/go-mp4/blob/v1.1.1/read.go#L172-L174 It is important to implement in |
Thanks for the pointers pointing me in the correct direction. |
@sunfish-shogi
Before this PR
There was no support for the
keys
box type (reference: #13). Additionally there was no support for numbered items under theilst
box type.Before PR changes
As a demonstration, I've updated the
testdata/sample_qt.mp4
file to have these new BoxTypes (atoms). The before mp4tool dump of this updatedtestdata/sample_qt.mp4
file is shown below. From the below output you can see lines[54-56]
show thekeys
andilst
blocks as "unsupported"After this PR
Adds support for both
keys
and numbered items under thelist
box type.After PR changes
From the below output you can now see lines
[54-56]
show thekeys
andlist
blocks as properly handled now.