878981 - [Video] Caf ics_chocolates' OMXCodec does not handle seek failure correctly

Status: RESOLVED WORKSFORME

(Firefox OS Graveyard :: General, defect)

Description

[Sotaro Ikeda [:sotaro]]

+++ This bug was initially created as a clone of Bug #868806 +++

When store attachment 745627 [details] in SD card and open Video app on v1.1 unagi, thumbnail generation continues indefinitely without success as in Bug 868806.

It happens by the caf's change to OMXCodec in ics_chocolate. This problem does not happen in v1.1 leo(ics_strawberry).

Comment 1

[Sotaro Ikeda [:sotaro]]

Attachment: stack trace of the infinite loop

get stack trace by using gdb.

Comment 2

[Sotaro Ikeda [:sotaro]]

OMXCodec destruct stopped in OMXCodec::stop() at following code. The mState was FLUSHING, the FLUSHING is not present in AOSP. It is added in CAF.

    while (isIntermediateState(mState)) {
        mAsyncCompletion.wait(mLock);
    }

Comment 3

[Sotaro Ikeda [:sotaro]]

FLUSHING is set in seek read in OMXCodec::stop(). But nobody unset it in v1.1 unagi case. Then mState remain "IntermediateState" and fall into infinite loop.

Comment 4

[Sotaro Ikeda [:sotaro]]

(In reply to Sotaro Ikeda [:sotaro] from comment #3)
> FLUSHING is set in seek read in OMXCodec::stop(). But nobody unset it in
> v1.1 unagi case. Then mState remain "IntermediateState" and fall into
> infinite loop.

Correction:
 FLUSHING is set in seek read in OMXCodec::read() and it failed.

Comment 5

[Sotaro Ikeda [:sotaro]]

It seems that it is already fixed in ics_strarwberry. In ics_strarwberry case, when seek read in OMXCodec::read() failed, ERROR state is set.

Comment 6

[Sotaro Ikeda [:sotaro]]

Diego, is it already fixed also in ics_chocolate?

Comment 7

[Michael Vines [:m1] [:evilmachines]]

FYI: ics_chocolate is not currently supported for v1.1

Comment 8

[Sotaro Ikeda [:sotaro]]

(In reply to Michael Vines [:m1] [:evilmachines] from comment #7)
> FYI: ics_chocolate is not currently supported for v1.1

Is there a plan for it? inari/ikura uses ics_chocolate. These devices needs to be updated to v1.1 in near future.

Comment 9

[cajbir (:cajbir)]

(In reply to Sotaro Ikeda [:sotaro] from comment #8)
> 
> Is there a plan for it? inari/ikura uses ics_chocolate. These devices needs
> to be updated to v1.1 in near future.

An a lot of us, including the NZ media team, only have unagi's making testing difficult if it's not supported.

Comment 10

[Diego Wilson [:diego]]

(In reply to Sotaro Ikeda [:sotaro] from comment #8)
> (In reply to Michael Vines [:m1] [:evilmachines] from comment #7)
> > FYI: ics_chocolate is not currently supported for v1.1
> 
> Is there a plan for it?

AFAIK: no

Comment 11

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Sotaro, can we patch OMXCodec.cpp locally to avoid this issue on unagis?

Comment 12

[Sotaro Ikeda [:sotaro]]

(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #11)
> Sotaro, can we patch OMXCodec.cpp locally to avoid this issue on unagis?

Yeah, from source code of OMXCodec, there is no problem to use locally.

Comment 13

[Sotaro Ikeda [:sotaro]]

OMXCodec of both unagi and buri devices got from git://codeaurora.org/. Tag is different.

- unagi
https://github.com/mozilla-b2g/b2g-manifest/blob/v1-train/otoro.xml
- buri
https://github.com/mozilla-b2g/b2g-manifest/blob/v1-train/hamachi.xml

Comment 14

[Robert O'Callahan (:roc) (email my personal email if necessary)]

git diff -r eb2bc75803ca179353c24c364a9c8a8ce23e8b78 -r b8676a4e390d1b29ef8b0e9f02642e1aa092ed60 media/libstagefright/OMXCodec.cpp is huge! I don't think I want to try to backport any of that to my build :-(

Comment 16

[Sotaro Ikeda [:sotaro]]

We are not working for ics_chocolate problem anymore.