MSE: Optimize frame processor appends to streams

MSE: Optimize frame processor appends to streams

This change collects contiguous processed frames for each track buffer
and appends them to track buffers' streams as groups of frames rather
than inefficiently appending each frame one at a time. One at a time
appends are still possible in the worst case, since collected processed
frames are appended at the end of ProcessFrames() and also prior to any
NotifyNewMediaSegmentStarting().

R=acolwell@chromium.org, damienv1@chromium.org
BUG=371197
TEST=no media_unittest, MSE layout test, or YT conformance unit test regression locally on Linux

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=282165

[wolenetz, 2014-07-08 23:59:15]

PTAL @ PS1. Thanks!

[acolwell GONE FROM CHROMIUM, 2014-07-09 00:11:47]

lgtm

https://codereview.chromium.org/381443002/diff/1/media/filters/frame_processo...
File media/filters/frame_processor.cc (right):

https://codereview.chromium.org/381443002/diff/1/media/filters/frame_processo...
media/filters/frame_processor.cc:70: // Adds |frame_| to the end of
|processed_frames_|.
nit: s/frame_/frame/

[damienv1, 2014-07-09 01:56:56]

https://codereview.chromium.org/381443002/diff/1/media/filters/frame_processo...
File media/filters/frame_processor.cc (right):

https://codereview.chromium.org/381443002/diff/1/media/filters/frame_processo...
media/filters/frame_processor.cc:324: result &=
itr->second->FlushProcessedFrames();
Notation might be confusing here.
I would rather do sth like:

if (!itr->second->FlushProcessedFrames())
  result = false;

Had you written: result = result && itr->second->FlushProcessedFrames();
then FlushProcessedFrames would not have been called for each stream (if
flushing fails for the first stream).

[damienv1, 2014-07-09 01:59:24]

https://codereview.chromium.org/381443002/diff/1/media/filters/frame_processo...
File media/filters/frame_processor.cc (right):

https://codereview.chromium.org/381443002/diff/1/media/filters/frame_processo...
media/filters/frame_processor.cc:152: DVLOG(3) << __FUNCTION__
nit: DVLOG_IF

[damienv1, 2014-07-09 18:10:09]

lgtm

[wolenetz, 2014-07-09 18:23:29]

Thank you for reviews (and pre-publish lgtm of PS2 :) )

https://codereview.chromium.org/381443002/diff/1/media/filters/frame_processo...
File media/filters/frame_processor.cc (right):

https://codereview.chromium.org/381443002/diff/1/media/filters/frame_processo...
media/filters/frame_processor.cc:70: // Adds |frame_| to the end of
|processed_frames_|.
On 2014/07/09 00:11:47, acolwell wrote:
> nit: s/frame_/frame/

Done.

https://codereview.chromium.org/381443002/diff/1/media/filters/frame_processo...
media/filters/frame_processor.cc:152: DVLOG(3) << __FUNCTION__
On 2014/07/09 01:59:24, damienv1 wrote:
> nit: DVLOG_IF

Done.

https://codereview.chromium.org/381443002/diff/1/media/filters/frame_processo...
media/filters/frame_processor.cc:324: result &=
itr->second->FlushProcessedFrames();
On 2014/07/09 01:56:56, damienv1 wrote:
> Notation might be confusing here.
> I would rather do sth like:
> 
> if (!itr->second->FlushProcessedFrames())
>   result = false;
> 
> Had you written: result = result && itr->second->FlushProcessedFrames();
> then FlushProcessedFrames would not have been called for each stream (if
> flushing fails for the first stream).

Done. Note, as you very probably know, C++ &= and |= are not short-circuiting.
There are also no ||= or &&= in C++ (which would probably short-circuit if they
did exist). We use &= and |= in many places -- when masking or setting up flags,
for instance. However, I'll make this explicit as you suggest to prevent
confusion here.

[wolenetz, 2014-07-09 18:23:46]

The CQ bit was checked by wolenetz@chromium.org

[commit-bot: I haz the power, 2014-07-09 18:24:59]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/wolenetz@chromium.org/381443002/20001

[commit-bot: I haz the power, 2014-07-09 23:10:39]

Change committed as 282165