+++ This bug was initially created as a clone of Bug #863441 +++ This bug is created as to investigate the youtube video freeze when data connection is relatively slow as in Bug 863441 comment #105.

This is a platform issue, not a 3rd party app issue.

Deadlock happened during youtube video playback like following STR. - [1] OMXCodec thread is stopped on data read from ChannelMediaResource. The thread continues to hold OMXCodec's mutex. - [2] nsBuiltinDecoderStateMachine's StateMachineThread is stoped to wait OMXCodec's mutex. It happens when the thread try to return video buffer to OMXCodec. The thread continues to hold nsBuiltinDecoder's ReentrantMonitor. - [3] The main thread is stopped to wait nsBuiltinDecoder's ReentrantMonitor. - [4] OMXCodec thread do not receive data forever from hannelMediaResource. Data is delivered via main thread.

(In reply to Sotaro Ikeda [:sotaro] from comment #4) > Created attachment 749474 [details] [diff] [review] > patch - fix deadlock during youtube video playback android::ALooper creates a thread for messaging. By using the ALooper, VideoGraphicBuffer::Unlock() prevents to wait OMXCodex's mutex.

Comment on attachment 749474 [details] [diff] [review] patch - fix deadlock during youtube video playback doublec, can you review the patch soon? It is tef+ bug. Thanks.

(In reply to Sotaro Ikeda [:sotaro] from comment #6) > Comment on attachment 749474 [details] [diff] [review] > patch - fix deadlock during youtube video playback > > doublec, can you review the patch soon? It is tef+ bug. Thanks. Is there any documentation on the API's you're using? I'm not familiar with them and it's hard to review. Can you provide comments/description in the code explaining what they do if not?

I do not know if there is a documentation about it. It mimics android Java's Looper. The way to use is similar to it. http://developer.android.com/reference/android/os/Looper.html http://developer.android.com/reference/android/os/Handler.html

I am going to add comments tomorrow.

(In reply to Sotaro Ikeda [:sotaro] from comment #10) > Created attachment 750088 [details] [diff] [review] > Part1 v2 - fix deadlock during youtube video playback Added some comments.

Comment on attachment 750090 [details] [diff] [review] Part2 v2 - add libstagefright_foundation You'll need to get a reviewer who is a peer for this component to review this, sorry.

Comment on attachment 750088 [details] [diff] [review] Part1 v2 - fix deadlock during youtube video playback Review of attachment 750088 [details] [diff] [review]: ----------------------------------------------------------------- Do you know if the same issue is in Android and would need to be made to media/omx-plugin/OmxDecoder.cpp? ::: content/media/omx/OmxDecoder.h @@ +114,5 @@ > // OMXCodec does not accept MediaBuffer during seeking. If MediaBuffer is > // returned to OMXCodec during seeking, OMXCodec calls assert. > Vector<MediaBuffer *> mPendingVideoBuffers; > + // The lock protects mPendingVideoBuffers. > + // Do not hold Mutex mPendingVideoBuffersLock -> mSeekLock order. I don't understand what this comment is saying. Is it saying that you shouldn't hold both locks? @@ +126,5 @@ > // Holding time should be minimum. > Mutex mSeekLock; > > + // ALooper is a message loop used in stagefright. > + // It creates a thread for messges and handles messages in the thread. Spelling of 'messges'.

(In reply to Chris Double (:doublec) from comment #14) > Comment on attachment 750088 [details] [diff] [review] > Part1 v2 - fix deadlock during youtube video playback > > Review of attachment 750088 [details] [diff] [review]: > ----------------------------------------------------------------- > > Do you know if the same issue is in Android and would need to be made to > media/omx-plugin/OmxDecoder.cpp? omx-plugin does not have this problem. It always copy video data within decode thread and return MediaBuffer to OXMCodec in the thread. So it does not lock StateMachineThread and ImageBridge thread. > > ::: content/media/omx/OmxDecoder.h > @@ +114,5 @@ > > // OMXCodec does not accept MediaBuffer during seeking. If MediaBuffer is > > // returned to OMXCodec during seeking, OMXCodec calls assert. > > Vector<MediaBuffer *> mPendingVideoBuffers; > > + // The lock protects mPendingVideoBuffers. > > + // Do not hold Mutex mPendingVideoBuffersLock -> mSeekLock order. > > I don't understand what this comment is saying. Is it saying that you > shouldn't hold both locks? I just say, mSeekLock need to be hold first when the code want to hold both locks. I think, the comment is not necessary. I am going to remove it. > > @@ +126,5 @@ > > // Holding time should be minimum. > > Mutex mSeekLock; > > > > + // ALooper is a message loop used in stagefright. > > + // It creates a thread for messges and handles messages in the thread. > > Spelling of 'messges'. I'll fix it in the next patch.

Comment on attachment 750090 [details] [diff] [review] Part2 v2 - add libstagefright_foundation mwu, can you review the patch?

(In reply to Sotaro Ikeda [:sotaro] from comment #15) > omx-plugin does not have this problem. It always copy video data within > decode thread and return MediaBuffer to OXMCodec in the thread. So it does > not lock StateMachineThread and ImageBridge thread. I think omx-plugin has been completely replaced by content/media/omx. What do you guys think of just getting rid of it? It just creates confusion.

It is about Firefox browser in android.

(In reply to Sotaro Ikeda [:sotaro] from comment #18) > It is about Firefox browser in android. Oh, I see. I think it would be nice if Android and B2G used the same thing to avoid duplicating effort, but I guess that's up to you guys.

Committable patch. Carry "chris.double: review+".

Committable patch. Carry "mwu: review+ ".

Committable patch. Carry "chris.double: review+".

Comment on attachment 750698 [details] [diff] [review] Part1 v3 b2g18 - fix deadlock during youtube video playback Committable patch. Carry "chris.double: review+".

Committable patch. Carry "mwu: review+ ".

Committable patch. Carry "chris.double: review+".

Committable patch. Carry "mwu: review+ ".

https://hg.mozilla.org/projects/birch/rev/b5a42a30e193 https://hg.mozilla.org/projects/birch/rev/2cad9e70289a

https://hg.mozilla.org/mozilla-central/rev/b5a42a30e193 https://hg.mozilla.org/mozilla-central/rev/2cad9e70289a

https://hg.mozilla.org/releases/mozilla-b2g18/rev/af11c849013a https://hg.mozilla.org/releases/mozilla-b2g18/rev/88d32aa777f5 https://hg.mozilla.org/releases/mozilla-b2g18_v1_0_1/rev/41ffa95cf01e https://hg.mozilla.org/releases/mozilla-b2g18_v1_0_1/rev/5927e20d17d6

This issue still reproduces on the Inari Device Build ID: 20130529070211 Environmental Variables: Kernel Date: Apr 25 Gecko: http://hg.mozilla.org/releases/mozilla-b2g18/rev/6ca32ed2bbc6 Gaia: 8f5ab7bfd4a2921aab4e2de11e0d79a29c1bb062 Video freezes after a few minutes of watching a youtube video with slow data. I was also able to reproduce bug#863441 where the video will freeze on a green screen. This issue was verified fixed so I can create a new bug or reopen the closed one.

There's already a bug on this - bug 876843.

(In reply to Sarah Parsons from comment #31) > Video freezes after a few minutes of watching a youtube video with slow > data. I was also able to reproduce bug#863441 where the video will freeze on > a green screen. This issue was verified fixed so I can create a new bug or > reopen the closed one. Green screen might be fixed by Bug 877400. In this case, video playback ended by error in decoding or networking. So it should not be dead lock. Just ended video playback because of error.

Video does not freeze after a few minutes (was able to play a 3 minute video with no interruption) of watching a Youtube video with slow data on Inari (H connectivity). This issue does not reproduces on the Inari v1.0.1 (tested on 2 different builds) Inari Build ID: Build ID: 20130529070211 Gecko: http://hg.mozilla.org/releases/mozilla-b2g18_v1_0_1/rev/f379dc8c9181 Gaia: ac293ce59acc3bede083fad1b973794fa8bf0253 Inari Build ID: 20130606070202 Gecko: http://hg.mozilla.org/releases/mozilla-b2g18_v1_0_1/rev/e329fbd2d2ed Gaia: 37b6527c3f73a497f75d057e90386f77ff5a552b Due to connectivity issue unable to verify for v1.1 on Unagi (only Edge was available. Unable to verify on Leo as Blocked by this bug https://bugzilla.mozilla.org/show_bug.cgi?id=859260.