Video playback broken for h264 on Nexus S

RESOLVED WONTFIX

Status

defect
RESOLVED WONTFIX
7 years ago
Last year

People

(Reporter: gerard-majax, Unassigned)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(8 attachments, 5 obsolete attachments)

108.87 KB, text/plain
Details
2.06 MB, video/mp4
Details
11.05 KB, text/plain
Details
1.69 KB, patch
eflores
: review+
Details | Diff | Splinter Review
1006 bytes, patch
Details | Diff | Splinter Review
1.04 KB, patch
Details | Diff | Splinter Review
335.04 KB, image/png
Details
1.24 KB, patch
Details | Diff | Splinter Review
Trying to play any H264 video on Nexus S fails, either Youtube or Camera-recorded video.
Looking at logcat, one can only see:
I/OMXCodec( 1495): [OMX.SEC.AVC.Decoder] AVC profile = 66 (Baseline), level = 30
I/OMXCodec( 1495): [OMX.SEC.AVC.Decoder] video dimensions are 640 x 360
W/SoftAAC ( 1495): Sample rate was 44100 Hz, but now is 22050 Hz
I/OMXCodec( 1495): [OMX.SEC.AVC.Decoder] video dimensions are 640 x 368
I/OMXCodec( 1495): [OMX.SEC.AVC.Decoder] Crop rect is 640 x 360 @ (0, 0)
E/OMXCodec( 1495): [OMX.SEC.AVC.Decoder] Timed out waiting for output buffers: 0/3
E/OMXCodec( 1495): [OMX.SEC.AVC.Decoder] Timed out waiting for output buffers: 0/3
E/OMXCodec( 1495): [OMX.SEC.AVC.Decoder] Timed out waiting for output buffers: 0/3
Forgot to mention that this happens inside the Video application.
There is something with buffers, but basically, this change makes hardware decoding working:

alex@portable-alex:~/codaz/B2G/frameworks/base/media/libstagefright$ git diff OMXCodec.cpp
diff --git a/media/libstagefright/OMXCodec.cpp b/media/libstagefright/OMXCodec.cpp
index 86b3fe4..97d0491 100755
--- a/media/libstagefright/OMXCodec.cpp
+++ b/media/libstagefright/OMXCodec.cpp
@@ -1980,10 +1980,12 @@ status_t OMXCodec::allocateOutputBuffersFromNativeWindow() {
         cancelEnd = def.nBufferCountActual;
     }
 
+    /*
     for (OMX_U32 i = cancelStart; i < cancelEnd; i++) {
         BufferInfo *info = &mPortBuffers[kPortIndexOutput].editItemAt(i);
         cancelBufferToNativeWindow(info);
     }
+    */
 
     return err;
 }

I'm missing the understanding of the gecko codebase and its interactions with OMX on B2G, so I can't explain. Someone with better knowledges could probably give us some help.
This is the log of OMXCodec for SEC.AVC.Decoder. One can see only one FILL happens
Video showing the playback of Big Buck Bunny through OMX.SEC.AVC.Decoder (as the logcat shows).
It also triggers a crash at the end of playback/thumbnail generation:

