Refactor audio and video decoder status into common file.

Refactor audio and video decoder status into common file.

Completes a long standing refactor that rileya@ initiated in days of yore
before we had DecoderStream. Will make it easier to add a new entry for
PLATFORM_FAILURE in a followup CL.

Should be no functional changes, though it does include a fix to the
pepper video decoder shim which was silenting converting the int type
of VideoDecoder::Status to a int32_t which was expected to be PP_xxx
type value. This change sends PP_OK (== 0) where previously it was
sending VideoDecoder::kAborted (== 1).

BUG=595076
TEST=none

Committed: https://crrev.com/8f75b99f6313d6a88aad2a759eb551afb183561b
Cr-Commit-Position: refs/heads/master@{#384020}

[DaleCurtis, 2016-03-28 23:56:40]

dalecurtis@chromium.org changed reviewers:
+ bbudge@chromium.org, xhwang@chromium.org

[DaleCurtis, 2016-03-28 23:56:41]

dalecurtis@chromium.org changed reviewers:
+ bbudge@chromium.org, xhwang@chromium.org

[DaleCurtis, 2016-03-28 23:56:41]

bbudge: Please check the content/renderer/pepper/ changes to video shim. Notably
this changes OnDecodeComplete to send PP_OK now instead of an invalid PP_xxx
value which was erroneously converted from VideoDecoder::kAborted.

[DaleCurtis, 2016-03-28 23:56:41]

bbudge: Please check the content/renderer/pepper/ changes to video shim. Notably
this changes OnDecodeComplete to send PP_OK now instead of an invalid PP_xxx
value which was erroneously converted from VideoDecoder::kAborted.

[DaleCurtis, 2016-03-28 23:56:52]

The CQ bit was checked by dalecurtis@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-03-28 23:57:19]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1834303005/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1834303005/1

[commit-bot: I haz the power, 2016-03-29 00:18:07]

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

[commit-bot: I haz the power, 2016-03-29 00:18:08]

Dry run: Try jobs failed on following builders:
  cast_shell_linux on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)

[DaleCurtis, 2016-03-29 01:46:57]

The CQ bit was checked by dalecurtis@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-03-29 01:47:13]

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

[commit-bot: I haz the power, 2016-03-29 02:56:59]

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

[commit-bot: I haz the power, 2016-03-29 02:57:00]

Dry run: This issue passed the CQ dry run.

[bbudge, 2016-03-29 17:48:59]

content/renderer/pepper LGTM
Thanks!

https://codereview.chromium.org/1834303005/diff/20001/content/renderer/pepper...
File content/renderer/pepper/video_decoder_shim.cc (right):

https://codereview.chromium.org/1834303005/diff/20001/content/renderer/pepper...
content/renderer/pepper/video_decoder_shim.cc:735: FROM_HERE,
base::Bind(&VideoDecoderShim::OnDecodeComplete, shim_, PP_OK,
Thanks for fixing this.

[xhwang, 2016-03-29 19:42:28]

Looking good. I just have a few comments/nits for discussion.

CCing sandersd@ for one comment.

https://chromiumcodereview.appspot.com/1834303005/diff/20001/media/base/decod...
File media/base/decoder_status.h (right):

https://chromiumcodereview.appspot.com/1834303005/diff/20001/media/base/decod...
media/base/decoder_status.h:8: #include <ostream>
Will "#include <iosfwd>" work?

See example:
https://code.google.com/p/chromium/codesearch#chromium/src/base/strings/strin...

https://chromiumcodereview.appspot.com/1834303005/diff/20001/media/base/decod...
media/base/decoder_status.h:14: enum class DecoderStatus {
Since this the status of the "decode" operation, not the general state/status of
the "decoder". Should it be named "DecodeStatus"?

Note that in mojo we actually use interfaces::AudioDecoder::DecodeStatus.

https://chromiumcodereview.appspot.com/1834303005/diff/20001/media/base/decod...
media/base/decoder_status.h:17: DECODE_ERROR,  // Decoder returned decode error.
The "DECODE" part seems redundant but apparently "ERROR" is defined as a special
macro on Windows. Shall we document it so people won't ask again?

Well, it won't look redundant when you add other error types. So totally up to
you :)

https://chromiumcodereview.appspot.com/1834303005/diff/20001/media/filters/de...
File media/filters/decoder_stream.h (left):

https://chromiumcodereview.appspot.com/1834303005/diff/20001/media/filters/de...
media/filters/decoder_stream.h:42: typedef typename Decoder::Status
DecoderStatus;
nice!

https://chromiumcodereview.appspot.com/1834303005/diff/20001/media/mojo/servi...
File media/mojo/services/mojo_audio_decoder.cc (right):

https://chromiumcodereview.appspot.com/1834303005/diff/20001/media/mojo/servi...
media/mojo/services/mojo_audio_decoder.cc:169: case
interfaces::AudioDecoder::DecodeStatus::OK:
+sandersd: When working on VDAv2, we probably want to have
interfaces::DecodeStatus directly to match the C++ code.

[DaleCurtis, 2016-03-30 00:34:19]

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

[DaleCurtis, 2016-03-30 00:34:19]

+alokp for chromecast/

https://chromiumcodereview.appspot.com/1834303005/diff/20001/media/base/decod...
File media/base/decoder_status.h (right):

https://chromiumcodereview.appspot.com/1834303005/diff/20001/media/base/decod...
media/base/decoder_status.h:8: #include <ostream>
On 2016/03/29 at 19:42:28, xhwang wrote:
> Will "#include <iosfwd>" work?
> 
> See example:
https://code.google.com/p/chromium/codesearch#chromium/src/base/strings/strin...

Done.

https://chromiumcodereview.appspot.com/1834303005/diff/20001/media/base/decod...
media/base/decoder_status.h:14: enum class DecoderStatus {
On 2016/03/29 at 19:42:28, xhwang wrote:
> Since this the status of the "decode" operation, not the general state/status
of the "decoder". Should it be named "DecodeStatus"?
> 
> Note that in mojo we actually use interfaces::AudioDecoder::DecodeStatus.

Renamed to DecodeStatus.

https://chromiumcodereview.appspot.com/1834303005/diff/20001/media/base/decod...
media/base/decoder_status.h:17: DECODE_ERROR,  // Decoder returned decode error.
On 2016/03/29 at 19:42:28, xhwang wrote:
> The "DECODE" part seems redundant but apparently "ERROR" is defined as a
special macro on Windows. Shall we document it so people won't ask again?
> 
> Well, it won't look redundant when you add other error types. So totally up to
you :)

Done.

[xhwang, 2016-03-30 00:48:22]

lgtm

[DaleCurtis, 2016-03-30 00:48:49]

The CQ bit was checked by dalecurtis@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-03-30 00:49:27]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1834303005/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1834303005/40001

