Skip to content

Parse 'pssh' box for eme playback.#43

Merged
rillian merged 2 commits intomozilla:masterfrom
alfredoyang:parse_pssh
Nov 9, 2016
Merged

Parse 'pssh' box for eme playback.#43
rillian merged 2 commits intomozilla:masterfrom
alfredoyang:parse_pssh

Conversation

@alfredoyang
Copy link
Copy Markdown
Contributor

No description provided.

Comment thread mp4parse/src/lib.rs Outdated
}

fn read_pssh<T: Read>(src: &mut BMFFBox<T>) -> Result<ProtectionSystemSpecificHeaderBox> {
let mut box_content = Vec::new();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

You might save some allocations here if you use Vec::with_capacity(src.head.size); but it probably doesn't matter.

Comment thread mp4parse/src/lib.rs
let item = try!(read_buf(pssh, 16));
kid.push(item);
count -= 1;
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think for _ in 0..count would be better than a while loop here. It's a little awkward, but there's one less line of code, and count doesn't need to be mut.

Comment thread mp4parse_capi/src/lib.rs Outdated
// pssh data format in gecko:
// system_id
// pssh size (in native endian)
// pssh box content (including header)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It might be better to put this description in the function-level doc-comment above, since it's something a caller would need to know, not just an internal implementation detail.

Comment thread mp4parse/src/lib.rs Outdated

let mut pssh_box = Vec::new();
try!(write_be_u32(&mut pssh_box, src.head.size as u32));
pssh_box.append(&mut vec![b'p', b's', b's', b'h']);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

You can pssh_box.extend(b"pssh"); which is a bit easier to read.

NB there's also Vec::extend_from_slice() but extend is equally fast in rust 1.14, so I don't think it's worth using the longer method name here. rust-lang/rust#37094

@alfredoyang
Copy link
Copy Markdown
Contributor Author

Thanks for review. All comments addressed.

@rillian rillian merged commit 30d1237 into mozilla:master Nov 9, 2016
@rillian
Copy link
Copy Markdown
Contributor

rillian commented Nov 9, 2016

Thanks!

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.

2 participants