Program received signal SIGSEGV, Segmentation fault.
android::GraphicBufferMapper::unlock (this=<value optimized out>, handle=<value optimized out>) at frameworks/base/libs/ui/GraphicBufferMapper.cpp:86
86	    err = mAllocMod->unlock(mAllocMod, handle);
(gdb) bt
#0  android::GraphicBufferMapper::unlock (this=<value optimized out>, handle=<value optimized out>) at frameworks/base/libs/ui/GraphicBufferMapper.cpp:86
#1  0x419a6b20 in android::GraphicBuffer::unlock (this=0x42868000) at frameworks/base/libs/ui/GraphicBuffer.cpp:181
#2  0x40c27018 in ~GraphicBufferAutoUnlock (this=<value optimized out>) at /home/alex/codaz/B2G/gecko/gfx/layers/GonkIOSurfaceImage.cpp:35
#3  mozilla::layers::GonkIOSurfaceImage::GetAsSurface (this=<value optimized out>) at /home/alex/codaz/B2G/gecko/gfx/layers/GonkIOSurfaceImage.cpp:159
#4  0x40c0bafe in mozilla::layers::ImageContainer::GetCurrentAsSurface (this=0x44346b00, aSize=0xbe849218) at /home/alex/codaz/B2G/gecko/gfx/layers/ImageContainer.cpp:316
#5  0x404c9c2c in nsLayoutUtils::SurfaceFromElement (aElement=0x41a88400, aSurfaceFlags=<value optimized out>) at /home/alex/codaz/B2G/gecko/layout/base/nsLayoutUtils.cpp:4532
#6  0x404ca13e in nsLayoutUtils::SurfaceFromElement (aElement=0x1e0, aSurfaceFlags=4) at /home/alex/codaz/B2G/gecko/layout/base/nsLayoutUtils.cpp:4567
#7  0x4062eaba in nsCanvasRenderingContext2DAzure::DrawImage (this=0x42c50400, image=..., sx=0, sy=0, sw=0, sh=0, dx=0, dy=0, dw=160, dh=160, optional_argc=2 '\002', error=...)
    at /home/alex/codaz/B2G/gecko/content/canvas/src/nsCanvasRenderingContext2DAzure.cpp:3703
#8  0x40b514a4 in nsCanvasRenderingContext2DAzure::DrawImage (cx=0x42c49bf0, obj=<value optimized out>, self=0x42c50400, argc=<value optimized out>, vp=0x42d00088)
    at /home/alex/codaz/B2G/gecko/content/canvas/src/nsCanvasRenderingContext2DAzure.h:275
#9  drawImage (cx=0x42c49bf0, obj=<value optimized out>, self=0x42c50400, argc=<value optimized out>, vp=0x42d00088)
    at /home/alex/codaz/B2G/objdir-gecko/dom/bindings/CanvasRenderingContext2DBinding.cpp:1228
#10 0x40b521f8 in genericMethod (cx=0x42c49bf0, argc=5, vp=0x42d00088) at /home/alex/codaz/B2G/objdir-gecko/dom/bindings/CanvasRenderingContext2DBinding.cpp:2659
#11 0x40e12fbc in CallJSNative (cx=0x42c49bf0, args=..., construct=js::NO_CONSTRUCT) at /home/alex/codaz/B2G/gecko/js/src/jscntxtinlines.h:364
#12 InvokeKernel (cx=0x42c49bf0, args=..., construct=js::NO_CONSTRUCT) at /home/alex/codaz/B2G/gecko/js/src/jsinterp.cpp:367
#13 0x40e10762 in js::Interpret (cx=0x42c49bf0, entryFrame=<value optimized out>, interpMode=<value optimized out>) at /home/alex/codaz/B2G/gecko/js/src/jsinterp.cpp:2475
#14 0x40e1286c in js::RunScript (cx=0x42c49bf0, script=<value optimized out>, fp=0x42d00038) at /home/alex/codaz/B2G/gecko/js/src/jsinterp.cpp:324
#15 0x40e13b98 in InvokeKernel (cx=0x42c49bf0, thisv=..., fval=<value optimized out>, argc=<value optimized out>, argv=0xbe849ac8, rval=0xbe849b88)
    at /home/alex/codaz/B2G/gecko/js/src/jsinterp.cpp:378
