Clean up WebSourceBufferClient interface

Clean up WebSourceBufferClient interface

Use a struct instead of tuple for media track info.
Use a WebVector instead of std::vector in blink interface.

Committed: https://crrev.com/da2def6a572055b921d6559ad33366524d26d9ef
Cr-Commit-Position: refs/heads/master@{#388348}

[servolk, 2016-04-15 20:34:14]

Description was changed from

==========
Clean up WebSourceBufferClient interface

Use a struct instead of tuple for media track info.
Use a WebVector instead of std::vector in blink interface.
==========

to

==========
Clean up WebSourceBufferClient interface

Use a struct instead of tuple for media track info.
Use a WebVector instead of std::vector in blink interface.
==========

[servolk, 2016-04-15 20:34:14]

servolk@chromium.org changed reviewers:
+ esprehn@chromium.org, philipj@opera.com, wolenetz@chromium.org

[esprehn, 2016-04-15 20:56:00]

https://codereview.chromium.org/1897533002/diff/1/media/blink/websourcebuffer...
File media/blink/websourcebuffer_impl.cc (right):

https://codereview.chromium.org/1897533002/diff/1/media/blink/websourcebuffer...
media/blink/websourcebuffer_impl.cc:191:
blink::WebVector<blink::WebMediaPlayer::TrackId> blinkTrackIds =
Whats the purpose of this if we just ignore the value?

[servolk, 2016-04-15 21:02:32]

https://codereview.chromium.org/1897533002/diff/1/media/blink/websourcebuffer...
File media/blink/websourcebuffer_impl.cc (right):

https://codereview.chromium.org/1897533002/diff/1/media/blink/websourcebuffer...
media/blink/websourcebuffer_impl.cc:191:
blink::WebVector<blink::WebMediaPlayer::TrackId> blinkTrackIds =
On 2016/04/15 20:56:00, esprehn wrote:
> Whats the purpose of this if we just ignore the value?

This information is necessary to be able to map blink track id values to
DemuxerStream* pointers in media::Pipeline. The previous CL that added this code
implemented plumbing to get media track info to blink and then this set of track
ids out of blink. The next CL that uses this set of track ids has not landed yet
(although it will hopefully land soon, see
https://codereview.chromium.org/1812543003/)

[haraken, 2016-04-16 01:06:01]

haraken@chromium.org changed reviewers:
+ haraken@chromium.org

[haraken, 2016-04-16 01:06:02]

LGTM

https://codereview.chromium.org/1897533002/diff/1/media/blink/websourcebuffer...
File media/blink/websourcebuffer_impl.cc (right):

https://codereview.chromium.org/1897533002/diff/1/media/blink/websourcebuffer...
media/blink/websourcebuffer_impl.cc:191:
blink::WebVector<blink::WebMediaPlayer::TrackId> blinkTrackIds =
On 2016/04/15 21:02:32, servolk wrote:
> On 2016/04/15 20:56:00, esprehn wrote:
> > Whats the purpose of this if we just ignore the value?
> 
> This information is necessary to be able to map blink track id values to
> DemuxerStream* pointers in media::Pipeline. The previous CL that added this
code
> implemented plumbing to get media track info to blink and then this set of
track
> ids out of blink. The next CL that uses this set of track ids has not landed
yet
> (although it will hopefully land soon, see
> https://codereview.chromium.org/1812543003/)

I'd suggest you add the |blinkTrackIds| in the next CL that actually uses it.

[philipj_slow, 2016-04-19 09:22:05]

Also LGTM % introducing things when they're used. Sorry I wasted your time with
the tuple idea.

[servolk, 2016-04-19 21:50:14]

The CQ bit was checked by servolk@chromium.org

[servolk, 2016-04-19 21:50:14]

The patchset sent to the CQ was uploaded after l-g-t-m from
haraken@chromium.org, philipj@opera.com
Link to the patchset: https://codereview.chromium.org/1897533002/#ps20001
(title: "Remove unused blinkTrackId variable for now")

[servolk, 2016-04-19 21:50:38]

On 2016/04/19 09:22:05, philipj_slow wrote:
> Also LGTM % introducing things when they're used. Sorry I wasted your time
with
> the tuple idea.

Ok, I've removed the unused variable in patchset #2

[commit-bot: I haz the power, 2016-04-19 21:50:40]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1897533002/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1897533002/20001

[commit-bot: I haz the power, 2016-04-19 21:59:31]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-04-19 21:59:32]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[wolenetz, 2016-04-19 22:35:02]

Sorry for late nit :)
LGTM % it (and if you need to fix the nit in follow-on CL, that's fine).

https://codereview.chromium.org/1897533002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/mediasource/SourceBuffer.cpp (right):

https://codereview.chromium.org/1897533002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/mediasource/SourceBuffer.cpp:522:
result[resultIdx++] = ++nextTrackId;
nit: I'm not sure it's really nextTrackId, since pre-increment causes the stable
state of this variable to store the nextTrackId_minus_one value in reality.
Maybe initialize to 1, and post-increment it here so that its semantic matches
its name?

[servolk, 2016-04-19 22:50:10]

https://codereview.chromium.org/1897533002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/mediasource/SourceBuffer.cpp (right):

https://codereview.chromium.org/1897533002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/mediasource/SourceBuffer.cpp:522:
result[resultIdx++] = ++nextTrackId;
On 2016/04/19 22:35:02, wolenetz wrote:
> nit: I'm not sure it's really nextTrackId, since pre-increment causes the
stable
> state of this variable to store the nextTrackId_minus_one value in reality.
> Maybe initialize to 1, and post-increment it here so that its semantic matches
> its name?

As I've explained offline this is actually to emulate what blink nextTrackId is
doing.
see
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit...
so I believe it's ok for now.

[servolk, 2016-04-19 22:50:15]

The CQ bit was checked by servolk@chromium.org

[commit-bot: I haz the power, 2016-04-19 22:50:40]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1897533002/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1897533002/20001

[commit-bot: I haz the power, 2016-04-19 23:20:55]

Description was changed from

==========
Clean up WebSourceBufferClient interface

Use a struct instead of tuple for media track info.
Use a WebVector instead of std::vector in blink interface.
==========

to

==========
Clean up WebSourceBufferClient interface

Use a struct instead of tuple for media track info.
Use a WebVector instead of std::vector in blink interface.
==========

[commit-bot: I haz the power, 2016-04-19 23:20:56]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-04-22 19:17:46]

Description was changed from

==========
Clean up WebSourceBufferClient interface

Use a struct instead of tuple for media track info.
Use a WebVector instead of std::vector in blink interface.
==========

to

==========
Clean up WebSourceBufferClient interface

Use a struct instead of tuple for media track info.
Use a WebVector instead of std::vector in blink interface.

Committed: https://crrev.com/da2def6a572055b921d6559ad33366524d26d9ef
Cr-Commit-Position: refs/heads/master@{#388348}
==========

[commit-bot: I haz the power, 2016-04-22 19:17:47]

Patchset 2 (id:??) landed as
https://crrev.com/da2def6a572055b921d6559ad33366524d26d9ef
Cr-Commit-Position: refs/heads/master@{#388348}