OMX: Don't try to send buffers when in resetting state.

OMX: Don't try to send buffers when in resetting state.

If we do this, the OMX_FillThisBuffer function will fail, and will put the OMX
class in error mode. Then resource freeing will not happen. This is especially
annoying since this can happen during Destroy and leads to resource leaks.

BUG=chrome-os-partner:12316
TEST=by hand: play a video, look for buffer leaks.

Change-Id: If14311d188f1a2048c11b5bb6186dbf5fc20c1f5


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

[Ami GONE FROM CHROMIUM, 2012-08-11 04:29:00]

LGTM % nits
Nice catch!

https://chromiumcodereview.appspot.com/10837212/diff/1/content/common/gpu/med...
File content/common/gpu/media/omx_video_decode_accelerator.cc (right):

https://chromiumcodereview.appspot.com/10837212/diff/1/content/common/gpu/med...
content/common/gpu/media/omx_video_decode_accelerator.cc:367: if
(current_state_change_ == RESETTING)
|| DESTROYING || ERRORING

https://chromiumcodereview.appspot.com/10837212/diff/1/content/common/gpu/med...
content/common/gpu/media/omx_video_decode_accelerator.cc:595: if
(!output_buffers_at_component_)
Does you change obviate the need for this?  (allow replacing the if with a
DCHECK(!output_buffers_at_component_); followed by an unconditional FOB())

https://chromiumcodereview.appspot.com/10837212/diff/1/content/common/gpu/med...
content/common/gpu/media/omx_video_decode_accelerator.cc:811: if
(current_state_change_ != DESTROYING)
Make this unconditional.

[piman, 2012-08-11 04:51:49]

LGTM if Ami/Pawel are happy

[marcheu, 2012-08-13 19:23:04]

https://chromiumcodereview.appspot.com/10837212/diff/1/content/common/gpu/med...
File content/common/gpu/media/omx_video_decode_accelerator.cc (right):

https://chromiumcodereview.appspot.com/10837212/diff/1/content/common/gpu/med...
content/common/gpu/media/omx_video_decode_accelerator.cc:367: if
(current_state_change_ == RESETTING)
On 2012/08/11 04:29:00, Ami Fischman wrote:
> || DESTROYING || ERRORING

Done.

https://chromiumcodereview.appspot.com/10837212/diff/1/content/common/gpu/med...
content/common/gpu/media/omx_video_decode_accelerator.cc:595: if
(!output_buffers_at_component_)
On 2012/08/11 04:29:00, Ami Fischman wrote:
> Does you change obviate the need for this?  (allow replacing the if with a
> DCHECK(!output_buffers_at_component_); followed by an unconditional FOB())

It might be that this is the case.

With the OMX implementation I tried, the OMX_FillThisBuffer call failed and
returned an error. If instead (with another OMX implementation) it succeeds,
then we'd have ended up there with pending buffers >0.

So yes, we might want to do what you describe in a separate CL, but definitely
not in this CL which I want to backport to M22 :)

https://chromiumcodereview.appspot.com/10837212/diff/1/content/common/gpu/med...
content/common/gpu/media/omx_video_decode_accelerator.cc:811: if
(current_state_change_ != DESTROYING)
On 2012/08/11 04:29:00, Ami Fischman wrote:
> Make this unconditional.

Done.

[Ami GONE FROM CHROMIUM, 2012-08-13 21:38:25]

still LGTM :)

[commit-bot: I haz the power, 2012-08-13 21:40:19]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/marcheu@chromium.org/10837212/6001

[commit-bot: I haz the power, 2012-08-14 00:10:52]

Change committed as 151396