Update OVDA to handle error condition.

Update OVDA to handle error condition.

With this changes, OVDA will inform video client
of any error that has occurred while decoding.
This change handles
1) NotifyError() to client.
2) Avoid calling OMX_GetState if component handle is NULL.
3) Update client_state_ after state transistion.
4) Return error value.

Tested=Emulated the error condition and verified that
       client receives the error notification.
Bug=http://code.google.com/p/chromium/issues/detail?id=88432

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

[Ami GONE FROM CHROMIUM, 2011-07-05 15:55:53]

Thanks for kickstarting this.

http://codereview.chromium.org/7308005/diff/1/content/common/gpu/media/omx_vi...
File content/common/gpu/media/omx_video_decode_accelerator.cc (right):

http://codereview.chromium.org/7308005/diff/1/content/common/gpu/media/omx_vi...
content/common/gpu/media/omx_video_decode_accelerator.cc:155:
StopOnError(VIDEODECODERERROR_UNINITIALIZED);
Most of the error codes seem like very poor translations to me (from
OMX_ERRORTYPE to VideoDecodeAccelerator::Error) caused by the mismatch between
OMX_ET and VDA::E)
Do you want to take a crack at rewriting the VDA::E (and
PP_VideoDecoderError_Dev) enum values to better match the plugin-visible APIs? 
Otherwise I can put it on my TODO list.

http://codereview.chromium.org/7308005/diff/1/content/common/gpu/media/omx_vi...
content/common/gpu/media/omx_video_decode_accelerator.cc:645: if
(component_handle_) {
If you reverse this test you can have the return as the then clause, and
de-indent the rest of the function.

http://codereview.chromium.org/7308005/diff/1/content/common/gpu/media/omx_vi...
content/common/gpu/media/omx_video_decode_accelerator.cc:651: break;
Here and below I think return is clearer than break, and makes it less likely
that code is accidentally added to the bottom of this function and executed as
fall-through unintentionally.

http://codereview.chromium.org/7308005/diff/1/content/common/gpu/media/omx_vi...
content/common/gpu/media/omx_video_decode_accelerator.cc:659: LOG(ERROR) <<
"Invalid state:"
s/:/: /

http://codereview.chromium.org/7308005/diff/1/content/common/gpu/media/omx_vi...
content/common/gpu/media/omx_video_decode_accelerator.cc:663: return;
If you take my suggestion above this goes away.
Otherwise this is still redundant.

[vjain, 2011-07-05 16:04:14]

http://codereview.chromium.org/7308005/diff/1/content/common/gpu/media/omx_vi...
File content/common/gpu/media/omx_video_decode_accelerator.cc (right):

http://codereview.chromium.org/7308005/diff/1/content/common/gpu/media/omx_vi...
content/common/gpu/media/omx_video_decode_accelerator.cc:155:
StopOnError(VIDEODECODERERROR_UNINITIALIZED);
I agree with you that error translation is not as expected in OVDA. I will at
this moment continue with this mapping and let you create a new error mapping.

http://codereview.chromium.org/7308005/diff/1/content/common/gpu/media/omx_vi...
content/common/gpu/media/omx_video_decode_accelerator.cc:645: if
(component_handle_) {
Will Fix this.

http://codereview.chromium.org/7308005/diff/1/content/common/gpu/media/omx_vi...
content/common/gpu/media/omx_video_decode_accelerator.cc:651: break;
will fix this.

http://codereview.chromium.org/7308005/diff/1/content/common/gpu/media/omx_vi...
content/common/gpu/media/omx_video_decode_accelerator.cc:659: LOG(ERROR) <<
"Invalid state:"
Done

http://codereview.chromium.org/7308005/diff/1/content/common/gpu/media/omx_vi...
content/common/gpu/media/omx_video_decode_accelerator.cc:663: return;
Done

[vjain, 2011-07-05 16:17:54]

Uploaded new patch with suggested changes.
Please review

[Ami GONE FROM CHROMIUM, 2011-07-05 16:27:36]

http://codereview.chromium.org/7308005/diff/4002/content/common/gpu/media/omx...
File content/common/gpu/media/omx_video_decode_accelerator.cc (right):

http://codereview.chromium.org/7308005/diff/4002/content/common/gpu/media/omx...
content/common/gpu/media/omx_video_decode_accelerator.cc:638:
media::VideoDecodeAccelerator::Error error) {
Indent 4 spaces from left margin.

http://codereview.chromium.org/7308005/diff/4002/content/common/gpu/media/omx...
content/common/gpu/media/omx_video_decode_accelerator.cc:661: LOG(ERROR) <<
"Invalid state::"
I meant for you to add a space after the colon, not add an extra colon.

[vjain, 2011-07-05 16:33:57]

Uploaded patch set3, which take care of previous suggestions

http://codereview.chromium.org/7308005/diff/4002/content/common/gpu/media/omx...
File content/common/gpu/media/omx_video_decode_accelerator.cc (right):

http://codereview.chromium.org/7308005/diff/4002/content/common/gpu/media/omx...
content/common/gpu/media/omx_video_decode_accelerator.cc:638:
media::VideoDecodeAccelerator::Error error) {
On 2011/07/05 16:27:36, Ami Fischman wrote:
> Indent 4 spaces from left margin.

Done.

http://codereview.chromium.org/7308005/diff/4002/content/common/gpu/media/omx...
content/common/gpu/media/omx_video_decode_accelerator.cc:661: LOG(ERROR) <<
"Invalid state::"
On 2011/07/05 16:27:36, Ami Fischman wrote:
> I meant for you to add a space after the colon, not add an extra colon.

Done.

[commit-bot: I haz the power, 2011-07-05 16:38:51]

No LGTM from valid reviewers yet.

[Ami GONE FROM CHROMIUM, 2011-07-05 16:44:39]

LGTM
(clicked "commit" checkbox before explicitly LGTM'ing, which apparently doesn't
work; will re-click)

[commit-bot: I haz the power, 2011-07-05 17:31:56]

Change committed as 91513