[commit-bot: I haz the power, 2016-03-30 03:27:01]

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

[commit-bot: I haz the power, 2016-03-30 03:27:02]

Dry run: Try jobs failed on following builders:
  linux_chromium_rel_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[alokp, 2016-03-30 04:36:13]

chromecast lgtm

[DaleCurtis, 2016-03-30 04:39:07]

The CQ bit was checked by dalecurtis@chromium.org

[DaleCurtis, 2016-03-30 04:39:07]

The patchset sent to the CQ was uploaded after l-g-t-m from bbudge@chromium.org
Link to the patchset: https://codereview.chromium.org/1834303005/#ps40001
(title: "Comments.")

[commit-bot: I haz the power, 2016-03-30 04:39:39]

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

[commit-bot: I haz the power, 2016-03-30 06:55:05]

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

[commit-bot: I haz the power, 2016-03-30 06:55:07]

Try jobs failed on following builders:
  win_chromium_x64_rel_ng on tryserver.chromium.win (JOB_TIMED_OUT,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[DaleCurtis, 2016-03-30 16:34:41]

The CQ bit was checked by dalecurtis@chromium.org

[commit-bot: I haz the power, 2016-03-30 16:35:07]

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

[commit-bot: I haz the power, 2016-03-30 17:55:08]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-03-30 17:56:41]

Description was changed from

==========
Refactor audio and video decoder status into common file.

Completes a long standing refactor that rileya@ initiated in days of yore
before we had DecoderStream. Will make it easier to add a new entry for
PLATFORM_FAILURE in a followup CL.

Should be no functional changes, though it does include a fix to the
pepper video decoder shim which was silenting converting the int type
of VideoDecoder::Status to a int32_t which was expected to be PP_xxx
type value. This change sends PP_OK (== 0) where previously it was
sending VideoDecoder::kAborted (== 1).

BUG=595076
TEST=none
==========

to

==========
Refactor audio and video decoder status into common file.

Completes a long standing refactor that rileya@ initiated in days of yore
before we had DecoderStream. Will make it easier to add a new entry for
PLATFORM_FAILURE in a followup CL.

Should be no functional changes, though it does include a fix to the
pepper video decoder shim which was silenting converting the int type
of VideoDecoder::Status to a int32_t which was expected to be PP_xxx
type value. This change sends PP_OK (== 0) where previously it was
sending VideoDecoder::kAborted (== 1).

BUG=595076
TEST=none

Committed: https://crrev.com/8f75b99f6313d6a88aad2a759eb551afb183561b
Cr-Commit-Position: refs/heads/master@{#384020}
==========

[commit-bot: I haz the power, 2016-03-30 17:56:43]

Patchset 3 (id:??) landed as
https://crrev.com/8f75b99f6313d6a88aad2a759eb551afb183561b
Cr-Commit-Position: refs/heads/master@{#384020}