+++ This bug was initially created as a clone of Bug #843599 +++ Since June/05, on master Firefox OS(gonk), when video app try to load hw codec, display becomes black and all b2g related process becomes dead. This problem happens by regression of Bug 843599. The problem happens at gralloc buffer allocation in b2g process. b2g process aborted in layers::GrallocBufferActor::Create().

By temporarily changing "MOZ_NOT_REACHED("Unknown gralloc pixel format");" part in BytesPerPixelForPixelFormat(), video playback problem was fixed on buri device.

Add handling default pixel format. A size of the format is 0. It is used for gralloc buffers for hw codec. The codec normally uses vendor proprietary color format. B2g does not use this size for processing, but uses it just for tracking the allocation.

By applying the attachment 759461 [details] [diff] [review], the crash in b2g is fixed. But video update during video playback stopped soon after started the playback. It seems other defect. I am not sure it is related to Bug 843599.

Comment on attachment 759461 [details] [diff] [review] patch - handle default pixel format in BytesPerPixelForPixelFormat() Review of attachment 759461 [details] [diff] [review]: ----------------------------------------------------------------- Can you add a comment in the default: block explaining why we're doing that instead of aborting? Just something like "Likely a platform-specific hw decoder format"

(In reply to Vladimir Vukicevic [:vlad] [:vladv] from comment #5) > Comment on attachment 759461 [details] [diff] [review] > > Can you add a comment in the default: block explaining why we're doing that > instead of aborting? Just something like "Likely a platform-specific hw > decoder format" Yes, will add to a next patch.

I think a proper fix should handle the YUV format explicitly, and update the calculation part as well.

(In reply to Chiajung Hung [:chiajung] from comment #7) > Created attachment 759674 [details] [diff] [review] > Patch > > I think a proper fix should handle the YUV format explicitly, and update the > calculation part as well. If you want to handle YUV format, can you create another bug for it? This bug needs to be fixed as soon as possible.

(In reply to Chiajung Hung [:chiajung] from comment #7) > Created attachment 759674 [details] [diff] [review] > Patch Some comments to the patch. >diff --git a/gfx/layers/ipc/ShadowLayerUtilsGralloc.cpp b/gfx/layers/ipc/ShadowLayerUtilsGralloc.cpp >--- a/gfx/layers/ipc/ShadowLayerUtilsGralloc.cpp >+++ b/gfx/layers/ipc/ShadowLayerUtilsGralloc.cpp >+ case 0x11: // NV21 >+ case 0x32315659: // YV12 >+ return -1; Write concrete value need to be prevented. Pixcel format is android's pixcel format definition. OEM pixcel format range if from 0x100 to 0x1FF, within this range any color format could be here. Almost all cases are YUV related proprietary formats. http://androidxref.com/4.0.4/xref/frameworks/base/libs/gui/SurfaceTexture.cpp#849

(In reply to Sotaro Ikeda [:sotaro] from comment #10) > (In reply to Chiajung Hung [:chiajung] from comment #7) > >diff --git a/gfx/layers/ipc/ShadowLayerUtilsGralloc.cpp b/gfx/layers/ipc/ShadowLayerUtilsGralloc.cpp > >--- a/gfx/layers/ipc/ShadowLayerUtilsGralloc.cpp > >+++ b/gfx/layers/ipc/ShadowLayerUtilsGralloc.cpp > > >+ case 0x11: // NV21 > >+ case 0x32315659: // YV12 > >+ return -1; > > Write concrete value need to be prevented. > Pixcel format is android's pixcel format definition. > > OEM pixcel format range if from 0x100 to 0x1FF, within this range any color > format could be here. Almost all cases are YUV related proprietary formats. I understand these values are come from system/core/include/system/graphics.h

(In reply to Sotaro Ikeda [:sotaro] from comment #12) > > > > OEM pixcel format range if from 0x100 to 0x1FF, within this range any color > > format could be here. Almost all cases are YUV related proprietary formats. > > I understand these values are come from system/core/include/system/graphics.h I thought only external format, 0x32315659(HAL_PIXEL_FORMAT_YV12) is not external format, so any value could be possible. Anyway, it is better to handle there formats in different bug.

Guys, please get something checked in today -- no reason to let things be broken. We should handle whatever we can reasonably handle if the issue is the bytes-per-pixel function; at a minimum, we should return 0 for formats we don't immediately understand like the original patch does. After that, we can look into a followup patch to extend the set formats the we do understand.

(In reply to Sotaro Ikeda [:sotaro] from comment #4) > By applying the attachment 759461 [details] [diff] [review], Sorry, I forgot to add comment to the patch. I created a patch that added a comment. But forgot to replace to the new one :-(

No problem; it sounds like there might be a followup anyway. Thanks!