#16 Invoke (cx=0x42c49bf0, thisv=..., fval=<value optimized out>, argc=<value optimized out>, argv=0xbe849ac8, rval=0xbe849b88) at /home/alex/codaz/B2G/gecko/js/src/jsinterp.h:109
#17 Invoke (cx=0x42c49bf0, thisv=..., fval=<value optimized out>, argc=<value optimized out>, argv=0xbe849ac8, rval=0xbe849b88) at /home/alex/codaz/B2G/gecko/js/src/jsinterp.cpp:411
#18 0x40daf56e in JS_CallFunctionValue (cx=0x42c49bf0, objArg=<value optimized out>, fval=..., argc=1, argv=0xbe849ac8, rval=0xbe849b88) at /home/alex/codaz/B2G/gecko/js/src/jsapi.cpp:5893
#19 0x406f5a18 in nsJSContext::CallEventHandler (this=0x4376dd30, aTarget=<value optimized out>, aScope=<value optimized out>, aHandler=<value optimized out>, aargv=0x42c79050, arv=0xbe849cd4)
    at /home/alex/codaz/B2G/gecko/dom/base/nsJSEnvironment.cpp:1948
#20 0x40755dda in nsJSEventListener::HandleEvent (this=0x4434dc80, aEvent=0x42c6c700) at /home/alex/codaz/B2G/gecko/dom/src/events/nsJSEventListener.cpp:215
#21 0x406473e2 in nsEventListenerManager::HandleEventSubType (this=<value optimized out>, aListenerStruct=<value optimized out>, aListener=0x4434dc80, aDOMEvent=0x42c6c700, aCurrentTarget=0x41a88400, 
    aPhaseFlags=6, aPusher=0xbe849e48) at /home/alex/codaz/B2G/gecko/content/events/src/nsEventListenerManager.cpp:889
#22 0x4064751c in nsEventListenerManager::HandleEventInternal (this=0x443448d0, aPresContext=<value optimized out>, aEvent=0x42ca46c0, aDOMEvent=0xbe849e38, aCurrentTarget=0x41a88400, aFlags=6, 
    aEventStatus=0xbe849e3c, aPusher=0xbe849e48) at /home/alex/codaz/B2G/gecko/content/events/src/nsEventListenerManager.cpp:962
#23 0x40657700 in nsEventListenerManager::HandleEvent (this=<value optimized out>, aVisitor=<value optimized out>, aFlags=6, aMayHaveNewListenerManagers=<value optimized out>, aPusher=0xbe849e48)
    at /home/alex/codaz/B2G/gecko/content/events/src/nsEventListenerManager.h:144
#24 nsEventTargetChainItem::HandleEvent (this=<value optimized out>, aVisitor=<value optimized out>, aFlags=6, aMayHaveNewListenerManagers=<value optimized out>, aPusher=0xbe849e48)
    at /home/alex/codaz/B2G/gecko/content/events/src/nsEventDispatcher.cpp:184
#25 0x406577ec in nsEventTargetChainItem::HandleEventTargetChain (this=<value optimized out>, aVisitor=..., aFlags=6, aCallback=0x0, aMayHaveNewListenerManagers=false, aPusher=0xbe849e48)
    at /home/alex/codaz/B2G/gecko/content/events/src/nsEventDispatcher.cpp:316
#26 0x40657d6e in nsEventDispatcher::Dispatch (aTarget=<value optimized out>, aPresContext=0x43ccd800, aEvent=0x42ca46c0, aDOMEvent=<value optimized out>, aEventStatus=0xbe849ec8, aCallback=0x0, 
    aTargets=0x0) at /home/alex/codaz/B2G/gecko/content/events/src/nsEventDispatcher.cpp:634
#27 0x40657f64 in nsEventDispatcher::DispatchDOMEvent (aTarget=0x41a88400, aEvent=0x42ca46c0, aDOMEvent=0x42c6c700, aPresContext=0x43ccd800, aEventStatus=0xbe849ec8)
    at /home/alex/codaz/B2G/gecko/content/events/src/nsEventDispatcher.cpp:694
