Comment on attachment 8858699 [details] Bug 1355756. P1 - change the type of MediaData::mDuration to TimeUnit. https://reviewboard.mozilla.org/r/130708/#review133292

Comment on attachment 8858700 [details] Bug 1355756. P2 - let VideoData::UpdateDuration() take TimeUnit. https://reviewboard.mozilla.org/r/130710/#review133294

Comment on attachment 8858701 [details] Bug 1355756. P3 - let CreateAndCopyData() and its friends take TimeUnit for duration. https://reviewboard.mozilla.org/r/130712/#review133296

Thanks for the review!

Pushed by jwwang@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/2534b8ae23de P1 - change the type of MediaData::mDuration to TimeUnit. r=gerald https://hg.mozilla.org/integration/autoland/rev/c077fb83e09b P2 - let VideoData::UpdateDuration() take TimeUnit. r=gerald https://hg.mozilla.org/integration/autoland/rev/58c6ac2e352c P3 - let CreateAndCopyData() and its friends take TimeUnit for duration. r=gerald

https://hg.mozilla.org/mozilla-central/rev/2534b8ae23de https://hg.mozilla.org/mozilla-central/rev/c077fb83e09b https://hg.mozilla.org/mozilla-central/rev/58c6ac2e352c

A reminder to myself, and a heads-up for you&me&everybody: It would be nice(r) if the commit descriptions also contained the "why" for the changes, e.g. in this case something like "TimeUnit gives better source readability, and compile-time contextual checks, than a naked int64_t to record millisecond durations". Sorry that I didn't catch this in past reviews. In the future, I will try and enforce this, and I would invite you to do the same. ;-) I know it's a bit more work, but hopefully the extra few minutes needed to write the longer descriptions will be useful to future readers&maintainers of our code. See https://goo.gl/AkK7jl for some discussions as to why this is useful.