Add initial support for edit lists in MSE BMFF.

Add initial support for edit lists in MSE BMFF.

BUG=135665
TEST=TrackRunIteratorTest.ReorderingTest

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=150478

[acolwell GONE FROM CHROMIUM, 2012-08-07 20:12:13]

Will this fix http://crbug.com/135665 ?

http://chromiumcodereview.appspot.com/10832176/diff/5001/media/mp4/track_run_...
File media/mp4/track_run_iterator.cc (right):

http://chromiumcodereview.appspot.com/10832176/diff/5001/media/mp4/track_run_...
media/mp4/track_run_iterator.cc:171: // both uncommon and better served by
higher-level protocols.
nit: It looks like there should be a 

if (stuff we don't support) { 
  DVLOG(1) << "we don't support that crazy feature"; 
  return false;
} 

in here?

http://chromiumcodereview.appspot.com/10832176/diff/5001/media/mp4/track_run_...
File media/mp4/track_run_iterator_unittest.cc (right):

http://chromiumcodereview.appspot.com/10832176/diff/5001/media/mp4/track_run_...
media/mp4/track_run_iterator_unittest.cc:276: EditListEntry entry;
I don't really understand what is happening here. A comment here indicating how
this entry modifies the timestamps would be nice.

http://chromiumcodereview.appspot.com/10832176/diff/5001/media/mp4/track_run_...
media/mp4/track_run_iterator_unittest.cc:286: cts_offsets[0] = 2;
Documenting these values would be nice as well. It is hard to tell how 2, 5, & 0
map to the times you specify in the big comment above.

[strobe_, 2012-08-07 22:54:39]

It will not fix their bug automatically, but it will provide  a way to do this
correctly.

http://chromiumcodereview.appspot.com/10832176/diff/5001/media/mp4/track_run_...
File media/mp4/track_run_iterator.cc (right):

http://chromiumcodereview.appspot.com/10832176/diff/5001/media/mp4/track_run_...
media/mp4/track_run_iterator.cc:171: // both uncommon and better served by
higher-level protocols.
On 2012/08/07 20:12:13, acolwell wrote:
> nit: It looks like there should be a 
> 
> if (stuff we don't support) { 
>   DVLOG(1) << "we don't support that crazy feature"; 
>   return false;
> } 
> 
> in here?

Done.

http://chromiumcodereview.appspot.com/10832176/diff/5001/media/mp4/track_run_...
File media/mp4/track_run_iterator_unittest.cc (right):

http://chromiumcodereview.appspot.com/10832176/diff/5001/media/mp4/track_run_...
media/mp4/track_run_iterator_unittest.cc:276: EditListEntry entry;
On 2012/08/07 20:12:13, acolwell wrote:
> I don't really understand what is happening here. A comment here indicating
how
> this entry modifies the timestamps would be nice.

Done.

http://chromiumcodereview.appspot.com/10832176/diff/5001/media/mp4/track_run_...
media/mp4/track_run_iterator_unittest.cc:286: cts_offsets[0] = 2;
On 2012/08/07 20:12:13, acolwell wrote:
> Documenting these values would be nice as well. It is hard to tell how 2, 5, &
0
> map to the times you specify in the big comment above.

Done.

[acolwell GONE FROM CHROMIUM, 2012-08-07 23:20:04]

LGTM % nit

http://chromiumcodereview.appspot.com/10832176/diff/2004/media/mp4/track_run_...
File media/mp4/track_run_iterator.cc (right):

http://chromiumcodereview.appspot.com/10832176/diff/2004/media/mp4/track_run_...
media/mp4/track_run_iterator.cc:178: if (edits[0].media_time > 0)
nit: Why are negative values not allowed? Isn't it valid to allow TS adjustments
in either direction or are there semantic differences here? Either remove
condition or add a comment indicating why we only care about positive values.

[strobe_, 2012-08-07 23:38:15]

http://chromiumcodereview.appspot.com/10832176/diff/2004/media/mp4/track_run_...
File media/mp4/track_run_iterator.cc (right):

http://chromiumcodereview.appspot.com/10832176/diff/2004/media/mp4/track_run_...
media/mp4/track_run_iterator.cc:178: if (edits[0].media_time > 0)
On 2012/08/07 23:20:05, acolwell wrote:
> nit: Why are negative values not allowed? Isn't it valid to allow TS
adjustments
> in either direction or are there semantic differences here? Either remove
> condition or add a comment indicating why we only care about positive values.

Done.

[commit-bot: I haz the power, 2012-08-07 23:38:24]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/strobe@google.com/10832176/2005

[commit-bot: I haz the power, 2012-08-08 01:33:36]

Change committed as 150478