#28 0x405f5520 in nsINode::DispatchEvent (this=0x41a88400, aEvent=0x42c6c700, aRetVal=0xbe849f07) at /home/alex/codaz/B2G/gecko/content/base/src/nsINode.cpp:1078
#29 0x405c43fa in nsContentUtils::DispatchEvent (aDoc=<value optimized out>, aTarget=0x41a88400, aEventName=..., aCanBubble=false, aCancelable=false, aTrusted=true, aDefaultAction=0xbe849f07)
    at /home/alex/codaz/B2G/gecko/content/base/src/nsContentUtils.cpp:3547
#30 0x405c443c in nsContentUtils::DispatchTrustedEvent (aDoc=<value optimized out>, aTarget=<value optimized out>, aEventName=<value optimized out>, aCanBubble=<value optimized out>, 
    aCancelable=<value optimized out>, aDefaultAction=0x0) at /home/alex/codaz/B2G/gecko/content/base/src/nsContentUtils.cpp:3518
#31 0x4069ccc0 in nsHTMLMediaElement::DispatchEvent (this=0x41a88400, aName=...) at /home/alex/codaz/B2G/gecko/content/html/content/src/nsHTMLMediaElement.cpp:3353
#32 0x4069cce0 in nsAsyncEventRunner::Run (this=<value optimized out>) at /home/alex/codaz/B2G/gecko/content/html/content/src/nsHTMLMediaElement.cpp:205
---Type <return> to continue, or q <return> to quit---
#33 0x40ba3406 in nsThread::ProcessNextEvent (this=0x41a068e0, mayWait=<value optimized out>, result=0xbe849faf) at /home/alex/codaz/B2G/gecko/xpcom/threads/nsThread.cpp:620
#34 0x40b8382e in NS_ProcessNextEvent_P (thread=0x0, mayWait=false) at /home/alex/codaz/B2G/objdir-gecko/xpcom/build/nsThreadUtils.cpp:237
#35 0x40a9cde8 in mozilla::ipc::MessagePump::Run (this=0x41a02310, aDelegate=0xbe84a8c0) at /home/alex/codaz/B2G/gecko/ipc/glue/MessagePump.cpp:82
#36 0x40a9ce9a in mozilla::ipc::MessagePumpForChildProcess::Run (this=0x41a02310, aDelegate=0xbe84a8c0) at /home/alex/codaz/B2G/gecko/ipc/glue/MessagePump.cpp:231
#37 0x40bc52c0 in MessageLoop::RunInternal (this=0x1000000) at /home/alex/codaz/B2G/gecko/ipc/chromium/src/base/message_loop.cc:216
#38 0x40bc5376 in MessageLoop::RunHandler (this=0xbe84a8c0) at /home/alex/codaz/B2G/gecko/ipc/chromium/src/base/message_loop.cc:209
#39 MessageLoop::Run (this=0xbe84a8c0) at /home/alex/codaz/B2G/gecko/ipc/chromium/src/base/message_loop.cc:183
#40 0x40a230e4 in nsBaseAppShell::Run (this=0x43744100) at /home/alex/codaz/B2G/gecko/widget/xpwidgets/nsBaseAppShell.cpp:163
#41 0x403ba79c in XRE_RunAppShell () at /home/alex/codaz/B2G/gecko/toolkit/xre/nsEmbedFunctions.cpp:646
#42 0x40a9ce68 in mozilla::ipc::MessagePumpForChildProcess::Run (this=0x41a02310, aDelegate=0xbe84a8c0) at /home/alex/codaz/B2G/gecko/ipc/glue/MessagePump.cpp:198
#43 0x40bc52c0 in MessageLoop::RunInternal (this=0x43744100) at /home/alex/codaz/B2G/gecko/ipc/chromium/src/base/message_loop.cc:216
#44 0x40bc5376 in MessageLoop::RunHandler (this=0xbe84a8c0) at /home/alex/codaz/B2G/gecko/ipc/chromium/src/base/message_loop.cc:209
#45 MessageLoop::Run (this=0xbe84a8c0) at /home/alex/codaz/B2G/gecko/ipc/chromium/src/base/message_loop.cc:183
#46 0x403bab40 in XRE_InitChildProcess (aArgc=<value optimized out>, aArgv=<value optimized out>, aProcess=GeckoProcessType_Content) at /home/alex/codaz/B2G/gecko/toolkit/xre/nsEmbedFunctions.cpp:485
#47 0x00008532 in main (argc=5, argv=0xbe84aa24) at /home/alex/codaz/B2G/gecko/ipc/app/MozillaRuntimeMain.cpp:60
Comment on attachment 734392 [details]
OMX SEC AAC Decoder logcat

