At the first time video application starts, it retrieves thumbnail from video contents. To avoid black thumbnail, video application seeked to (total duration / 4) position. Then, OMXcodec doesn't receive any frame from video codec for 3 seconds. And also shows log like... E 457 OMXCodec [OMX.qcom.video.decoder.mpeg4] Timed out waiting for output buffers: 0/4 It's really similar as 837051

Adding needinfo on the reporter to get some clarification on the steps a) Does this happen with specific video formats ? b) Can you attach a sample file ? c) Do you see the black thumbnail after seeking it to a particular position which is same always ? d) Is it reproducible 100% of the time ?

(In reply to bhavana bajaj [:bajaj] from comment #1) > Adding needinfo on the reporter to get some clarification on the steps > a) Does this happen with specific video formats ? It always happen in MPEG4 encoded files regardless of extension. So..if you add 10 MPEG4 files, video application takes more than 30 seconds to load thumbnails. > b) Can you attach a sample file ? I'll check it. > c) Do you see the black thumbnail after seeking it to a particular position which is same always ? Sometimes. But black thumbnail is not related to this issue. Thumbnail is always displayed but it takes useless time. > d) Is it reproducible 100% of the time ? Not all time. But I think more than 80%

(In reply to bhavana bajaj [:bajaj] from comment #1) > Adding needinfo on the reporter to get some clarification on the steps > a) Does this happen with specific video formats ? > b) Can you attach a sample file ? > c) Do you see the black thumbnail after seeking it to a particular position > which is same always ? > d) Is it reproducible 100% of the time ? You can see this problem more easily if you push attached file more than 10 ~

triage: leo+ as 3 sec per video is not acceptable

I flashed MozBuild ROM to leo device. Leo device with MozBuild ROM failed to create mpeg4 hw codec.

I flashed partner build ROM(BuildID 20130425170448) to leo device and stored 40 video files of attachment 742194 [details] on sdcard. But I did not see "OMXCodec [OMX.qcom.video.decoder.mpeg4] Timed out waiting for". Is it still happens?

(In reply to Joe Cheng [:jcheng] from comment #5) > triage: leo+ as 3 sec per video is not acceptable On partner build ROM(leo BuildID 20130425170448), it did not take 3sec per video.

(In reply to Sotaro Ikeda [:sotaro] from comment #8) > (In reply to Joe Cheng [:jcheng] from comment #5) > > triage: leo+ as 3 sec per video is not acceptable > > On partner build ROM(leo BuildID 20130425170448), it did not take 3sec per > video. I had checked this issue and I found that it happen by MPEG4 contents that have b-frame. Then, buffer order of FTD is diffent with that of FTB. And first frame buffer still remain gecko side. I think it can make buffer starvation. I will send you test clip by email because it's little big to attach here.

Thanks for the video clip. I confirmed the timeout on leo by partner build. But I faced the problem to reproduce the bug with MozBuild device. [1] Leo and Buri device with MozBuild ROM can not create mpeg4 hw codec. Although, Unagi and Inari with MozBuild ROM can create the mpeg4 hw codec. [2] All MozBuild ROM failed to parse the b-frame mp4 videos.

[2] of comment #10 happened because the the esds box value was abnormal. It failed by esds's sanity checks. The sanity checks are already removed from stagefright in recent android.

By applying attachment 744238 [details] [diff] [review], I confirmed the problem with mozBuild on Inari v1.0.1.

A buffer starvation happens because the videos have longer inter-dependency between video frames. It could be solved by increasing buffers that hw codec can hold. A strategy to address the problem should be same as Bug 837051.

By Bug 837051, gecko reduces the number of video frames holing within gecko as few as possible. There are no way to reduce from there.

There are other places that we can change. It is OMXCodec. It became apparent in Bug 832653. There are already patch to do it. By applying attachment 743220 [details] [diff] [review], hw codec can get 2 more buffers. I confirmed on Ikura, and confirmed that the following logs did not appeared to the log out. > Timed out waiting for output buffers: 0/4

Nominated to tef. It also happened on Inari device.

The patch moved from Bug 832653.

By attachment 744257 [details] [diff] [review], there are no place to reduce in gecko and OMXCodec. If hw codec requests more, it is necessary to change hw codec side to increase the out video buffers.

mvines, since the Bug 832653 comment #38, I found the good reason to uplift attachment 744257 [details] [diff] [review]. I nominated to tef. How do you think about it?

Inder, can you please take a closer look at attachment 744257 [details] [diff] [review].

(In reply to Sotaro Ikeda [:sotaro] from comment #10) > Thanks for the video clip. I confirmed the timeout on leo by partner build. > But I faced the problem to reproduce the bug with MozBuild device. > > [1] Leo and Buri device with MozBuild ROM can not create mpeg4 hw codec. > Although, Unagi and Inari with MozBuild ROM can create the mpeg4 hw codec. It happened that MozBuild for leo did not include "libOmxMpeg4Dec.so". By adding the library. mpeg4 is decodec also on leo device.

(In reply to Michael Vines [:m1] [:evilmachines] from comment #21) > Inder, can you please take a closer look at attachment 744257 [details] [diff] [review] > [diff] [review]. Looks ok to me. I have asked the OMX experts to opine if they see any issues with it.

Blocks a blocker.

mvines, is it possible to apply the changes to caf?

This seems like a regression on Gecko. I'll take a closer look.

I rethink about attachment 744238 [details] [diff] [review]. It is better to change GonkNativeWindow's min undequeued buffer Count to 0. I am going to check if it works.

(In reply to Sotaro Ikeda [:sotaro] from comment #27) > I rethink about attachment 744238 [details] [diff] [review]. Correction: rethink about attachment 744257 [details] [diff] [review].

Actually, I think it's the other way around. I think we need to *increase* the min undequeued buffers. GonkNativeWindow currently has a min undequeued buffer count of '2'. In fact, I think it should be equal to our video queue size (3) + one rendered buffer = 4.

(In reply to Diego Wilson [:diego] from comment #29) > Actually, I think it's the other way around. I think we need to *increase* > the min undequeued buffers. Increase the hw codec's buffer count needs to be careful. In some hw might not support the count. For example, in unagi camera hw case, when the count increase to 3, camera hw made crash during it's initialization. 2 is used just because it is used as default value in Android.

We may need to have a different min undequeued buffer count for video playback and camera

If I understand correctly, the min undequeued buffer counts means "how many buffers does gecko want to keep outside the decoder (or camera) at any given time". In the case of video playback, we want 3 buffers in the video queue and 1 in the compositor. The decoder then allocates the buffers it needs (for reference and such) *plus* the buffers gecko wants (min undequeued bufs).

(In reply to Diego Wilson [:diego] from comment #31) > We may need to have a different min undequeued buffer count for video > playback and camera Yes, I think so.

(In reply to Diego Wilson [:diego] from comment #32) > If I understand correctly, the min undequeued buffer counts means "how many > buffers does gecko want to keep outside the decoder (or camera) at any given > time". In the case of video playback, we want 3 buffers in the video queue > and 1 in the compositor. If qcom's hw codec provide support for the above. It is a most good way to do it.

What if we make the min undequeued buffer count settable in GonkNativeWindow and we just set it in OmxDecoder?

(In reply to Diego Wilson [:diego] from comment #35) > What if we make the min undequeued buffer count settable in GonkNativeWindow > and we just set it in OmxDecoder? Right now, I am writing the code to do it!

(In reply to Diego Wilson [:diego] from comment #35) > What if we make the min undequeued buffer count settable in GonkNativeWindow > and we just set it in OmxDecoder? Yeah, it could minimize the impact of code change.

I think we're on the right track. Android seems to default stafright to 4 min undequeued buffers: https://www.codeaurora.org/gitweb/quic/la/?p=platform/frameworks/base.git;a=blob;f=include/media/stagefright/SurfaceMediaSource.h;h=b805e82d9cfc1987404ab791ca69d03e7a50a446;hb=refs/heads/b2g/ics_strawberry#l37 And everything else (including camera) to 2 min undequeued buffers: https://www.codeaurora.org/gitweb/quic/la/?p=platform/frameworks/base.git;a=blob;f=include/gui/SurfaceTexture.h;h=07f05cae0ee78287955343af66362adaee218472;hb=refs/heads/b2g/ics_strawberry#l43

(In reply to Diego Wilson [:diego] from comment #38) > I think we're on the right track. > > Android seems to default stafright to 4 min undequeued buffers: > > https://www.codeaurora.org/gitweb/quic/la/?p=platform/frameworks/base.git; > a=blob;f=include/media/stagefright/SurfaceMediaSource.h; > h=b805e82d9cfc1987404ab791ca69d03e7a50a446;hb=refs/heads/b2g/ > ics_strawberry#l37 yeah, it is used also for camera effect. So the ics android's camera also should support 4. but unagi's camera did not support it... I trapped to 2 because of it:-(

Interesting. Then doesn't it make more sense to default it to 4 and let unagi override it if need be?

I locally increased the undequeued buffer to 4 for OMXCodec on v1.0.1 buri. It crashed during video app creating thumbnails.

The patch try to increment undequeued buffer count to 4 only for OMXCode. I tried it on v1.0.1 buri. But it made crash during codec initialization.

(In reply to Diego Wilson [:diego] from comment #40) > Interesting. Then doesn't it make more sense to default it to 4 and let > unagi override it if need be? I checked buri and inari device. Inari also do not support 4. It causes camera app to crash. Buri device seems to support 4. The camera app works in 4. In v1.0.1, camera does not need to use 4 as undequeued buffer count. 2(1 for compositor, 1 for ImageContainer) is enough. But Firefox OS 1.2 is going to support WebRTC, it will request more undequeued buffers.

(In reply to Diego Wilson [:diego] from comment #32) > If I understand correctly, the min undequeued buffer counts means "how many > buffers does gecko want to keep outside the decoder (or camera) at any given > time". In android, the min undequeued buffer counts means how many buffers stay undequeued from ANativeWindow not to block a task in rendering target. ANativeWindow is a buffer source and a rendering target. But in Firefox OS, OMXCodec uses just as the buffer source. Then meaning becomes different in Firefox OS's OMXCodec case. Same comment in Bug 832653 comment #13.

(In reply to Sotaro Ikeda [:sotaro] from comment #41) > I locally increased the undequeued buffer to 4 for OMXCodec on v1.0.1 buri. > It crashed during video app creating thumbnails. In Inari case, video app freezes during thumbnail creation. some thumbnails were generated.

It is better to split an undequeued buffer count problem to new bug. All current v1.0.1 devices do not work correctly on "undequeued buffer=4".

(In reply to Sotaro Ikeda [:sotaro] from comment #46) > It is better to split an undequeued buffer count problem to new bug. All > current v1.0.1 devices do not work correctly on "undequeued buffer=4". created Bug 869443.

(In reply to Sotaro Ikeda [:sotaro] from comment #46) > It is better to split an undequeued buffer count problem to new bug. All > current v1.0.1 devices do not work correctly on "undequeued buffer=4". I agree that this bug may not be necessarily solved by increasing the undequeued buffers. I'll continue that discussion in bug 869433.

(In reply to Diego Wilson [:diego] from comment #48) > (In reply to Sotaro Ikeda [:sotaro] from comment #46) > > It is better to split an undequeued buffer count problem to new bug. All > > current v1.0.1 devices do not work correctly on "undequeued buffer=4". > > I agree that this bug may not be necessarily solved by increasing the > undequeued buffers. I'll continue that discussion in bug 869433. s/bug 869433/bug 869443

Attachment 744257 [details] [diff] does not solve the mpeg4 test clip freeze on my device. In addition to the "GetAmpleVideoFrames() to 3" patch, do I also need attachment 744238 [details] [diff] [review] or some other patch?

> Attachment 744257 [details] [diff] does not solve the mpeg4 test clip freeze > on my device. In addition to the "GetAmpleVideoFrames() to 3" patch, do I > also need attachment 744238 [details] [diff] [review] or some other patch? I confirmed video clips in comment #9 received by email. I confirmed that attachment 742194 [details] video clip freezes.

Correction: last week, I used video clips in comment #9 for confirmatin and did not check attachment 742194 [details].

(In reply to Sotaro Ikeda [:sotaro] from comment #52) > Correction: > last week, I used video clips in comment #9 for confirmatin And the above video clips failed at MediaDB in today's v.1.0.1 :-( E GeckoConsole: Content JS WARN at app://video.gaiamobile.org/gaia_build_defer_index.js:215 in metadataError: MediaDB: error parsing metadata for Movies/test3.mp4 : test3

(In reply to Sotaro Ikeda [:sotaro] from comment #53) > And the above video clips failed at MediaDB in today's v.1.0.1 :-( It might be regression of Bug 856542.

(In reply to Sotaro Ikeda [:sotaro] from comment #51) > I confirmed that attachment 742194 [details] video clip freezes. Sorry, I forgot to apply patches. confirm again on buri.

(In reply to Sotaro Ikeda [:sotaro] from comment #55) > (In reply to Sotaro Ikeda [:sotaro] from comment #51) > > I confirmed that attachment 742194 [details] video clip freezes. > > Sorry, I forgot to apply patches. confirm again on buri. Confirmed that it fixes it or that it still freezes?

Confirmed the it fixed on attachment 742194 [details] on buri. But Bug 856542 still exist. During thumbnail creation by video app, try to start video failed with black screen.

(In reply to Sotaro Ikeda [:sotaro] from comment #53) > And the above video clips failed at MediaDB in today's v.1.0.1 :-( By updated ROM, the above videos' thumbnails are generated without timeout and can be playback.

On today's ROM it seems that video playback soon after thumbnail generation failed by some reason.

On inari device, I confirmed the patches works.

OK, I confirmed attachment 744257 [details] [diff] [review] fixes the freeze in the mpeg4 test clip after applying the patch in Bug 863441. I just need a little time to understand why. It seems to be addressing a symptom of a larger problem.

Comment on attachment 745999 [details] [diff] [review] patch - increment undequeued buffer count to 4 for OMXCodec The patch moved to Bug 869443.

The Video app seeks ahead to take an initial screenshot because it is common for videos to begin with a black frame. The spec always said to seek ahead something like 10 seconds. The code that seeks ahead 25% of the duration is a bug. I've recently uplifted a big Video app patch to v1-train. As part of that patch, I removed the seek to 25% code. Now it seeks ahead 5 seconds or one tenth of the video duration, whichever is shorter. If seeking ahead a shorter amount will solve the problem here, you could just do that. See bug 856542. It is probably too big a patch to uplift all of it, but you could copy the code that changes the seek-ahead duration.

I confirmed that the hw codec resource contention happened in following use case. It is not clear the relationship to Bug 856542. I am going to create a new bug to track this. - Store 40 attachment 742194 [details] videos in SD card. - Start video playback during thumbnails are being generated by video app. ROM: custom ROM v1.0.1 for buri. gecko source(May/08) and apply attachment 744238 [details] [diff] [review] and attachment 744257 [details] [diff] [review].

mvines, is it OK to apply the changes to caf? The changes are necessary to playback mpeg4 videos since Bug 869443 was fixed.

(In reply to Sotaro Ikeda [:sotaro] from comment #65) > mvines, is it OK to apply the changes to caf? The changes are necessary to > playback mpeg4 videos since Bug 869443 was fixed. Correction: The changes are necessary to playback mpeg4 videos since Bug 863441 was fixed.

Bug 863441 isn't tef+ so why does this bug depend on it? But please work with Diego on this, he can land a patch on CAF if needed.

Sotaro, It seems the prob is that the OMX codec doesn't accept the buffers it undequeues on initialization. That's why skipping the undequeues (your path) works. I'm still investigating why. But I think skiping the undequeue will have some bad side-effects. Let me investigate further.

Hmm, so I just confirmed the above by doing this: if I set min undequeued buffers to zero here: https://mxr.mozilla.org/mozilla-b2g18/source/dom/camera/GonkNativeWindow.h#55 The mpeg4 test clip plays fine! But as I mentioned earlier, I think we do want at least some min undequeued buffers otherwise gecko's video queue may starve the OMX decoder. Then the question I'm need to answer is, why does OMXCodec fail to use min undequeued buffers? When I figure that out we'll have a proper solution.

Since it seems like Diego is investigating, assigning to him. Feel free to correct if that's wrong.

(In reply to Diego Wilson [:diego] from comment #68) > But I think skiping the undequeue will have some bad side-effects. Let me investigate further. Diego, What is the bad side-effect? By applying attachment 744257 [details] [diff] [review], the undequeue count works in OmxCodec just as to increase the video out buffer only when nBufferCountActual is less than (nBufferCountMin + minUndequeuedBufs). http://androidxref.com/4.0.4/xref/frameworks/base/media/libstagefright/OMXCodec.cpp#1913

In android, the undequeue count is used to return buffers to ANativeWindow(SurfaceTexture). ANativeWindow is also rendering target. Returning buffers to ANativeWindow gives enough buffers to rendering target to handle rendering. Returned buffers's states are set as OWNED_BY_NATIVE_WINDOW. They are used in future by calling OMXCodec::dequeueBufferFromNativeWindow(). But ANatieWindow continues to keep the undequeue count number of video buffers. New rendered buffers are going to be in ANatieWindow. http://androidxref.com/4.0.4/xref/frameworks/base/media/libstagefright/OMXCodec.cpp#2009

In android, rendered video buffers are queued to ANativeWindow in AwesomeNativeWindowRenderer::render(). http://androidxref.com/4.0.4/xref/frameworks/base/media/libstagefright/AwesomePlayer.cpp#132

(In reply to Diego Wilson [:diego] from comment #69) > Then the question I'm need to answer is, why does OMXCodec fail to use min > undequeued buffers? When I figure that out we'll have a proper solution. As in comment #72, default OMXCodec also uses min undequeued buffers they are canceled in OMXCodec::allocateOutputBuffersFromNativeWindow(). But the OMXCodec keep min undequeued count of buffers in ANativeWindow.

I found there is a problem in Firefox OS v1.1 to dequeue all buffers from GonkNativeWindow. There is a guard to remainthe undequeue count of buffers. It is not a problem on v1.0.1. It is in GonkNativeWindow::dequeueBuffer(). http://mxr.mozilla.org/mozilla-b2g18/source/dom/camera/GonkNativeWindow.cpp#272

(In reply to Sotaro Ikeda [:sotaro] from comment #75) > I found there is a problem in Firefox OS v1.1 to dequeue all buffers from > GonkNativeWindow. There is a guard to remainthe undequeue count of buffers. > It is not a problem on v1.0.1. From v1.0.1 -> v1.1, GonkNativeWindow is changed significantly in Bug 803471.

tef+ -> tef? This bug was tef+'d at comment 24 as "blocks a blocker", but I now see that all tef+ bugs that this bug blocks are verified...

Without this bug's fix, some youtube videos playback are not smooth and failed to playback some mpeg 4 videso as in following comment. - Bug 863441 comment #68 - Bug 863441 comment #92

Ah, ok. Bug 863441 should not of been marked fixed yet then I guess. Back to tef+, carry on :)

(In reply to Sotaro Ikeda [:sotaro] from comment #74) > As in comment #72, default OMXCodec also uses min undequeued buffers they > are canceled in OMXCodec::allocateOutputBuffersFromNativeWindow(). But the > OMXCodec keep min undequeued count of buffers in ANativeWindow. You're right! Any undequeued buffers kept by ANativeWindow have no purpose in Gecko. So I guess we're just usint minundequeuedbuffers as a way to force OMXCodec to allocate the extra buffers needed by Gecko for it's video queue. I'll work on pulling in attachment 744257 [details] [diff] [review] to CAF.

(In reply to Diego Wilson [:diego] from comment #80) > > I'll work on pulling in attachment 744257 [details] [diff] [review] to CAF. Thanks!

(In reply to Sotaro Ikeda [:sotaro] from comment #75) > I found there is a problem in Firefox OS v1.1 to dequeue all buffers from > GonkNativeWindow. There is a guard to remainthe undequeue count of buffers. > It is not a problem on v1.0.1. I rechecked the GonkNativeWindow's code. It is not a problem also in v1.1. The check only hit if it is used by camera. GonkCameraHardware calls GonkNativeWindow::getCurrentBuffer(). It enables the check. The function is a Firefox OS specific function and OMXCodec does not use it.

camera hal still uses ANativeWindow as buffer source and rendering target. It is different from OMXCodec.

Comment on attachment 744238 [details] [diff] [review] patch - disable ESDS sanity check set to obsolete, because the patches do not related to the bug.

(In reply to Diego Wilson [:diego] from comment #80) > (In reply to Sotaro Ikeda [:sotaro] from comment #74) > > As in comment #72, default OMXCodec also uses min undequeued buffers they > > are canceled in OMXCodec::allocateOutputBuffersFromNativeWindow(). But the > > OMXCodec keep min undequeued count of buffers in ANativeWindow. > > You're right! Any undequeued buffers kept by ANativeWindow have no purpose > in Gecko. So I guess we're just usint minundequeuedbuffers as a way to force > OMXCodec to allocate the extra buffers needed by Gecko for it's video queue. > > I'll work on pulling in attachment 744257 [details] [diff] [review] to CAF. In SR, I mentioned number of buffer that H/W codec is using. (OMX codec calls useBuffer 12 times with 12 allocated buffer but H/W codec accually uses 3~4 of them.) Is it possible for all timeout problems to be affected by that?

The patch has been released through CAF: https://www.codeaurora.org/gitweb/quic/b2g/?p=b2g/build.git;a=commit;h=8c3ae1b4fb15d48205c42dd7967641c22c779862 It is available to v1.1 device vendors starting release AU_LINUX_GECKO_ICS_STRAWBERRY.01.01.00.019.093 I'll have v1 details shortly

mwu, can you apply the patch in comment #86 to MozBuild? The patch is for OMXCodec in android stagefright. I think it is enough only for ICS_STRAWBERRY.

(In reply to Sotaro Ikeda [:sotaro] from comment #87) > mwu, can you apply the patch in comment #86 to MozBuild? The patch is for > OMXCodec in android stagefright. I think it is enough only for > ICS_STRAWBERRY. Can you submit a pull request on the gonk-patches repo?

Ok, I will submit the pull request.

For v1.0.1 the patch is available in CAF for device vendors starting release AU_LINUX_GECKO_ICS_STRAWBERRY_V1.01.00.01.019.107 https://www.codeaurora.org/gitweb/quic/b2g/?p=b2g/build.git;a=commit;h=2c201fbe726441f4f4c3626873bf23f0a8979d6e

As per sotaro's comment here: https://bugzilla.mozilla.org/show_bug.cgi?id=870564#c22 it seems this patch needs to be pulled in by mozilla's github.

Or rather, Mozilla's vendor build needs to be updated? mwu?

mwu is going review it. someone needs to send a pull request.

I'm just wondering if gonk-patches is even the right place for this. I don't know enough about how Mozilla constructs its device builds.

gonk-patches is appropriate for patches coming from CAF's build repo. We use that *only* for patches from CAF. However, our QA testing is going over to vendor builds + gecko/gaia/ril replacement, so this would be largely for our developers.

So what's the next step here? Is CAF expected to create a new gonk-patches pull request for each new patch?

(sounds like the work remaining here just to make the patch from comment 90 available to moz builds so clearing assignee. Perhaps this isn't tef+ anymore)

(In reply to Diego Wilson [:diego] from comment #97) > So what's the next step here? Is CAF expected to create a new gonk-patches > pull request for each new patch? This is a little unusual since this is the first patch we've needed to add since we imported the patch set. If you submit a pull request, I can review it. There's no specific expectations here since we've never done this before.

how about keep this bug closed and file another bug to track comment 90

The patch comment 90 is the one and only patch coming out of this bug. The remaining action is to land it in the appropriate repos/branches. It makes sense to keep tracking that in this bug.

mwu, I use CAF source for my gonk builds. I think it makes more sense for a mozilla-b2g/gonk-patches user (sotaro maybe?) to make the pull request since he/she would already have the right setup for testing it. Also, if this is how Mozilla will consume all CAF patches going forward, I assume just leaving the bug open until the patch lands on gonk-patches should be good enough for tracking. Otherwise we'll need a new flag or something.

I'll land this.

Hi mwu, Tara also need this patch. I suggest that mozilla can maintain vendor's patches and integrate to build system, otherwise vendors should push the patches into their own repo. If the patches are still in quic/lf/b2g/build repo, people can't fetch them by config.sh.

the patch is enabled on MozBuild for hamachi(buri) and leo.

Can you please provide steps to verify this fix - as we can perform blackbox testing from the UI?