Parse mvex to retrieve fragment_duration. #27
Conversation
rillian
left a comment
rillian
left a comment
There was a problem hiding this comment.
lgtm. r=m with comments addressed.
mp4parse_capi/src/lib.rs
Outdated
| return MP4PARSE_OK; | ||
| match context.mvex { | ||
| Some(_) => {}, | ||
| None => return MP4PARSE_OK, |
There was a problem hiding this comment.
When taking action on a single branch of an Option match, I think is_none() is better. The Some line doesn't do anything useful here.
if context.mvex.is_none() {
return MP4PARSE_OK;
}
mp4parse/src/lib.rs
Outdated
|
|
||
| fn read_mvex<T: Read>(src: &mut BMFFBox<T>) -> Result<MovieExtendsBox> { | ||
| let mut iter = src.box_iter(); | ||
| let mut duration: Option<u64> = None; |
There was a problem hiding this comment.
The compiler should be able to infer this type from the signature of read_mehd so you only need this if you especially want to document it locally. Up to you.
mp4parse/src/lib.rs
Outdated
| }) | ||
| } | ||
|
|
||
| fn read_mehd<T: Read>(src: &mut BMFFBox<T>) -> Result<Option<u64>> { |
There was a problem hiding this comment.
mehd always has a fragment duration, so you don't need to return an Option here. A simpler Result<u64> or Result<MediaScaledTime> would work. This complicates the way you update duration, but I think it's better to have a temporary there and keep the semantics of the function signature correct.
e.g.
let mut fragment_duration = None;
...
let duration = try!(read_mehd(&mut b));
fragment_duration = Some(duration);
...
Ok(MovieExtendsBox {
fragment_duration: fragment_duration,
})The duration of a fragmented file can be found if MovieExtendsHeaderBox is exist. Add mp4parse_get_fragment_info() to retrieve information in MovieExtendsBox.
alfredoyang
force-pushed
the
parse_mvex
branch
from
ff8b9dd to
239c0ce
Compare
|
Should I update parser to gecko in the same bug? Because there is another gecko patch depending on this in the same bug. |
|
If you need these changes in gecko we can bump the in-tree version on the bugzilla side. You can do it, or I can, I've had lots of practice. |
2 participants
Parse fragment_duration for https://bugzilla.mozilla.org/show_bug.cgi?id=1304254.
Test will be added in another PR later.