Actually this is the AAC decoder log, showing process when it works correctly.
Attachment #734392 - Attachment description: OMX SEC AVC Decoder logcat → OMX SEC AAC Decoder logcat
This is the OMX.SEC.AVC decoder log, which makes use of hardware capabilities.
The code for output buffer handling in Stagefright's OMXCodec.cpp has this comment: 

    // XXX: Is this the right logic to use?  It's not clear to me what the OMX
    // buffer counts refer to - how do they account for the renderer holding on
    // to buffers?

Based on that I don't think it accounts for the renderer holding on to buffers. This patch will add an amount equal to the amount of buffers that Gecko holds on to. This amount is the value of MediaOmxStateMachine::GetAmpleVideoFrames() plus one for the MediaOmxReader::mLastVideoFrame reference.

This prevents buffer starvation and allows playback within the browser of H.264 videos. Youtube also works. 

Unfortunately the video app seems to trigger a bug whereby destruction of the video decoder causes a crash in gralloc buffer references. I have not yet found the cause of this.
Assignee: nobody → chris.double
Status: NEW → ASSIGNED
The fix in attachment 734527 [details] [diff] [review] gets stagefright hardware decoding working. Unfortunately it hits a bug in the video app when video instances are destroyed causing a crash in releasing graphic buffer resources. I haven't found the source of this yet so a workaround is to disable thumbnail generation which this patch does.

