1231257 - Flush individual port instead of OMX_ALL

Status: RESOLVED FIXED

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

Description

[Alfredo Yang (:alfredo)]

During developing of video decoding bug 1224889, I found some omx components doesn't event OMX_ALL event back when flush with port OMX_ALL (flush all of port).
Instead, it send flush complete event for each port separately.

To have better compatibility, we should flush each port instead of OMX_ALL.

Comment 1

[Alfredo Yang (:alfredo)]

Attachment: change_omx_all_flush

Comment 2

[Sotaro Ikeda [:sotaro]]

By the way, codeaurora omx have problem around OMX_CommandFlush. See Bug 1101826.

Comment 3

[Alfredo Yang (:alfredo)]

(In reply to Sotaro Ikeda [:sotaro] from comment #2)
> By the way, codeaurora omx have problem around OMX_CommandFlush. See Bug
> 1101826.

hmm... it looks like some components even failed to handle flushing inport and outport continuously.
To address the problem like bug 1101826, we can't send flush command before previous flush is completed.
Is it correct?

Comment 4

[Alfredo Yang (:alfredo)]

Attachment: change_omx_all_flush

Flush port one by one before previous flush is completed.

Comment 5

[Sotaro Ikeda [:sotaro]]

(In reply to Alfredo Yang (:alfredo) from comment #3)
> (In reply to Sotaro Ikeda [:sotaro] from comment #2)
> > By the way, codeaurora omx have problem around OMX_CommandFlush. See Bug
> > 1101826.
> 
> hmm... it looks like some components even failed to handle flushing inport
> and outport continuously.
> To address the problem like bug 1101826, we can't send flush command before
> previous flush is completed.
> Is it correct?

It should be OK. But it could cause the problem in some il components like Bug 1101826.

The il component in Bug 1101826 actually flush all ports when one port flush is requested. Then if another port flush command is sent before the previous one`s complete, it could not handle both command correctly in some timing cases.

Comment 6

[Sotaro Ikeda [:sotaro]]

Comment on attachment 8697980 [details] [diff] [review]
change_omx_all_flush

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

Don't we need to check if aParam1 is OMX_ALL?

Comment 7

[Alfredo Yang (:alfredo)]

(In reply to Sotaro Ikeda [:sotaro] from comment #6)
> Comment on attachment 8697980 [details] [diff] [review]
> change_omx_all_flush
> 
> Review of attachment 8697980 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Don't we need to check if aParam1 is OMX_ALL?

We do.
OMX_ALL is actually 0xFFFFFFFF, so a OMX_ALL will divide into 2 flush commands, OMX_DirInput and OMX_DirOutput respectively.
And then adding to mFlushCommands list to perform flush one by one.

Comment 8

[Alfredo Yang (:alfredo)]

Attachment: change_omx_all_flush

Fixed the correct OMX_ALL. And make sure each port is flushed.

Comment 9

[Sotaro Ikeda [:sotaro]]

Comment on attachment 8699346 [details] [diff] [review]
change_omx_all_flush

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

Looks good! Port type handling becomes clear.

Comment 10

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/ff2cd5efa012

Comment 11

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/ff2cd5efa012