media: Stop flushing the renderer for pipeline suspend

media: Stop flushing the renderer for pipeline suspend

Previously when performing a pipeline suspend we would flush the
renderer before destructing it. This CL removes the flush because
it's no longer necessary and has issues:
1) it adds ~100-150ms to suspend on a Galaxy Nexus (which makes
   a difference for fullscreen transitions), and
2) on <API level 18 devices it will cause AVDA to go into an
   error state when it handles the flush by creating a new
   MediaCodec without a valid output surface.

BUG=596641, 595545
TEST=media_unittests

Committed: https://crrev.com/9edddb9ded49023460a204675ee4e9bb763b9da7
Cr-Commit-Position: refs/heads/master@{#382417}

[watk, 2016-03-21 20:30:23]

watk@chromium.org changed reviewers:
+ sandersd@chromium.org

[watk, 2016-03-21 20:30:23]

I tested this on Android by causing a bunch of suspends and wasn't able to cause
any issues.

I also tried it on desktop with --enable-media-suspend and tab switching a lot.
No issues AFAICT.

[DaleCurtis, 2016-03-21 20:32:06]

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

[DaleCurtis, 2016-03-21 20:32:06]

Should give this a BUG= since we might want to merge later since this speeds up
the transition and may reduce the number decode errors too.

[DaleCurtis, 2016-03-21 20:32:19]

dalecurtis@chromium.org changed reviewers:
- dalecurtis@chromium.org

[watk, 2016-03-21 20:35:49]

Description was changed from

==========
media: Stop flushing the renderer for pipeline suspend

Previously when performing a pipeline suspend we would flush the
renderer before destructing it. This CL removes the flush because
it's no longer necessary and adds ~100-150ms to suspend on a
Galaxy Nexus (which makes a difference for fullscreen transitions).

BUG=
==========

to

==========
media: Stop flushing the renderer for pipeline suspend

Previously when performing a pipeline suspend we would flush the
renderer before destructing it. This CL removes the flush because
it's no longer necessary and adds ~100-150ms to suspend on a
Galaxy Nexus (which makes a difference for fullscreen transitions).

BUG=596641
TEST=media_unittests
==========

[watk, 2016-03-21 20:36:56]

Description was changed from

==========
media: Stop flushing the renderer for pipeline suspend

Previously when performing a pipeline suspend we would flush the
renderer before destructing it. This CL removes the flush because
it's no longer necessary and adds ~100-150ms to suspend on a
Galaxy Nexus (which makes a difference for fullscreen transitions).

BUG=596641
TEST=media_unittests
==========

to

==========
media: Stop flushing the renderer for pipeline suspend

Previously when performing a pipeline suspend we would flush the
renderer before destructing it. This CL removes the flush because
it's no longer necessary and adds ~100-150ms to suspend on a
Galaxy Nexus (which makes a difference for fullscreen transitions).

BUG=596641,595545
TEST=media_unittests
==========

[watk, 2016-03-21 20:38:22]

Description was changed from

==========
media: Stop flushing the renderer for pipeline suspend

Previously when performing a pipeline suspend we would flush the
renderer before destructing it. This CL removes the flush because
it's no longer necessary and adds ~100-150ms to suspend on a
Galaxy Nexus (which makes a difference for fullscreen transitions).

BUG=596641,595545
TEST=media_unittests
==========

to

==========
media: Stop flushing the renderer for pipeline suspend

Previously when performing a pipeline suspend we would flush the
renderer before destructing it. This CL removes the flush because
it's no longer necessary and has issues:
1) it adds ~100-150ms to suspend on a Galaxy Nexus (which makes
   a difference for fullscreen transitions), and
2) on <API level 18 devices it will cause AVDA to go into an
   error state when it handles the flush by creating a new
   MediaCodec without a valid output surface.

BUG=596641,595545
TEST=media_unittests
==========

[sandersd (OOO until July 31), 2016-03-21 21:16:41]

lgtm

[watk, 2016-03-21 21:35:35]

The CQ bit was checked by watk@chromium.org

[commit-bot: I haz the power, 2016-03-21 21:36:08]

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

[commit-bot: I haz the power, 2016-03-21 22:21:18]

Description was changed from

==========
media: Stop flushing the renderer for pipeline suspend

Previously when performing a pipeline suspend we would flush the
renderer before destructing it. This CL removes the flush because
it's no longer necessary and has issues:
1) it adds ~100-150ms to suspend on a Galaxy Nexus (which makes
   a difference for fullscreen transitions), and
2) on <API level 18 devices it will cause AVDA to go into an
   error state when it handles the flush by creating a new
   MediaCodec without a valid output surface.

BUG=596641,595545
TEST=media_unittests
==========

to

==========
media: Stop flushing the renderer for pipeline suspend

Previously when performing a pipeline suspend we would flush the
renderer before destructing it. This CL removes the flush because
it's no longer necessary and has issues:
1) it adds ~100-150ms to suspend on a Galaxy Nexus (which makes
   a difference for fullscreen transitions), and
2) on <API level 18 devices it will cause AVDA to go into an
   error state when it handles the flush by creating a new
   MediaCodec without a valid output surface.

BUG=596641,595545
TEST=media_unittests
==========

[commit-bot: I haz the power, 2016-03-21 22:21:19]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-03-21 22:22:31]

Description was changed from

==========
media: Stop flushing the renderer for pipeline suspend

Previously when performing a pipeline suspend we would flush the
renderer before destructing it. This CL removes the flush because
it's no longer necessary and has issues:
1) it adds ~100-150ms to suspend on a Galaxy Nexus (which makes
   a difference for fullscreen transitions), and
2) on <API level 18 devices it will cause AVDA to go into an
   error state when it handles the flush by creating a new
   MediaCodec without a valid output surface.

BUG=596641,595545
TEST=media_unittests
==========

to

==========
media: Stop flushing the renderer for pipeline suspend

Previously when performing a pipeline suspend we would flush the
renderer before destructing it. This CL removes the flush because
it's no longer necessary and has issues:
1) it adds ~100-150ms to suspend on a Galaxy Nexus (which makes
   a difference for fullscreen transitions), and
2) on <API level 18 devices it will cause AVDA to go into an
   error state when it handles the flush by creating a new
   MediaCodec without a valid output surface.

BUG=596641,595545
TEST=media_unittests

Committed: https://crrev.com/9edddb9ded49023460a204675ee4e9bb763b9da7
Cr-Commit-Position: refs/heads/master@{#382417}
==========

[commit-bot: I haz the power, 2016-03-21 22:22:32]

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

[watk, 2016-03-22 00:56:20]

A revert of this CL (patchset #2 id:20001) has been created in
https://codereview.chromium.org/1824923002/ by watk@chromium.org.

The reason for reverting is: This hit a CHECK in
PipelineIntegrationTest.SuspendWhilePlaying

[12876:12876:0321/173811:1285613567:FATAL:ffmpeg_demuxer.cc(592)] Check failed:
read_cb_.is_null(). Overlapping reads are not supported

https://build.chromium.org/p/chromium.memory.fyi/builders/Chromium%20OS%20%28...

This was something we didn't consider and probably can't be relanded without a
fair amount of work..

[DaleCurtis, 2016-03-22 17:16:47]

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

[DaleCurtis, 2016-03-22 17:16:47]

Ah, you'll likely need to abort the current read or wait for it to complete
before allowing resume. It might be possible to coordinate the demuxer and
pipeline in some way to resolve this.