Videos will then play in the video app but they still crash when you press the "Back" button in the top left. They do not crash when you pause the app, press the home key, or go back. I provide this patch to at least allow video app playback.
(In reply to Chris Double (:doublec) from comment #8)
> Created attachment 734527 [details] [diff] [review]
> Workaround stagefright buffer starvation
> 
> The code for output buffer handling in Stagefright's OMXCodec.cpp has this
> comment: 
> 
>     // XXX: Is this the right logic to use?  It's not clear to me what the
> OMX
>     // buffer counts refer to - how do they account for the renderer holding
> on
>     // to buffers?
> 
> Based on that I don't think it accounts for the renderer holding on to
> buffers. This patch will add an amount equal to the amount of buffers that
> Gecko holds on to. This amount is the value of
> MediaOmxStateMachine::GetAmpleVideoFrames() plus one for the
> MediaOmxReader::mLastVideoFrame reference.
> 
> This prevents buffer starvation and allows playback within the browser of
> H.264 videos. Youtube also works. 
> 
> Unfortunately the video app seems to trigger a bug whereby destruction of
> the video decoder causes a crash in gralloc buffer references. I have not
> yet found the cause of this.

I noticed that it also works with retained = 2, while you use retained = 3 in your code.
I do not have Nexus S. Though the bug might be related to Bug 837051.
(In reply to Chris Double (:doublec) from comment #8)
> Created attachment 734527 [details] [diff] [review]
> Workaround stagefright buffer starvation
> 

The change do not work on unagi. During Bug 837051, I did same thing, but unagi phone failed to allocate the buffers.
(In reply to Alexandre LISSY :gerard-majax from comment #2)
> There is something with buffers, but basically, this change makes hardware
> decoding working:
> 
> alex@portable-alex:~/codaz/B2G/frameworks/base/media/libstagefright$ git
> diff OMXCodec.cpp
> diff --git a/media/libstagefright/OMXCodec.cpp
> b/media/libstagefright/OMXCodec.cpp
> index 86b3fe4..97d0491 100755
> --- a/media/libstagefright/OMXCodec.cpp
> +++ b/media/libstagefright/OMXCodec.cpp
> @@ -1980,10 +1980,12 @@ status_t
> OMXCodec::allocateOutputBuffersFromNativeWindow() {
>          cancelEnd = def.nBufferCountActual;
>      }
>  
> +    /*
>      for (OMX_U32 i = cancelStart; i < cancelEnd; i++) {
>          BufferInfo *info = &mPortBuffers[kPortIndexOutput].editItemAt(i);
>          cancelBufferToNativeWindow(info);
>      }
> +    */
>  
>      return err;
>  }
> 
> I'm missing the understanding of the gecko codebase and its interactions
> with OMX on B2G, so I can't explain. Someone with better knowledges could
> probably give us some help.

The change should work for Firefox OS. It is a good point! I did not care about it until now. In android, ANativeWindow is used as a buffer source and rendering target by OMXCodec. To not block rendering target work, it is necessary to return MinUndequeuedBufs to ANativeWindow. But in Firefox OS, ANativeWindow is used just as buffer source by OMXCodec. Then It is not necessary to return MinUndequeuedBufs to ANativeWindow.
But Camera hal still use ANativeWindow as a buffer source and rendering target in Firefox OS.
(In reply to Sotaro Ikeda [:sotaro] from comment #14)
> But Camera hal still use ANativeWindow as a buffer source and rendering
> target in Firefox OS.

Camera hal does not use the above code, then the change should not affect to camera hal.
(In reply to Alexandre LISSY :gerard-majax from comment #5)
> It also triggers a crash at the end of playback/thumbnail generation:
> 

I am not sure about the crash. Can you see generated thumbnails on Nexus S?
(In reply to Sotaro Ikeda [:sotaro] from comment #16)
> (In reply to Alexandre LISSY :gerard-majax from comment #5)
> > It also triggers a crash at the end of playback/thumbnail generation:
> > 
> 
> I am not sure about the crash. Can you see generated thumbnails on Nexus S?

Nope. And I tried a LOT of times :). The only way to circumvent this is to force use of software codecs only just to generate the thumbnails. So it's not usable in practice, but it gives a workaround for testing purpose.
Did you tried OMXCodec::kClientNeedsFramebuffer flag as in following?
http://androidxref.com/4.0.4/xref/frameworks/base/media/libstagefright/StagefrightMetadataRetriever.cpp#118

I am wondering that it might be failed to map GraphicBuffer on Nexus S.
(In reply to Sotaro Ikeda [:sotaro] from comment #18)
> Did you tried OMXCodec::kClientNeedsFramebuffer flag as in following?
> http://androidxref.com/4.0.4/xref/frameworks/base/media/libstagefright/
> StagefrightMetadataRetriever.cpp#118
> 
> I am wondering that it might be failed to map GraphicBuffer on Nexus S.

I removed the hack from OMXCodec.cpp (libstagefright), added this flag in OmxDecoder.cpp (gecko), rebuild, reflashed system and I'm back to being unable to play via hardware acceleration.
(In reply to Sotaro Ikeda [:sotaro] from comment #18)
> Did you tried OMXCodec::kClientNeedsFramebuffer flag as in following?
> http://androidxref.com/4.0.4/xref/frameworks/base/media/libstagefright/
> StagefrightMetadataRetriever.cpp#118
> 
> I am wondering that it might be failed to map GraphicBuffer on Nexus S.

There is however a slight impact: I saw artifacts on decoding of the first frame :)
(In reply to Sotaro Ikeda [:sotaro] from comment #11)
> I do not have Nexus S. Though the bug might be related to Bug 837051.

Yes, I thought that too. I tried the fix from the bug but it didn't work. It's unfortunate that increasing the buffer count like in my catch breaks on Unagi. Maybe the fix in comment 2 is the better approach. 

I haven't made in progress in the crash stack in comment 5 unfortunately.
(In reply to Sotaro Ikeda [:sotaro] from comment #13)
> The change should work for Firefox OS. It is a good point! I did not care
> about it until now. In android, ANativeWindow is used as a buffer source and
> rendering target by OMXCodec. To not block rendering target work, it is
> necessary to return MinUndequeuedBufs to ANativeWindow. But in Firefox OS,
> ANativeWindow is used just as buffer source by OMXCodec. Then It is not
> necessary to return MinUndequeuedBufs to ANativeWindow.

I've tried the change from coment 2 on an Otoro and it seems to work fine. No crashes when exiting like the Nexus S.
The crash in comment 2 happens when unlocking a GraphicBuffer with an unknown OMX format. We don't need to lock/unlock the buffer when querying if we know the format. Moving the lock/unlock code past the early return that checks the format fixes the crash.
Attachment #734528 - Attachment is obsolete: true
Attachment #734988 - Flags: approval-gaia-v1?
If the thumbnail for an MP4 file can't be generated due to not being able to access the pixel data then duplicate entries for each video appear in the video list. This patch adds a catch for errors to prevent the duplicate entries.
Attachment #734992 - Flags: review?(dkuo)
Comment on attachment 734988 [details] [diff] [review]
Fix GraphicBuffer crash

Oops, wrong flag.
Attachment #734988 - Flags: approval-gaia-v1? → review?(edwin)
Comment on attachment 734992 [details] [diff] [review]
Prevent duplicate entries in video app list when thumbnail generation fails

Removing review for now. Still getting duplicates in some circumstances.
Attachment #734992 - Flags: review?(dkuo)
Changes to the approach from comment 2 after taking into consideration comments from Sotaro regarding Unagi compatibility.
Attachment #734527 - Attachment is obsolete: true
With attachment 735002 [details] [diff] [review] and attachment 734988 [details] [diff] [review] I get no crashes playing videos on Nexus S. Unfortunately I don't get thumbnails - even if I use kClientNeedsFramebuffer I don't get access to the pixel data.
This prevents duplicate entries in the thumbnail list when we can't generate thumbnails. For some reason the exception catch in the video app uses 'console.error' which doesn't seem to exist in B2G.
Attachment #734992 - Attachment is obsolete: true
Woah, that's starting to be working nice now. With those patches:
 - Hardware acceleration works, I can play videos
 - No more crash on thumbnail generation

Yet, as stated, still empty thumbnail. And in logcat, I get:
E/OMXCodec(  535): [OMX.SEC.AVC.Decoder] Timed out waiting for output buffers: 0/3
E/GeckoConsole(  535): Content JS ERROR at app://video.gaiamobile.org/gaia_build_defer_index.js:401 in doneSeeking: Failed to create a poster image: [Exception... "Component is not available"  nsresult: "0x80040111 (NS_ERROR_NOT_AVAILABLE)"  location: "JS frame :: app://video.gaiamobile.org/gaia_build_defer_index.js :: doneSeeking :: line 400"  data: no]

Which kinds of reminds me of bug 832745 and bug 832650.
Duplicate of this bug: 822303
Breakpointing in GonkIOSurfaceImage::GetAsSurface(), I get this:

(gdb) print sColorIdMap 
$3 = {259, 19, 258, 22, 265, 21, 17, 4294967295, 266, 4294967295, 842094169, 19, 0, 0}
(gdb) print format
$4 = 256

i.e., I dont see any mapping between the pixel format returned by graphicBuffer->getPixelFormat(); and something known.
This should handle pixel format conversion. But I don't know why it retriggers the unlock() crash :/
(In reply to Alexandre LISSY :gerard-majax from comment #33)
> Created attachment 735055 [details] [diff] [review]
> Adding some missing color conversion for video thumbnails on Nexus S
> 
> This should handle pixel format conversion. But I don't know why it
> retriggers the unlock() crash :/

Commenting the "unlock()" makes the crash going away (obviously ...), but I still don't see any picture. Yet, at least, I don't have anymore logcat showing some "COMPONENT_NOT_AVAILABLE". So it means the issue is now somewhere else.
(In reply to Alexandre LISSY :gerard-majax from comment #34)
> (In reply to Alexandre LISSY :gerard-majax from comment #33)
> > Created attachment 735055 [details] [diff] [review]
> > Adding some missing color conversion for video thumbnails on Nexus S
> > 
> > This should handle pixel format conversion. But I don't know why it
> > retriggers the unlock() crash :/
> 
> Commenting the "unlock()" makes the crash going away (obviously ...), but I
> still don't see any picture. Yet, at least, I don't have anymore logcat
> showing some "COMPONENT_NOT_AVAILABLE". So it means the issue is now
> somewhere else.

Wait, I reloaded the Video app, thumbnail got generated ! It's not very nice, showing artifacts, but it's there!
Please find attached the thumbnail list, with the first one being OMX.SEC.AVC.Decoder generated. It appeared after restarting video app, and onfirms the issue was color conversion missing. Also had to comment the unlock call to avoid crash.
mvine, from this bug, it becomes clear that attachment 735002 [details] [diff] [review] provide 2 more buffers to gecko. It it better to apply the change. It does not block tef. It is better to change it after v1.1. How do you think about it?
Flags: needinfo?(mvines)
Until we have good reason to uplift to v1.0.1/v1.1 then that sounds good to me.
Flags: needinfo?(mvines)
simpler patch to OMXCodec to fix buffer starvation.
I feel that attachment 743220 [details] [diff] [review] could hanle also error cases.
I move attachment 743220 [details] [diff] [review] to Bug 864230. Because the bug is already leo+ and I nominated to tef. Need to fix it soon.
Attachment #743220 - Attachment is obsolete: true
Comment on attachment 735005 [details] [diff] [review]
Prevent duplicate entries in video app list when thumbnail generation fails

I've spun this out into bug 869289 and rebased to master.
Attachment #735005 - Attachment is obsolete: true
None of those tricks seems to work anymore on current b2g-18 tree.
Status: ASSIGNED → NEW
Status: NEW → ASSIGNED
Reverting back to attachement 734527 from comment #8 fixes again the issue for me.
Blocks: b2g-nexuss
Hey, just found https://github.com/sotaroikeda/platform_frameworks_av/commit/ec46d95f9d369ec883768f8617224b640cf073cb which seems very very similar ...
Flags: needinfo?(sotaro.ikeda.g)
Flags: needinfo?(chris.double)
I defer to Sotaro for changes to this area of the codebase.
Flags: needinfo?(chris.double)
(In reply to Alexandre LISSY :gerard-majax from comment #45)
> Hey, just found
> https://github.com/sotaroikeda/platform_frameworks_av/commit/
> ec46d95f9d369ec883768f8617224b640cf073cb which seems very very similar ...

This is a safe way to add 2 more unused gralloc buffers for video decoding. It does not request more gralloc buffer allocation. In b2g, ANativeWindow under OMXCodec is used differently than android. In android, ANativeWindow is buffer source and rendering target. But in b2g, ANativeWindow under OMXCodec use used just as buffer source and 2 buffers are kept unused within ANativeWindow. By the change, the 2 unused buffers are used for video decoding and prevent buffer starvation at OMXCodec. It is added by Bug 864230. See Bug 864230 Comment 80. 

But comment #8 is an unsafe way. It request to allocate 3 more gralloc buffers. It consume more pmem and current production b2g phones become reboot, if the phones try comment #8.
Flags: needinfo?(sotaro.ikeda.g)
Assignee: cajbir.bugzilla → nobody
Firefox OS is not being worked on
Status: ASSIGNED → RESOLVED
Closed: Last year
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.