This is the beginnings of a patch to add audio/video decoding via libstagefright/OMX to Gecko. This likely will only ever work with Gonk where we have solid control of the libstagefright source and OEM-portions of OMX, due to Android's non-existing clean APIs for this. How does libstagefright work? libstagefright implements a bunch of data sources, including a HTTP streaming client (DataSource). DataSource can be passed to a MediaExtractor, which sniffs for the media type and splits it into one or two MediaSource streams (depending on whether we have audio or video or both). MediaSource is piped into the actual decoders, which are controlled through OMXCodec, which contains a lot of horrible quirks to deal with OEMs being unable to write codecs in any sane way. The decoders spit out MediaBuffers, which are basically a video frame or a chunk of audio data. MetaData rides along and can be used to figure out the format. There are two ways to render MediaBuffers. For some MediaBuffers the data is available in them and can be read out in one of a few YUV formats. Others (in particular hardware decoders) don't cough up the data. Instead, the MediaBuffer contains a bufferId which can be passed to a OEM-specific renderer which knows how to put it onto an overlay. The latter allows secure decoders where the image data is not actually accessible at all. How does this work with Gecko? We are not using libstagefright's clownshoes of a network/buffering layer. Instead, we wrap nsMediaStream into a DataSource and hand that to the decoder. OMXGlue.cpp contains code to invoke the decoder, and it also contains declarations the decoder-side uses to access the wrapped nsMediaStream. When decoding audio and video we have 2 frames in flight at all times so we can determine the current time as well as the end time of a frame (which is the current time of the next frame). The YUV data of libstagefright can be planar (which we already support), as well as semi-planar (y and uv). I added extra fields to VideoData to express that. We should rename YUV_PLANAR to just YUV and add support to ImageLayer and change the shaders accordingly. I will do that in the next version of the patch.

I forgot to mention what we are going to do with MediaBuffers that don't allow the YUV content to be extracted. I will wire up the binary blob that does the rendering to render that content directly to the screen using an overlay, or if I can trick it to render into video memory, we will have it render into a texture. We will wrap this magical kind of image into a new kind of ImageLayer, similar as we do for certain textures on Mac.

I think I may have this almost working on Android. Its quite a hack and it will require special handling for each major version of android (2.2 2.3 4.0 etc), but I think it can be done OEM-neutral, which would make this feasible for product deployment. I will poke at this some more and post a more detailed summary.

Factor code that talks to libstagefright into libomxplugin.so and load it dynamically through a clean interface. Build dummy libutils/libbinder/libstagefright/libmediaplayerservice and link the plugin against that. At load time the real libs will be picked up. If that fails, firefox still loads (hence the libomxplugin). MPAPI.h defines how libomxplugin.so talks to the media stack. I will probably factor overlay support into the same API. This is still not working right, this is just a code dump.

(Note: the android source has to be present to build libomxplugin since we use header files that are not in the NDK)

Making good progress here, new update soon.

I created attachment 592577 [details] [diff] [review] patch. it adds libstagefright/OMX-based decoding by using sw decoder and hw decoder. but still has problems, like folowing. - drawed color of SemiPlanarYUV420Frame is not correct. + PlanarYUV420Frame seems correct + this might be a problem of ImageLayer - played audio is not correct (weird sound) - occasionally, b2g process crashes

592577 tentatively disables security chekeck of nsHTMLMediaElement.cpp for operation cheking, I added video tag to a b2g' application. but It seems that httpd.js could not handle read request to Movie content. I do not know how to solve this problem. Then, simply use following file uri. To do this, the security check is temporarily disabled. for program operation cheking, I use following code. > <video controls autoplay> > <source src="file:///system/home/apps/browser/sample.m4v" type="video/mp4"/> > </video>

topic of the patch - libstagefright/OMX-based decoding within b2g' process - MediaPlayerService disabled It seems that the patch still has bugs to MediaBuffer handling. If audio data handling is enabled, b2g process crashes, then it tentatively disables audio.

