1215441 - Skip flush before Init() is completed

Status: RESOLVED FIXED

(Core :: Audio/Video: Playback, defect)

Description

[Alfredo Yang (:alfredo)]

It causes OMX returning error.

Comment 1

[Alfredo Yang (:alfredo)]

Attachment: skip_flush_before_init_complete

Comment 2

[Alfredo Yang (:alfredo)]

Thanks for quick review!

Comment 3

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/7766307e4759

Comment 4

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/7766307e4759

Comment 5

[Jean-Yves Avenard [:jya]]

Comment on attachment 8674750 [details] [diff] [review]
skip_flush_before_init_complete

Review of attachment 8674750 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/media/platforms/gonk/GonkMediaDataDecoder.cpp
@@ +103,5 @@
>      GMDD_LOG("Decoder is not initialized");
>      return NS_ERROR_UNEXPECTED;
>    }
> +
> +  if (!mInitPromise.IsEmpty()) {

this doesn't appear thread-safe.
this will be called on the reader's taskqueue, while mInitPromise is typically resolved on the ACodec's TaskQueue.

So the InitPromise is accessed via two different threads without locking.

Comment 6

[Alfredo Yang (:alfredo)]

(In reply to Jean-Yves Avenard [:jya] from comment #5)
> Comment on attachment 8674750 [details] [diff] [review]
> skip_flush_before_init_complete
> 
> Review of attachment 8674750 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: dom/media/platforms/gonk/GonkMediaDataDecoder.cpp
> @@ +103,5 @@
> >      GMDD_LOG("Decoder is not initialized");
> >      return NS_ERROR_UNEXPECTED;
> >    }
> > +
> > +  if (!mInitPromise.IsEmpty()) {
> 
> this doesn't appear thread-safe.
> this will be called on the reader's taskqueue, while mInitPromise is
> typically resolved on the ACodec's TaskQueue.
> 
> So the InitPromise is accessed via two different threads without locking.

I think mInitPromise is resolved in reader task queue.

https://dxr.mozilla.org/mozilla-central/rev/1a157155a4fe0074b3d03b54fe9e466472c2cd56/dom/media/platforms/gonk/GonkVideoDecoderManager.cpp#121