Comment on attachment 596301 [details] [diff] [review] Part 1: gonk patch to fix some quirks and disable old drivers for akami >diff --git a/media/libstagefright/OMXCodec.cpp b/media/libstagefright/OMXCodec.cpp >index b5d00bf..a0c81cc 100644 >--- a/media/libstagefright/OMXCodec.cpp >+++ b/media/libstagefright/OMXCodec.cpp >@@ -166,18 +166,18 @@ static const CodecInfo kDecoderInfo[] = { > // { MEDIA_MIMETYPE_AUDIO_AAC, "OMX.PV.aacdec" }, > { MEDIA_MIMETYPE_AUDIO_G711_ALAW, "G711Decoder" }, > { MEDIA_MIMETYPE_AUDIO_G711_MLAW, "G711Decoder" }, >- { MEDIA_MIMETYPE_VIDEO_MPEG4, "OMX.qcom.7x30.video.decoder.mpeg4" }, >+// { MEDIA_MIMETYPE_VIDEO_MPEG4, "OMX.qcom.7x30.video.decoder.mpeg4" }, > { MEDIA_MIMETYPE_VIDEO_MPEG4, "OMX.qcom.video.decoder.mpeg4" }, > { MEDIA_MIMETYPE_VIDEO_MPEG4, "OMX.TI.Video.Decoder" }, > { MEDIA_MIMETYPE_VIDEO_MPEG4, "OMX.SEC.MPEG4.Decoder" }, > { MEDIA_MIMETYPE_VIDEO_MPEG4, "M4vH263Decoder" }, > // { MEDIA_MIMETYPE_VIDEO_MPEG4, "OMX.PV.mpeg4dec" }, >- { MEDIA_MIMETYPE_VIDEO_H263, "OMX.qcom.7x30.video.decoder.h263" }, >+// { MEDIA_MIMETYPE_VIDEO_H263, "OMX.qcom.7x30.video.decoder.h263" }, > { MEDIA_MIMETYPE_VIDEO_H263, "OMX.qcom.video.decoder.h263" }, > { MEDIA_MIMETYPE_VIDEO_H263, "OMX.SEC.H263.Decoder" }, > { MEDIA_MIMETYPE_VIDEO_H263, "M4vH263Decoder" }, > // { MEDIA_MIMETYPE_VIDEO_H263, "OMX.PV.h263dec" }, >- { MEDIA_MIMETYPE_VIDEO_AVC, "OMX.qcom.7x30.video.decoder.avc" }, >+// { MEDIA_MIMETYPE_VIDEO_AVC, "OMX.qcom.7x30.video.decoder.avc" }, > { MEDIA_MIMETYPE_VIDEO_AVC, "OMX.qcom.video.decoder.avc" }, > { MEDIA_MIMETYPE_VIDEO_AVC, "OMX.TI.Video.Decoder" }, > { MEDIA_MIMETYPE_VIDEO_AVC, "OMX.SEC.AVC.Decoder" }, >@@ -381,6 +381,7 @@ uint32_t OMXCodec::getComponentQuirks( > if (!strncmp(componentName, "OMX.qcom.7x30.video.encoder.", 28)) { > } > if (!strncmp(componentName, "OMX.qcom.video.decoder.", 23)) { >+ quirks |= kRequiresAllocateBufferOnInputPorts; > quirks |= kRequiresAllocateBufferOnOutputPorts; > quirks |= kDefersOutputBufferAllocation; > } >@@ -3601,6 +3602,8 @@ void OMXCodec::initOutputFormat(const sp<MetaData> &inputFormat) { > } else { > mOutputFormat->setInt32(kKeyWidth, video_def->nFrameWidth); > mOutputFormat->setInt32(kKeyHeight, video_def->nFrameHeight); >+ mOutputFormat->setInt32(kKeyStride, video_def->nStride); >+ mOutputFormat->setInt32(kKeySliceHeight, video_def->nSliceHeight); > } > > mOutputFormat->setInt32(kKeyColorFormat, video_def->eColorFormat); >diff --git a/media/mediaserver/main_mediaserver.cpp b/media/mediaserver/main_mediaserver.cpp >index 7094cfa..1c9e020 100644 >--- a/media/mediaserver/main_mediaserver.cpp >+++ b/media/mediaserver/main_mediaserver.cpp >@@ -39,7 +39,7 @@ int main(int argc, char** argv) > sp<IServiceManager> sm = defaultServiceManager(); > LOGI("ServiceManager: %p", sm.get()); > AudioFlinger::instantiate(); >- MediaPlayerService::instantiate(); >+ /* MediaPlayerService::instantiate(); */ > CameraService::instantiate(); > AudioPolicyService::instantiate(); > ProcessState::self()->startThreadPool();

The patch now avoids having more than one MediaBuffer outstanding, reducing the amount of patching necessary to libstagefright. The attached patch plays the Big Buck Bunny H264 360p video with full framerate (30FPS) on akami. Major issues: - We are seeing horrific tearing on akami. The background shines through for every second frame or so. Looks like we aren't getting double buffering on the GL context on akami, and we pretty reliably get hit by vsync at the worst possible time. This is probably not the fault of this patch. - On akami, B2G crashes after a few seconds or minutes. Its pretty easy to reproduce so shouldn't be too hard to debug. An anonymous silicon vendor engineer contributed to this patch (in addition to Ikeda.)

Patches I made were alrealy oblsolete. then, this is just informational. The patches had defects related to MediaBuffers hadling. When Audio is enabled, B2G process crashes soon. An attached patch fixes the defects. By this fixes, "STEREO Audio" could be palyed. But still has defect to "MONO audio handling".

Ikeda, I forgot to attach my omx patch. I will do that in a sec. I removed using 2 buffers in Read() so now its not necessary any more to modify stagefright. Can you attach an interdiff so I see what you changed for the audio fix?

(In reply to Andreas Gal :gal from comment #21) > Created attachment 596932 [details] [diff] [review] > part 2: gecko patch Andreas, current "part 2: gecko patch" do not include follwoing files. - /B2G/gecko/content/media/plugins/* Can you update the patch?

Strange. Sure, let me check again.

Ok, let me know if this is complete. This still - stops playback or crashes occasionally playing a long video - has audio/video sync issues on seek (mostly video just stops)

Thank you for updating the patch. New patch is complete. There seems following problems, in emulator environment. 　- crashes occasionally during playback - no video drawing I'm going to see the problems tomorrow.

Good idea to run this in the emulator. I didn't actually try that. The emulator uses software decoding which probably emits a different image format which we might not support (or not support correctly).

interdiff patch to fix following problems on emulator - no video drawing - seek

597288 also fixes seeking issues on Akami as well. Thanks! I didn't pull in the OmxDecoder::ReadVideo() retry stuff into my build though (worked fine with out.) What is this needed for BTW, and why 20?

> I didn't pull in the OmxDecoder::ReadVideo() retry stuff into my build > though (worked fine with out.) What is this needed for BTW, and why 20? There are two reasons for "retry". - sw codec "i.e. AVCDecoder" frequently output consecutive zero sized Video frames. - handle format change (rarely happens) On emulator, sw codecs are always used. In AVCDecoder's case, OmxDecoder often receives zero sized Video frames. But there are no big reason for "20". Yesterday, I watched about 10 consecutive zero sized Video frames from log output. Then I set tentatively maximum retry number to 20. After ics, there is no AVCDecoder class. The class is replaced by SimpleSoftOMXComponent and SoftAVC. The situation might be changed. But I did not checked about it.

> On emulator, sw codecs are always used. In AVCDecoder's case, OmxDecoder > often receives zero sized Video frames. But there are no big reason for > "20". Yesterday, I watched about 10 consecutive zero sized Video frames from > log output. Then I set tentatively maximum retry number to 20. It seems that "AVCDecoder::read()" returns for each NAL. If a NAL does not include video frame(i.e. SPS, PPS, SEI, AUD, FILL, EOSEQ), AVCDecoder returns zero sized MediaBuffer.

Thanks for the info. Do you know how Android normally deals with this? Do they have a similar loop w/ "20" retry attempts?

> Thanks for the info. Do you know how Android normally deals with this? Do > they have a similar loop w/ "20" retry attempts? Android's implementation is use "for (;;)" loop. It is implemented within "AwesomePlayer::onVideoEvent()" code is like follwoing. This way is simpler than "loop w/MaxCount". > for (;;) { >> //omit code > if (mVideoBuffer->range_length() == 0) { > // Some decoders, notably the PV AVC software decoder > // return spurious empty buffers that we just want to ignore. > > mVideoBuffer->release(); > mVideoBuffer = NULL; > continue; > } > break; > }

Cool, lets do that then. The for(;;) is definitely better than a random number.

Latest and greatest patch. This is untested but fixes a couple conceptual issues. Amongst others we buffer VideoData before submitting it so we can calculate the proper end time, and we have to use end time when deciding whether we are within the threshold. We should fix httpd.js to send the right mime type instead of hacking away CORS and the content security checks (TODO tomorrow).

I also removed Pin/Unpin for MPAPI. We can hide that in Gecko. Fewer things to get wrong in the plugin.

This patch fixes up a couple decoders in libstagefright that are broken in upstream Android. Commercial android builds probably already fix this.

This patch fixes a bunch of stuff: - fix mime type handling - make sure httpd.js sends the right mime type - fix a 14 year old bug in nsBinaryOutputStream (I am not even kidding) which killed httpd on short writes (pipe full) - add a way to stop httpd.js from starting up, because it simply can't handle large files (it sucks large files into memory, twice) - add support for octet files to omx-plugin when playing files from disk - add proper slow path for all YUV video formats With this patch video works great. There is no fast path for YUV format adjustment yet, so we suck 35% CPU time for a 320p H.264 video on akami, but even that is pretty respectable.

Comment on attachment 598641 [details] [diff] [review] Part 2: gecko patch Bugzilla broken. I can't request review. Awesome. Will do via email.

I landed part 1 on github. Only the gecko part remains now.

This patch fixes some bad interaction between the media cache and the DataSource abstraction. This works now via http pretty well with a local web server. Haven't tested over the network with any significant latency.

Cleaned-up patch. Works great on akami/maguro. Samsung Galaxy S2 doesn't permit reading back frames. I will work on using the overlay hardware in a follow-up patch.

Comment on attachment 600724 [details] [diff] [review] patch Review of attachment 600724 [details] [diff] [review]: ----------------------------------------------------------------- First look based on trying to build with patch applied, and running tests. More to come. A build with this patch applied, and plugins disabled, crashes when loading Ogg videos. It's missing the setting of mOffset and mSkip in the planar data. Something like: diff --git a/content/media/ogg/nsOggReader.cpp b/content/media/ogg/nsOggReader.cpp index 579c7a7..543da41 100644 --- a/content/media/ogg/nsOggReader.cpp +++ b/content/media/ogg/nsOggReader.cpp @@ -463,6 +463,7 @@ nsresult nsOggReader::DecodeTheora(ogg_packet* aPacket, PRInt64 aTimeThreshold) b.mPlanes[i].mHeight = buffer[i].height; b.mPlanes[i].mWidth = buffer[i].width; b.mPlanes[i].mStride = buffer[i].stride; + b.mPlanes[i].mOffset = b.mPlanes[i].mSkip = 0; } VideoData *v = VideoData::Create(mInfo, ::: configure.in @@ +5028,5 @@ > + > +if test -n "$MOZ_OMX_PLUGIN"; then > + AC_DEFINE(MOZ_OMX_PLUGIN) > +fi > + This needs to be moved to after the MOZ_ARG_DISABLE_BOOL for 'media-plugins' and 'omx-plugin', otherwise compiling with "--disable-media-plugins" does not disable the media plugins and build fails. I think media plugins and omx-plugin should be disabled by default, and enabled explicitly. Or maybe disabled on all but Android/B2G? ::: content/media/plugins/nsMediaPluginHost.cpp @@ +46,5 @@ > +#include "pratom.h" > +#include "nsMediaPluginReader.h" > + > +#include "android/log.h" > +#define LOG(args...) __android_log_print(ANDROID_LOG_INFO, "MediaPluginHost" , ## args) Is nsMediaPluginHost and friends supposed to compile on non-android platforms? If so the above needs to be #ifdef'd out in some manner as "enable-media-plugins" fails to build on non-android platforms. Same comment applies to nsMediaPluginReader.cpp.

I would like to build MPAPI everywhere but we load the plugin on android only, and we disable MPAPI via pref by default.

Use new CheckedInt64 return value.

don't configure video if we have audio only and vice versa

don't try to configure audio/video if there is no track for it

Fix for audio-only stream likely overlaps with just-posted patch.

This patch feels wrong but fixes another crasher.

Ok thats wrong we shouldn't get there.

Comment on attachment 604312 [details] [diff] [review] patch Clearly not in line with Mozilla policy.

There's a leak in the existing patch in attachment 604312 [details] [diff] [review]. The attached patch fixes it.

This is a DUP of bug 541286

If the reason is H.264, please enable only H.264 and MPEG2 (see bug 541286), but not all codecs that the platform might support. This and all the negative effects of enabling all codecs, esp. fragmentation and security problems, have been discussed extensively in bug 435339.

(In reply to Ben Bucksch (:BenB) from comment #58) > If the reason is H.264, please enable only H.264 and MPEG2 (see bug 541286), > but not all codecs that the platform might support. H.264 is the only encumbered video codec we need to support to be able to consume the same content Safari, IE9, Chrome and the Android stock browser consume. We should whitelist only H.264 of video codecs. Especially we shouldn't enabled the Web to become dependent on the MPEG-2 patent mess.

How about making this a pref? I.e. user can enable any format per pref, but only H.264 is enabled by default? (MPEG2 is just the earlier version of the same ISO standard. There's tons of content in MPEG2 format out there - e.g. from TV / DVB.)

(In reply to Ben Bucksch (:BenB) from comment #60) > How about making this a pref? I.e. user can enable any format per pref, but > only H.264 is enabled by default? > > (MPEG2 is just the earlier version of the same ISO standard. There's tons of > content in MPEG2 format out there - e.g. from TV / DVB.) There isn't a ton of MPEG-2 content out there *on the Web* *in HTML5 video*. MPEG-2 isn't "just" an earlier version. It's a whole different patent situation. Different typical audio tracks. Different typical containers. This bug is about Gonk and MPEG-2 TV/DVB stream over IP don't make sense at current mobile network speeds anyway. Let's continue this in dev-media if continuing this is necessary.

http://www.mythtv.org/wiki/MythWeb And, yes, that makes a lot of sense on tablets at home. My main point was: the end user can enable any format per pref, but only H.264 is enabled by default.

doublec, do you own this now? I don't think Andreas is working on this anymore.

doublec is on vacation, back in a couple of weeks.

while I tried to play H.264 video using hw codec on ics's B2G, I made a temporal patch. the patch adds - support video size info(kKeyCropRect meta data) on ICS - support OMX_TI_COLOR_FormatYUV420PackedSemiPlanar color format After ICS, actual video size info is provided by kKeyCropRect metadata. Without the metadata, we can not get actual Video frame sizes in some cases. when we did not add this, we faced segmentation fault under "videoImage->CopyData(data,..)" function. I referenced following ICS's source code to add "OMX_TI_COLOR_FormatYUV420PackedSemiPlanar" support, - \frameworks\base\media\libstagefright\colorconversion\ColorConverter.cpp

Comment on attachment 604312 [details] [diff] [review] patch The newly added files need to have the boilerplate changed to use the new MPL 2 headers. r+ with that and the change to fix the memory leak from attachment 605287 [details] [diff] [review].

This can land.

I would prefer bug 735769 be investigated first, but we've been living with that regression for quite a while in b2g land so not a hard blocker.

(In reply to Chris Jones [:cjones] [:warhammer] from comment #68) > I would prefer bug 735769 be investigated first, but we've been living with > that regression for quite a while in b2g land so not a hard blocker. The work in this bug is really important; it's one of our Q2 goals (delivering Platform Decoder/MPAPI support) and will be strongly desired for K9o. Do we need to resolve Bug 735769 before we ship Platform Decoders (MPAPI support) in B2G and Android? The answer may well be yes, but I looked at the comments in 735769, and they were pretty sparse with not much context. Can you expand on what problems Bug 735769 causes? (Either here or in Bug 735769.) Thanks.

#0 0x40950e14 in nsMediaPluginReader::GetBuffered (this=0x46ec1200, aBuffered=<value optimized out>, aStartTime=<value optimized out>) at /local/mnt/workspace/dwilson/ics.b2g/gecko/content/media/plugins/nsMediaPluginReader.cpp:327 #1 0x409434fa in nsBuiltinDecoderStateMachine::GetBuffered ( this=<value optimized out>, aBuffered=0xbedc24c8) at /local/mnt/workspace/dwilson/ics.b2g/gecko/content/media/nsBuiltinDecoderStateMachine.cpp:2165 #2 0x40941f0c in nsBuiltinDecoder::GetBuffered (this=<value optimized out>, aBuffered=0xbedc2450) at /local/mnt/workspace/dwilson/ics.b2g/gecko/content/media/nsBuiltinDecoder.h:485 #3 0x40944214 in nsBuiltinDecoderStateMachine::NotifyDataArrived ( this=0x45eb5000, aBuffer=<value optimized out>, aLength=<value optimized out>, aOffset=0) at /local/mnt/workspace/dwilson/ics.b2g/gecko/content/media/nsBuiltinDecoderStateMachine.cpp:1202 #4 0x40941f76 in nsBuiltinDecoder::NotifyDataArrived ( this=<value optimized out>, aBuffer=0xbedc2450 "", aLength=0, aOffset=0) at /local/mnt/workspace/dwilson/ics.b2g/gecko/content/media/nsBuiltinDecoder.h:506 #5 0x40947cce in mozilla::ChannelMediaResource::CopySegmentToCache ( aInStream=<value optimized out>, aClosure=0xbedc25e4, aFromSegment=0x46fc9000 "", aToOffset=<value optimized out>, aCount=512, aWriteCount=0xbedc25a4) at /local/mnt/workspace/dwilson/ics.b2g/gecko/content/media/MediaResource.cpp:375 #6 0x40c6baae in nsPipeInputStream::ReadSegments (this=0x476e8708, writer=0x40947ca5 <mozilla::ChannelMediaResource::CopySegmentToCache(nsIInputStream*, void*, char const*, unsigned int, unsigned int, unsigned int*)>, closure=0xbedc25e4, count=512, readCount=0xbedc25ec) at /local/mnt/workspace/dwilson/ics.b2g/gecko/xpcom/io/nsPipe3.cpp:799 #7 0x40947312 in mozilla::ChannelMediaResource::OnDataAvailable ( this=<value optimized out>, aRequest=<value optimized out>, aStream=0x476e8708, aCount=512) at /local/mnt/workspace/dwilson/ics.b2g/gecko/content/media/MediaResource.cpp:408 #8 0x4094734c in mozilla::ChannelMediaResource::Listener::OnDataAvailable ( this=<value optimized out>, aRequest=0xbedc2450, aContext=<value optimized out>, aStream=<value optimized out>, aOffset=0, aCount=512) at /local/mnt/workspace/dwilson/ics.b2g/gecko/content/media/MediaResource.cpp:127 #9 0x407e1020 in nsHTMLMediaElement::MediaLoadListener::OnDataAvailable ( this=<value optimized out>, aRequest=0xbedc2450, aContext=0x0, aStream=0x0, aOffset=0, aCount=512) at /local/mnt/workspace/dwilson/ics.b2g/gecko/content/html/content/src/nsHTMLMediaElement.cpp:395 #10 0x4051cffa in nsUnknownDecoder::FireListenerNotifications ( this=0x432f0ca0, request=0x47b8fc30, aCtxt=0x0) at /local/mnt/workspace/dwilson/ics.b2g/gecko/netwerk/streamconv/converters/nsUnknownDecoder.cpp:653 #11 0x4051d0a8 in nsUnknownDecoder::OnDataAvailable (this=0x432f0ca0, request=0x47b8fc30, aCtxt=0x0, aStream=0x476e99c8, aSourceOffset=512, aCount=392704) at /local/mnt/workspace/dwilson/ics.b2g/gecko/netwerk/streamconv/converters/nsUnknownDecoder.cpp:185 #12 0x40554d8e in nsHttpChannel::OnDataAvailable (this=0x47b8fc00, request=<value optimized out>, ctxt=<value optimized out>, input=0x476e99c8, offset=0, count=393216) at /local/mnt/workspace/dwilson/ics.b2g/gecko/netwerk/protocol/http/nsHttpChannel.cpp:4604 #13 0x404feb36 in nsInputStreamPump::OnStateTransfer (this=0x47af6f60) at /local/mnt/workspace/dwilson/ics.b2g/gecko/netwerk/base/src/nsInputStreamPump.cpp:518 #14 0x404fec46 in nsInputStreamPump::OnInputStreamReady (this=0x47af6f60, stream=<value optimized out>) at /local/mnt/workspace/dwilson/ics.b2g/gecko/netwerk/base/src/nsInputStreamPump.cpp:402 #15 0x40c6c064 in nsInputStreamReadyEvent::Run (this=0x47a24260) at /local/mnt/workspace/dwilson/ics.b2g/gecko/xpcom/io/nsStreamUtils.cpp:193 #16 0x40c76ae4 in nsThread::ProcessNextEvent (this=0x41b12460, mayWait=<value optimized out>, result=0xbedc27a7) at /local/mnt/workspace/dwilson/ics.b2g/gecko/xpcom/threads/nsThread.cpp:656 #17 0x40c550d8 in NS_ProcessNextEvent_P (thread=0x0, mayWait=true) at /local/mnt/workspace/dwilson/ics.b2g/out/target/product/msm7627a/obj/objdir-gecko/xpcom/build/nsThreadUtils.cpp:245 #18 0x40c76c72 in nsThread::Shutdown (this=0x47ddf700) at /local/mnt/workspace/dwilson/ics.b2g/gecko/xpcom/threads/nsThread.cpp:498 #19 0x405053be in nsRunnableMethodImpl<void (nsPACMan::*)(), true>::Run ( this=<value optimized out>) at ../../../dist/include/nsThreadUtils.h:345 #20 0x40c76ae4 in nsThread::ProcessNextEvent (this=0x41b12460, mayWait=<value optimized out>, result=0xbedc282f) at /local/mnt/workspace/dwilson/ics.b2g/gecko/xpcom/threads/nsThread.cpp:656 #21 0x40c550d8 in NS_ProcessNextEvent_P (thread=0x0, mayWait=false) at /local/mnt/workspace/dwilson/ics.b2g/out/target/product/msm7627a/obj/objdir-gecko/xpcom/build/nsThreadUtils.cpp:245 #22 0x40be8828 in mozilla::ipc::MessagePump::Run (this=0x41b0f1f0, aDelegate=0x41b1a0e0) at /local/mnt/workspace/dwilson/ics.b2g/gecko/ipc/glue/MessagePump.cpp:110 #23 0x40c9a36a in MessageLoop::RunInternal (this=0xbedc2450) at /local/mnt/workspace/dwilson/ics.b2g/gecko/ipc/chromium/src/base/message_loop.cc:208 #24 0x40c9a44a in MessageLoop::RunHandler (this=0x41b1a0e0) at /local/mnt/workspace/dwilson/ics.b2g/gecko/ipc/chromium/src/base/message_loop.cc:201 #25 MessageLoop::Run (this=0x41b1a0e0) at /local/mnt/workspace/dwilson/ics.b2g/gecko/ipc/chromium/src/base/message_loop.cc:175 #26 0x40b740d4 in nsBaseAppShell::Run (this=0x42392340) at /local/mnt/workspace/dwilson/ics.b2g/gecko/widget/xpwidgets/nsBaseAppShell.cpp:189 #27 0x40a9fc0e in nsAppStartup::Run (this=0x4235efd0) at /local/mnt/workspace/dwilson/ics.b2g/gecko/toolkit/components/startup/nsAppStartup.cpp:295 #28 0x404e62da in XREMain::XRE_mainRun (this=0xbedc2a74) at /local/mnt/workspace/dwilson/ics.b2g/gecko/toolkit/xre/nsAppRunner.cpp:3768 #29 0x404e8d84 in XREMain::XRE_main (this=0xbedc2a74, argc=<value optimized out>, argv=0xbedc4c64, aAppData=<value optimized out>) at /local/mnt/workspace/dwilson/ics.b2g/gecko/toolkit/xre/nsAppRunner.cpp:3845 #30 0x404e8efe in XRE_main (argc=1, argv=0xbedc4c64, aAppData=0xab80) at /local/mnt/workspace/dwilson/ics.b2g/gecko/toolkit/xre/nsAppRunner.cpp:3921 #31 0x0000897e in do_main (argc=1, argv=0xbedc4c64) at /local/mnt/workspace/dwilson/ics.b2g/gecko/b2g/app/nsBrowserApp.cpp:186 #32 main (argc=1, argv=0xbedc4c64) at /local/mnt/workspace/dwilson/ics.b2g/gecko/b2g/app/nsBrowserApp.cpp:269

Current "OmxPlugin.cpp" implementaion is not good enough from performance and memory usage. Even when using hw decoder, cpu reads video frames from video buffers and copy the buffers to memories, then the video data are uploaded to OpenGL texture when rendering. This spend cpu time and large amount of memories. and need to care about platform dependent proprietary color formats! I played some H264 videos on my phone (omap dualcore, ics). - H.264 600*480 30fps: spent 50% cpu time. - H.264 720P 24fps: soon after start playback, video rendering stopped. IMHO, b2g need to solve this problem. Then I wrote a tentative patch. the patch has a lot of ploblems, currently. But It could avoud video buffer copy, and save memories. The patch is so clumsy. But this way of video rendering is necessay. Thought about this? * characteristics of the patch - use ANativeWindow for GraphicBuffer allocation the video output is written to GraphicBuffer. - do not copy video frames by using cpu, but deliver the buffers to ImageLayerOGL. - the buffer(GraphicBuffer) are directly bound to OpenGL texture() and rendered - no need to care about proprietary color formats *limitations/problems (write just some...) + [1] disable jemalloc tentatively GraphicBuffer make allocator/deallocator mismatch. refer bug 740997 + [2] implementation is so temporal. + [3] do not destruct objects correctly. + [4] build only on ics platform(could not build on gingerbread) + [5] do not support to overlay need to add support of HWComposer to LayerManagerOGL and ImageLayerOGL. + [6] could play only a few sizes of videos(i.e. 720p on my ics phone) do not handle that omap hw codec output do not map all decoded video output buffer areas. the codec memory map all actual video area (crop region). but outside of the crop region is not always memory mapped. attachment 617798 [details] [diff] [review] avoid the problem by only read actual video area (crop region). but in this patch , I could not find a way to solve this. on my phone, I could played 720p video. so, I checked the video playback by using 720p video.

by using attachment 623069 [details] [diff] [review] on my phone (omap dualcore, ics). H.264 720P 24fps : spent about 20% cpu time during playback.

There may be lots of issues with your patch, but the performance improvement looks to be significant - copying video buffers is always to be avoided when possible. What's the delta in performance at 640x480 30fps? These chips (especially DSP-based ones like OMAP) often have all sorts of handy hardware engines for memory copies, etc; doing them in the CPU simply sucks. (Hell, copying video data in Desktop sucks too if can possibly be avoided.) It sounds like (for B2G) the jemalloc issue can be resolved per the bug you reference.

>What's the delta in performance at 640x480 30fps? because of the patch's problem, 640x480 30fps could not be played...Video out buffers starved soon after start playback for some reason. then OmxCodec::read() failed because of timeout. I could check the delta on smaller video size. video size:with copy->no copy -320*240 :cpu 29% -> cpu 17% -720p :video update stopped -> cpu 20%

just fix a small problem of the attachment 623069 [details] [diff] [review]. I had selected a wrong "gecko/toolkit/library/Makefile.in" file.

by modifying the code, 600*480 could be played with the patch. video size :with copy ->no copy -H.264 320*240 25fps :cpu 29% -> cpu 17% -H.264 600*480 30fps :cpu 50% -> cpu 18% -H.264 720P 24fps :video update stopped -> cpu 20%

temporary fix for 600*480 video play problem. comment out following code. the change is also tentative. nsMediaPluginReader::DecodeVideoFrame() // if (mLastVideoFrame->mEndTime < aTimeThreshold) { // delete mLastVideoFrame; // mLastVideoFrame = NULL; // continue; // }

update attachment 623590 [details] [diff] [review]. fix buffer starvation during playing 600*480 video. suppress actual number of video frames owned by the OmxCodec' client-side at most to two, otherwise OmxCodec starves video buffer during playing 600*480 video on my phone. At first I tried to increase number of video output buffer, but it's request is rejected by TI OMX component. i.e: stagefright's AwesomePlayer concurrently own at most only one video frame.

Are we going to land the existing patch for libstagefright support and look at the no-copy work in a separate bug, or should we wait for the no-copy work?

I would definitely do this incrementally. Land what we have, continue this awesome zero-copy work in a new bug. Also, the current patch needs an additional fix in GetBuffered() to return 0 if !mPlugin otherwise we crash on ICS. We seem to call GetBuffered from two threads. Trying to instantiate the plugin in GetBuffered() deadlocks. Maybe doublec can help figuring out why. We couldn't explain it at the work week.

We call GetBuffered() from inside the state machine thread and decode threads in order to figure out whether we have enough undecoded data remaining to play without stopping. It can also be called on the main thread by JS.

Can you audit the code to make sure that synchronization is sane? At the very least we access mPlugin from 2 threads without synchronizing properly. Hellgrind would probably barf at this.

Correction about following. > + [6] could play only a few sizes of videos(i.e. 720p on my ics phone) > omap hw codec output do not map all decoded video output buffer areas. > the codec memory map all actual video area (crop region). > but outside of the crop region is not always memory mapped. > attachment 617798 [details] [diff] [review] avoid the problem by only read > actual video area (crop region). > but in this patch , I could not find a way to solve this. I have to say it was my misunderstanding. There is no problem like it when do rendering by directly bound GraphicBuffer to texture. By using attachment 623069 [details] [diff] [review], segmentation faults happened depends on the video sizes, then I made wrong assumption... Attachment 623971 [details] [diff] do not have such restriction. There is currently no restriction like [6]. It seems that the faults happend because of wrong MediaBuffer handling. i.e: within nsMediaPluginReader::DecodeVideoFrame() > if (mLastVideoFrame->mEndTime < aTimeThreshold) { > delete mLastVideoFrame; > mLastVideoFrame = NULL; > continue; > } the above code is not a problem in "with video copy" case. but a problem in "no video copy" case, then changed the code like following. > if (mLastVideoFrame->mEndTime < aTimeThreshold) { > delete mLastVideoFrame; > mLastVideoFrame = NULL; > mLastVideoFrame = v; //update mLastVideoFrame > continue; > }

attachment 623971 [details] [diff] [review] could not build on b2g. My local ics-b2g code enviroment modified differently from normal b2g. That cause to making wrong patch. I am going to check the patch and update, soon.

update attachment 623971 [details] [diff] [review] add following to the patch - /glue/gonk-ics/frameworks/base/include/gui/ISurfaceTexture.h - /glue/gonk-ics/frameworks/base/include/ui/GraphicBuffer.h

Took out hunk which reduces media cache size due to bug 755533. Reducing media cache size split out into bug 755609.

update again... I moved following call to wrong position and move back. android::sp<android::GraphicBuffer> activeBuffer = surfaceTexture->getCurrentBuffer();

Comment on attachment 624281 [details] [diff] [review] Patch Review of attachment 624281 [details] [diff] [review]: ----------------------------------------------------------------- Let's land this. I'm rubber-stamping since doublec already reviewed.

Comment on attachment 605287 [details] [diff] [review] Fix memory leak Before landing needs attachment 605287 [details] [diff] [review] as well to fix memory leak in original patch.

Comment on attachment 605287 [details] [diff] [review] Fix memory leak Review of attachment 605287 [details] [diff] [review]: ----------------------------------------------------------------- Maybe we should just fold this patch into the former patch? ::: content/media/plugins/nsMediaPluginHost.cpp @@ +166,5 @@ > aDecoder->DestroyDecoder(aDecoder); > delete aDecoder; > } > > +nsMediaPluginHost *sMediaPluginHost = nsnull; I'd add a blank line here between these two lines for readability.

Splits configure/build changes from main patch. Changes 'disable-media-plugins' to 'enable-media-plugins' so they're disabled by default. This patch adds: * --enable-media-plugins to media the content/media/plugin code * --enable-omx-plugin to build the omx-plugin.so shared library * --with-android-source for setting the director for the android source code (used for non-b2g android builds)

Splits out the non-configure build changes from the original patch for separate review. * Rolls up previously individual patches. * Moves omx-plugin from toolkit/mozapps/omx-plugin to media/omx-plugin * Changes MPL license in new files to MPL2 Review carried forward.

Comment on attachment 625546 [details] [diff] [review] Part 1: Configure/Toolkit changes A few changes to this are needed - a new patch will be up shortly.

Fixes issues with previous patch. Previous comment duplicated below. Splits configure/build changes from main patch. Changes 'disable-media-plugins' to 'enable-media-plugins' so they're disabled by default. This patch adds: * --enable-media-plugins to media the content/media/plugin code * --enable-omx-plugin to build the omx-plugin.so shared library * --with-android-source for setting the director for the android source code (used for non-b2g android builds)

Tweaks OmxPlugin to build on andreasgal/B2G gingerbread builds (ie. akami) as well as mozilla-b2g/B2G ICS builds (eg. Nexus S).

Fixes bitrot from the 'seeking in buffered ranges' changes that landed recently.

update attachment 624282 [details] [diff] [review] update following two. - comment out the following within SurfaceTextureGonk::updateTexImage() gl->fTexParameteriv(LOCAL_GL_TEXTURE_EXTERNAL_OES, LOCAL_GL_TEXTURE_CROP_RECT_OES, crop); GL_TEXTURE_CROP_RECT_OES is not necessary. And it seems that GL_TEXTURE_CROP_RECT_OES do not work for GL_TEXTURE_EXTERNAL_OES. - add some code within ImageLayerOGL::RenderLayer() to care the case that a texture crop of SurfaceTextureGonk is [0,0,0,0] . when a texture crop of SurfaceTextureGonk is [0,0,0,0], all region of GraphicBuffer/texture is valid. I recognized it from bug 757341 You could understand this from SurfaceTexture::computeCurrentTransformMatrix(). http://androidxref.com/source/xref/frameworks/base/libs/gui/SurfaceTexture.cpp#804

Chris, I think I am almost ready to begin testing this patch. If you could highlight some areas where you feel we need coverage, and across which operating systems. That would be great. https://developer.mozilla.org/en/DOM/HTMLMediaElement Assuming that we'd want to test against the Spec, and make sure all of the properties work as expected. Any clues as to how to pick up on any memory leaks would be much appreciated. I am starting to put together a test page / content which will allow me to exercise different features of the api. Assuming we care about mobile / b2g at the higher levels of priority.

We have a bunch of mochitests for video in content/media/tests. If they could be run on mobile, or some way of running these to test the stagefright backend, it'd do wonders for making sure the code is correct.

Comment on attachment 625897 [details] [diff] [review] Part 1: Configure/Toolkit changes Requesting review from khuey in case ted is swamped.

Testing will move down two paths. One manual, one automated. There are currently a large repository of media tests insited content/media/test. The goal will be to add tests in there and run the tests across desktop and b2g. Testing across the more popular chipsets. Manual Testing. I have put together a dummy test side with audio / video clips of different sizes / encoding rates. http://people.mozilla.com/~dclarke/ Generally there will be some set of manual tests that will be iterated through. My next step is to build a version of B2G with GB as the kernel + above patches, and get to the state where we can begin manual tests.

You'll probably need to remove the use of <select> in your test as it appears to not work in the b2g browser. Possibly bug 759511.

OmxPlugin is picking up an incorrect channel count when initializing the audio.

Comment on attachment 625897 [details] [diff] [review] Part 1: Configure/Toolkit changes Removed :khuey since he's on vacation.

Comment on attachment 625897 [details] [diff] [review] Part 1: Configure/Toolkit changes Review of attachment 625897 [details] [diff] [review]: ----------------------------------------------------------------- Mostly nits. ::: configure.in @@ +221,5 @@ > > +MOZ_ARG_WITH_STRING(android-source, > +[ --with-android-source=DIR > + location where the Android source can be found], > + android_source=$withval) Since this is really for gonk, and since I don't think we should encourage that being used for android, I'd say please rename to --with-gonk-source (and move it with other gonk stuff) @@ +480,5 @@ > fi > > AC_SUBST(ANDROID_NDK) > AC_SUBST(ANDROID_TOOLCHAIN) > +AC_SUBST(ANDROID_SOURCE) This variable is not added to autoconf.mk.in. @@ +5661,5 @@ > +[ --enable-omx-plugin Enable building OMX plugin (android)], > + MOZ_OMX_PLUGIN=1, > + MOZ_OMX_PLUGIN=) > + > +if test -n "$MOZ_OMX_PLUGIN"; then Please add a check that OS_TARGET is Android (or whatever it is for B2G).

(In reply to Mike Hommey [:glandium] from comment #105) > Comment on attachment 625897 [details] [diff] [review] > Part 1: Configure/Toolkit changes > > Review of attachment 625897 [details] [diff] [review]: > ----------------------------------------------------------------- > > Mostly nits. > > ::: configure.in > @@ +221,5 @@ > > > > +MOZ_ARG_WITH_STRING(android-source, > > +[ --with-android-source=DIR > > + location where the Android source can be found], > > + android_source=$withval) > > Since this is really for gonk, and since I don't think we should encourage > that being used for android, I'd say please rename to --with-gonk-source > (and move it with other gonk stuff) Oh wait, I didn't see that: > * --with-android-source for setting the director for the android source > code (used for non-b2g android builds) So even better, just don't put the configure flag at all, but still AC_SUBST(ANDROID_SOURCE). People who want to test it can still use export ANDROID_SOURCE=/path/ in their mozconfig.

Thanks for the review! I'll address your comments in the next patch. (In reply to Mike Hommey [:glandium] from comment #105) > that being used for android, I'd say please rename to --with-gonk-source > (and move it with other gonk stuff) As per your comment 106 I'll remove this entirely and let people export ANDROID_SOURCE if they need it. > > AC_SUBST(ANDROID_NDK) > > AC_SUBST(ANDROID_TOOLCHAIN) > > +AC_SUBST(ANDROID_SOURCE) > > This variable is not added to autoconf.mk.in. It's already in autoconf.mk.in as it's set and used in gonk builds.

Address review comments, carrying forward r+

https://hg.mozilla.org/integration/mozilla-inbound/rev/d9ac6a5b9aea https://hg.mozilla.org/integration/mozilla-inbound/rev/85a3f4b90adc https://hg.mozilla.org/integration/mozilla-inbound/rev/09da0033b3a0

https://hg.mozilla.org/mozilla-central/rev/d9ac6a5b9aea https://hg.mozilla.org/mozilla-central/rev/85a3f4b90adc https://hg.mozilla.org/mozilla-central/rev/09da0033b3a0

Have been trying to get mp4 content to play in browser on B2G, and have not had much success. I have verified that these patches have been applied. http://www.808.dk/pics/video/gizmo.mp4 Can someone verify that with these patches you can play the h264 encoded content above ?

Chris whats up here? Our silicon vendor friends were complaining about horribly low frame rates too (5FPS etc).

(In reply to Andreas Gal :gal from comment #112) > Chris whats up here? Our silicon vendor friends were complaining about > horribly low frame rates too (5FPS etc). No idea. What device? Do they have an example video? Are they accessing over the network or from the offline cache? Buffering in the patch is pretty bad so may be related to that.

(In reply to dclarke@mozilla.com from comment #111) > > Can someone verify that with these patches you can play the h264 encoded > content above ? Video plays for me but sound is at the wrong rate. Note that currently you need to have the media in a <video> or <audio> element, you can't use a direct link. Try: http://cd.pn/t2

Hi Chris, I'm unable to view your test video from the last comment. Running on a Mac nightly, built from changeset: http://hg.mozilla.org/mozilla-central/rev/dd6ec482a85d. screnshot of video status spinning. Is there dependent fixes that Firefox Nightly needs to add before this streams correctly?

(In reply to Tony Chung [:tchung] from comment #115) > Created attachment 630008 [details] > screenshot of no video > > Hi Chris, I'm unable to view your test video from the last comment. > Running on a Mac nightly, built from changeset: > http://hg.mozilla.org/mozilla-central/rev/dd6ec482a85d. > > screnshot of video status spinning. > > Is there dependent fixes that Firefox Nightly needs to add before this > streams correctly? Nevermind, its on b2g phone only. confirmed, playback is really slow there.

(In reply to Tony Chung [:tchung] from comment #116) > > Nevermind, its on b2g phone only. confirmed, playback is really slow there. Audio playback is slow, right? Video playback is fine?

Confirmed audio playback is slow, also the controls for volume / rewind didn't seem to respond. Is this expected ?

Volume only mutes/unmutes as far as I know - and works for me. I don't see a rewind, only a seek bar, which works for me. Can you raise a bug for the slow audio playback issue?

I see that bug 761762 has been filed for the slow audio playback issue. Thanks, David! Are there any additional bugs here that need to be filed (like unable to control the volume on the phone or unable to rewind the video)? I don't have a B2G phone to look myself -- so if there there are additional bugs here, can we file them? Thanks!

Yes please raise bugs for issues identified in comment 112 and comment 118 (the rewind issue, the slow audio has a patch in bug 761762). I don't see these and would like more detail. Other than those reports I don't know of any other user facing issue with B2G MP4 playback so if you know any, please file.

Maire, ok , but i noticed was that there is volume control on the samsung sII on the left hand side of the device, so having one on the content might not be necessary. Rewind issue was more related to me getting used to B2G device interface. It works for me now.

(In reply to dclarke@mozilla.com from comment #122) > Maire, ok , but i noticed was that there is volume control on the samsung > sII on the left hand side of the device, so having one on the content might > not be necessary. > > Rewind issue was more related to me getting used to B2G device interface. It > works for me now. Great, David! Really I meant to ask if there were any bugs not filed that should be. It sounds like you believe all the open issues have been filed as bugs (which is excellent -- thank you!) And I agree that as long as there is at least one good way to easily and intuitively adjust the volume on the phone we are shipping (like volume control buttons on the side that are known to work), then I agree we're good. To Chris D's (doublec) question: Have you seen the problems that Andreas mentioned in comment 112 ("horribly low frame rates.. (5fps)") at any time in your testing? It sounds like you haven't? Have you been doing all of your testing on the Samsung SII or have you been testing on other phones as well? Thanks!

Yes I have, but I assume we are fixing that in the follow up bug, where the hardware accelerated version is being tracked. people.mozilla.com/~dclarke/b.html <--- I have a 720p sample. I think 720p on mobile is fairly standard nowadays. I am filing bugs as i find / think of them.

(In reply to dclarke@mozilla.com from comment #124) > Yes I have, but I assume we are fixing that in the follow up bug, where the > hardware accelerated version is being tracked. Is that Bug 759506? If so, could you update that bug with your comments on what you see? We don't have many details in the bug on exactly what the problem is from the user's point of view, and I believe the developers who are working the problem (in NZ) aren't seeing the problem themselves yet. And is all your testing on the Samsung SII so far? Thanks so much, David!

I'm not aware of a "hardware accelerated version" followup bug. Do you have a bug number? Bug 759506 optimizes code to reduce copying afaik. Maire, I have not seen horribly low frame rates but I haven't been testing large videos yet. David, when you see things like that can you raise a bug specifically for it? We can always dupe it if it's covered by something else. Maire, all my testing is on a Galaxy Nexus S.

David please do not remove the MozTrap flag in this case - we need someone to add MozTrap test cases for this feature, especially since manual test cases will need to exist that are cross-functional across B2G and Android. We should get automation in place as well, so the in-testsuite? flag makes sense.

Well I actually do mind it being added because this is not really a feature that is testable. Maybe if you had worked in this area, or understood the bug better that would help.