803394 - Implement color conversion routine used by Sony Ericsson devices to enable hardware acceleration on Android

Status: RESOLVED FIXED

(Core :: Audio/Video, defect)

Description

[cajbir (:cajbir)]

This is a followup to bug 787319. The Sony Ericsson devices use color conversion format 0x7fa30c03 (QOMX_COLOR_FormatYUV420PackedSemiPlanar64x32Tile2m8ka). We need to implement this for hardware accelerated video playback on stagefright to work. Currently playback is done using the stagefright software decoder.

Comment 1

[cajbir (:cajbir)]

Some information on this color format:

http://stackoverflow.com/questions/10059738/qomx-color-formatyuv420packedsemiplanar64x32tile2m8ka-color-format

Comment 2

[cajbir (:cajbir)]

Android has a ColorConverter class as mentioned in bug 794061 comment 3. Investigate if we can use this here.

Comment 3

[Maire Reavy [:mreavy]]

Edwin, can you look at this after Bug 784019?  Thanks.

Comment 4

[Edwin Flores [inactive from 2016-12-01] [:eflores] [:edwin]]

Attachment: Something that works - WIP

Works on my phone, but far from finished. Still need to:
 - Get it working with async image containers
 - Make the buffer callback not ****
 - Generally clean it up a bit
 - Other little TODOs

Comment 5

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 682231 [details] [diff] [review]
Something that works - WIP

Review of attachment 682231 [details] [diff] [review]:
-----------------------------------------------------------------

You should break this patch down into smaller patches --- one that adds the new Image type, one that adds support to OMX/libstagefright, one that adds support to nsBuiltinDecoderReader etc.

::: content/html/content/src/nsHTMLMediaElement.cpp
@@ +3202,5 @@
>    if (!video)
>      return nullptr;
>  
>    mVideoFrameContainer =
> +    new VideoFrameContainer(this, LayerManager::CreateImageContainer());

Why are you changing this?

::: content/media/plugins/MPAPI.h
@@ +17,5 @@
> +
> +class BufferCallback {
> +public:
> +  virtual void *operator()(size_t aWidth, size_t aHeight,
> +                           ColorFormat aColorFormat) = 0;

Needs documentation.

::: gfx/layers/ImageContainer.h
@@ +733,5 @@
>    nsRefPtr<BufferRecycleBin> mRecycleBin;
>  };
>  
>  /**
> + * Stores RGB data, plain and simple.

Can you say something more about this? Channel layout etc?

You probably should say that this is for storing thread-safe buffers of in-memory RGB data.

If we're going to support, say, A8, we may want to rename this to something else. Say GfxFormatImage?

@@ +740,5 @@
> +  typedef gfxASurface::gfxImageFormat gfxImageFormat;
> +public:
> +  struct Data {
> +    uint8_t *mBuffer;
> +    size_t mBufferSize;

Why do we need mBufferSize?

Do we need to store a stride?

@@ +742,5 @@
> +  struct Data {
> +    uint8_t *mBuffer;
> +    size_t mBufferSize;
> +    gfxIntSize mSize;
> +    gfxImageFormat mImageFormat;

What image formats are allowed here?

Comment 6

[Edwin Flores [inactive from 2016-12-01] [:eflores] [:edwin]]

Attachment: WIP

Rebase'd

Comment 7

[Edwin Flores [inactive from 2016-12-01] [:eflores] [:edwin]]

Attachment: WIP

Forgot ColorConverter.h

Comment 8

[Edwin Flores [inactive from 2016-12-01] [:eflores] [:edwin]]

Attachment: Patch 1 - Add image class for RGB images in shared memory

Comment 9

[Edwin Flores [inactive from 2016-12-01] [:eflores] [:edwin]]

Attachment: Patch 2 - Pass a callback to media plugins to request gecko-owned video buffers

Comment 10

[Edwin Flores [inactive from 2016-12-01] [:eflores] [:edwin]]

Attachment: Patch 3 - Use Android ColorConverter to support unknown color formats from libstagefright

Comment 11

[Edwin Flores [inactive from 2016-12-01] [:eflores] [:edwin]]

Should note that this patch works on my HTC One X, but we don't have a Sony Ericsson device with us until next week when we're back in the office, but this *should* work.

Comment 12

[Nicolas Silva [:nical]]

Comment on attachment 682652 [details] [diff] [review]
Patch 1 - Add image class for RGB images in shared memory

Review of attachment 682652 [details] [diff] [review]:
-----------------------------------------------------------------

::: gfx/layers/ipc/SharedRGBImage.cpp
@@ +25,5 @@
> +
> +SharedRGBImage::~SharedRGBImage()
> +{
> +  mImageContainerChild->DeallocShmemAsync(*mShmem);
> +  mImageContainerChild->Release();

please assert that you are on the ImageBridgeChild thread here

@@ +98,5 @@
> +  }
> +
> +  if (mImageContainerChild->AllocUnsafeShmemSync(size, OptimalShmemType(), mShmem)) {
> +    mAllocated = true;
> +  }

Please assert that you are on the imageBridgeChild thread

::: gfx/thebes/gfxASurface.cpp
@@ +687,5 @@
> +  switch (aImageFormat) {
> +    case ImageFormatARGB32:
> +      return 4;
> +    case ImageFormatRGB24:
> +      return 3;

cairo's doc says:

CAIRO_FORMAT_RGB24
each pixel is a 32-bit quantity, with the upper 8 bits unused. Red, Green, and Blue are stored in the remaining 24 bits in that order

so you should return 4 here, else what you added in ImageLayerOGL.cpp to compute the stride will lead to nasty things.

Comment 13

[Nicolas Silva [:nical]]

(In reply to Nicolas Silva [:nical] from comment #12)
> Comment on attachment 682652 [details] [diff] [review]
> Patch 1 - Add image class for RGB images in shared memory
> 
> Review of attachment 682652 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: gfx/layers/ipc/SharedRGBImage.cpp
> @@ +25,5 @@
> > +
> > +SharedRGBImage::~SharedRGBImage()
> > +{
> > +  mImageContainerChild->DeallocShmemAsync(*mShmem);
> > +  mImageContainerChild->Release();
> 
> please assert that you are on the ImageBridgeChild thread here
> 
> @@ +98,5 @@
> > +  }
> > +
> > +  if (mImageContainerChild->AllocUnsafeShmemSync(size, OptimalShmemType(), mShmem)) {
> > +    mAllocated = true;
> > +  }
> 
> Please assert that you are on the imageBridgeChild thread

Sorry, my mistake: you don't actually need to assert being on the ImageBridgeChild thread in these two methods.

Comment 14

[Edwin Flores [inactive from 2016-12-01] [:eflores] [:edwin]]

Attachment: Patch 1 v2 - Add image class for RGB images in shared memory

Rebased and addressed review comment

Comment 15

[Edwin Flores [inactive from 2016-12-01] [:eflores] [:edwin]]

Attachment: Patch 3 v2 - Use Android ColorConverter to support unknown color formats from libstagefright

Plug memory leak

Comment 16

[Edwin Flores [inactive from 2016-12-01] [:eflores] [:edwin]]

Attachment: Patch 4 - Feed MIME type into libstagefright

The libstagefright MIME type sniffing causes us to crash on Sony devices. Since we already know the MIME type, we can just pass it into stagefright to avoid it altogether.

Comment 17

[cajbir (:cajbir)]

(In reply to Edwin Flores [:eflores] [:edwin] from comment #16)
> The libstagefright MIME type sniffing causes us to crash on Sony devices.
> Since we already know the MIME type, we can just pass it into stagefright to
> avoid it altogether.

The crashing happens on the sony ericsson device because our DataSource layout is incorrect. See bug 787319.

Comment 18

[Edwin Flores [inactive from 2016-12-01] [:eflores] [:edwin]]

(In reply to Chris Double (:doublec) from comment #17)
> The crashing happens on the sony ericsson device because our DataSource
> layout is incorrect. See bug 787319.

Ah, forgot about that. That sucks.

Comment 19

[cajbir (:cajbir)]

Comment on attachment 682653 [details] [diff] [review]
Patch 2 - Pass a callback to media plugins to request gecko-owned video buffers

Review of attachment 682653 [details] [diff] [review]:
-----------------------------------------------------------------

::: content/media/plugins/MPAPI.h
@@ +22,5 @@
> +class BufferCallback {
> +public:
> +  virtual void *operator()(size_t aWidth, size_t aHeight,
> +                           ColorFormat aColorFormat) = 0;
> +};

Include a comment for who owns the void* memory returned by operator(). Add a virtual destructor to this class.

::: content/media/plugins/MediaPluginReader.cpp
@@ +349,5 @@
> +}
> +
> +void *
> +MediaPluginReader::ImageBufferCallback::operator()(size_t aWidth, size_t aHeight,
> +                                                     MPAPI::ColorFormat aColorFormat)

Align MPAPI with size_t above it.

::: media/omx-plugin/OmxPlugin.cpp
@@ +147,5 @@
>    void ToVideoFrame_YUV420SemiPlanar(VideoFrame *aFrame, int64_t aTimeUs, void *aData, size_t aSize, bool aKeyFrame);
>    void ToVideoFrame_YVU420SemiPlanar(VideoFrame *aFrame, int64_t aTimeUs, void *aData, size_t aSize, bool aKeyFrame);
>    void ToVideoFrame_YUV420PackedSemiPlanar(VideoFrame *aFrame, int64_t aTimeUs, void *aData, size_t aSize, bool aKeyFrame);
>    void ToVideoFrame_YVU420PackedSemiPlanar32m4ka(VideoFrame *aFrame, int64_t aTimeUs, void *aData, size_t aSize, bool aKeyFrame);
> +  bool ToVideoFrame(VideoFrame *aFrame, int64_t aTimeUs, void *aData, size_t aSize, bool aKeyFrame, BufferCallback *aBufferCallback);

Add a comment explaining what aBufferCallback is supposed to do.

Comment 20

[cajbir (:cajbir)]

Comment on attachment 684269 [details] [diff] [review]
Patch 3 v2 - Use Android ColorConverter to support unknown color formats from libstagefright

Review of attachment 684269 [details] [diff] [review]:
-----------------------------------------------------------------

You'll need to rebase on top of the Gingerbread and Honeycomb changes. I suggest just #ifdefing on MOZ_ANDROID_GB for now and turn off the color conversion code there and raise a bug for GB support where it can be fixed there.

update.sh and update.patch in media/omx-plugin/include/ics will need to be adjusted for the new ColorConverter.h header file.

::: media/omx-plugin/include/ics/stagefright/ColorConverter.h
@@ +16,5 @@
> +
> +#ifndef COLOR_CONVERTER_H_
> +
> +#define COLOR_CONVERTER_H_
> +

update.sh and update.patch will need to be fixed as well.

Comment 21

[cajbir (:cajbir)]

When building with patch 1 applied I get a link error:

gfx/layers/ipc/SharedRGBImage.cpp:77: undefined reference to `gfxASurface::BytesPerPixel(gfxASurface::gfxImageFormat)

Comment 22

[Edwin Flores [inactive from 2016-12-01] [:eflores] [:edwin]]

Attachment: Patch 4 - Gingerbread support

Comment 23

[Edwin Flores [inactive from 2016-12-01] [:eflores] [:edwin]]

Attachment: Patch 4 rev 2 - Gingerbread support

Comment 24

[Edwin Flores [inactive from 2016-12-01] [:eflores] [:edwin]]

Attachment: Patch 5 - Build change for Gingerbread ColorConverter library

Comment 25

[cajbir (:cajbir)]

Comment on attachment 688579 [details] [diff] [review]
Patch 4 rev 2 - Gingerbread support

Review of attachment 688579 [details] [diff] [review]:
-----------------------------------------------------------------

Do you need to make changes for gingerbread versions greater than 2.3.5? Is it the same as pre 2.3.5? I'm not sure what the "zero length frame" handling error code is for either. I've r- based on needing the other gingerbread code changed and an investigation of what actually is going on that required the zero length frame fix.

::: media/omx-plugin/OmxPlugin.cpp
@@ +805,5 @@
>      LOG("mVideoSource ERROR %#x", err);
>    }
> +  else {
> +    LOG("Got zero length frame");
> +    return ReadVideo(aFrame, aSeekTimeUs, aBufferCallback);

I don't understand what this is for. Zero length frames are handled by the caller. And if ToVideoFrame is failing the length is not zero for that code to be called.

Comment 26

[Edwin Flores [inactive from 2016-12-01] [:eflores] [:edwin]]

Attachment: Patch 4 rev 3 - Gingerbread support

mSize wasn't being initialised, it seems.

Runs fine on the Samsung Galaxy Y running Android 2.3.6.

Comment 27

[Maire Reavy [:mreavy]]

(In reply to Edwin Flores [:eflores] [:edwin] from comment #26)
> Created attachment 689045 [details] [diff] [review]
> Patch 4 rev 3 - Gingerbread support
> 
> mSize wasn't being initialised, it seems.
> 
> Runs fine on the Samsung Galaxy Y running Android 2.3.6.

I may have misunderstood Chris Double, but I *think* playback was already working on the Samsung Galaxy Y.

Have you tried your patches from this bug on the Samsung Galaxy S2 or Galaxy Note?  

I know playback on those (S2 and Note) were failing, and the Android folks are anxious to get H.264 playback working on those because they are so popular in the field.

I believe both the Galaxy S2 and Galaxy Note, with GB already installed, were shipped to the NZ office a few weeks back and Chris D was testing his decoding patches against them.  So they should be in the office there or with Chris D.

Thanks!

Comment 28

[cajbir (:cajbir)]

(In reply to Maire Reavy [:mreavy] from comment #27)
> I may have misunderstood Chris Double, but I *think* playback was already
> working on the Samsung Galaxy Y.

It was the Gingerbread Otoro it worked on, I was mistaken about the Galaxy Y.

> I believe both the Galaxy S2 and Galaxy Note, with GB already installed,
> were shipped to the NZ office a few weeks back and Chris D was testing his
> decoding patches against them.  So they should be in the office there or
> with Chris D.

Video is still not working on the S2 - we're investigating what to do there. We don't have a Note with GB on it - are you sure one was shipped? If so, we never got it.

Comment 29

[Maire Reavy [:mreavy]]

(In reply to Chris Double (:doublec) from comment #28)
> (In reply to Maire Reavy [:mreavy] from comment #27)
> > I may have misunderstood Chris Double, but I *think* playback was already
> > working on the Samsung Galaxy Y.
> 
> It was the Gingerbread Otoro it worked on, I was mistaken about the Galaxy Y.
> 
> > I believe both the Galaxy S2 and Galaxy Note, with GB already installed,
> > were shipped to the NZ office a few weeks back and Chris D was testing his
> > decoding patches against them.  So they should be in the office there or
> > with Chris D.
> 
> Video is still not working on the S2 - we're investigating what to do there.
> We don't have a Note with GB on it - are you sure one was shipped? If so, we
> never got it.

doublec -- Thanks for the clarification on all the points above.  I misremembered about the Note.  (I checked my notes -- sorry for the pun.)  We didn't ship you a Note.  I asked cpeterson and blassey to try out your decoding patch on a Note running GB since they had access to Galaxy Notes, and cpeterson reported "My Galaxy S2 and Note display the video's playback controls, but pressing play just displays a white box and no audio."  See https://bugzilla.mozilla.org/show_bug.cgi?id=787228#c11

I also thought you had a Galaxy Note that you personally owned (but it sounds like that was bad info or I misheard/misunderstood).

Is it worth my asking cpeterson or blassey to try this patch on a Galaxy Note running GB to see if video playback now works, or do you think video playback is unlikely to work since video playback on the S2 doesn't work yet?  

Thanks!!

Comment 30

[cajbir (:cajbir)]

(In reply to Maire Reavy [:mreavy] from comment #29)
> I also thought you had a Galaxy Note that you personally owned (but it
> sounds like that was bad info or I misheard/misunderstood).

I personally own a Note and a Note 2. The former runs ICS the latter JB unfortunately. Well, fortunately for me as a user, unfortunately for us as wanting to test GB!

> Is it worth my asking cpeterson or blassey to try this patch on a Galaxy
> Note running GB to see if video playback now works, or do you think video
> playback is unlikely to work since video playback on the S2 doesn't work
> yet?  

Testing this patch on a Note running GB would be great. We should also get the HTC desire on Monday which we'll test on.

Worse case for the SGS 2 we'll have to implement it's colour conversion function specifically (once we find out what format it is).

Comment 31

[Maire Reavy [:mreavy]]

blassey, cpeterson -- Can one of you apply the patches from this bug and see if H.264 video playback now works on a Galaxy Note running GB?  

We're interested in knowing what you see and hear when you try to play a H.264 video on the Note.  Do you get audio and video or just audio or neither?  Is the behavior any different from what cpeterson observed a few weeks ago in https://bugzilla.mozilla.org/show_bug.cgi?id=787228#c11?

Thanks!

Comment 32

[Chris Peterson [:cpeterson]]

(In reply to Maire Reavy [:mreavy] from comment #31)
> blassey, cpeterson -- Can one of you apply the patches from this bug and see
> if H.264 video playback now works on a Galaxy Note running GB?  

I have a Galaxy Note running GB 2.3.5.

Comment 33

[Chris Peterson [:cpeterson]]

eflores, doublec:

With this patches applied to mozilla-central, I get the following build error because the libstagefright_color_conversion files seem to be missing:

make[5]: *** No rule to make target `/Users/cpeterson/Code/mozilla/central/media/omx-plugin/lib/gb/libstagefright_color_conversion/Makefile.in', needed by `media/omx-plugin/lib/gb/libstagefright_color_conversion/Makefile'.  Stop.

Comment 34

[Edwin Flores [inactive from 2016-12-01] [:eflores] [:edwin]]

Attachment: Patch 4 rev 4 - Gingerbread support

I made a dumb. *sigh*

Added those files.

Comment 35

[cajbir (:cajbir)]

Attachment 682653 [details] [diff] needs to be rebased for the s/nsVideo/Video/ changes.

Comment 36

[Edwin Flores [inactive from 2016-12-01] [:eflores] [:edwin]]

Attachment: Patch 1 v3 - Add image class for RGB images in shared memory

Fixed link error with gfxASurface::BytesPerPixel.

Carry over r=nical

Comment 37

[Edwin Flores [inactive from 2016-12-01] [:eflores] [:edwin]]

Build for testing this:

https://people.mozilla.com/~eflores/builds/fennec-20.0a1.en-US.android-arm.apk

Comment 38

[Chris Peterson [:cpeterson]]

I'm still unable to get this build to work on my Galaxy Note running GB. I set about:config pref "stagefright.force-enabled" = true, but when I try to play a video, adb logcat reports:

E/GeckoConsole( 3980): [JavaScript Warning: "HTTP "Content-Type" of "video/mp4" is not supported. Load of media resource https://people.mozilla.com/~cpeterson/videos/toodee-480p.mp4 failed." {file: "https://people.mozilla.com/~cpeterson/videos/toodee-480p.html" line: 0}]

Comment 39

[cajbir (:cajbir)]

Can you provide the portion of logcat that shows from the libomxplugin.so shared library that is loaded up to the bit with the error message you quote?

Comment 40

[cajbir (:cajbir)]

I want to see what the Note says for the following (which is from an SGS 2):

I/MediaPluginHost( 4995): Android Version is: 10
I/MediaPluginHost( 4995): Android Release Version is: 2.3.4
I/MediaPluginHost( 4995): Android Device is: GT-I9100
I/MediaPluginHost( 4995): Loading OMX Plugin: lib/libomxplugingb235.so
I/MediaPluginHost( 4995): OMX plugin successfully loaded

Comment 41

[Chris Peterson [:cpeterson]]

Good news! I was not able to play a 720p video, but I was able to play a couple 480p videos. Here are links and logs:


* FAIL! This 720p video does NOT play:
https://people.mozilla.com/~cpeterson/videos/toodee-720p.mp4

I/MediaPluginHost( 4348): Android Version is: 10
I/MediaPluginHost( 4348): Android Release Version is: 2.3.5
I/MediaPluginHost( 4348): Loading OMX Plugin: lib/libomxplugingb235.so
I/MediaPluginHost( 4348): OMX plugin successfully loaded
I/MediaExtractor( 4348): Autodetected media content as 'video/mpeg4' with confidence 0.40
I/OMXCodec( 4348): OMXCodec::Create matchComponentName ((null)), flags (0)
D/OMXCodec( 4348): componentName=OMX.SEC.avcdec, quirks=73728, flags=0
I/OMXCodec( 4348): eColorFormat (19)
I/OmxPlugin( 4348): Unknown video color format: 0x2c
I/OmxPlugin( 4348): Falling back to software decoder
I/OMXCodec( 4348): OMXCodec::Create matchComponentName ((null)), flags (1)
I/OMXCodec( 4348): OMXCodec::Create matchComponentName ((null)), flags (0)
D/OMXCodec( 4348): componentName=OMX.SEC.aacdec, quirks=73728, flags=0
I/OMXCodec( 4348): start()
I/OMXCodec( 4348): init()
I/OMXCodec( 4348): [OMX.SEC.aacdec] allocating 5 buffers of size 8192 on input port
I/OMXCodec( 4348): [OMX.SEC.aacdec] allocating 9 buffers of size 8192 on output port
I/OmxPlugin( 4348): crop rect not available, assuming no cropping
I/OmxPlugin( 4348): rotation not available, assuming 0
I/OmxPlugin( 4348): width: 1280 height: 720 component: AVCDecoder format: 0x13 stride: 1280 sliceHeight: 720 rotation: 0 crop: 0,0-1279,719
I/OMXCodec( 4348): [OMX.SEC.aacdec] allocating 9 buffers of size 8192 on output port
I/OMXCodec( 4348): [OMX.SEC.aacdec] allocating 9 buffers of size 8192 on output port
I/OmxPlugin( 4348): mVideoSource INFO_FORMAT_CHANGED
I/OmxPlugin( 4348): crop rect not available, assuming no cropping
I/OmxPlugin( 4348): rotation not available, assuming 0
I/OmxPlugin( 4348): width: 16 height: 32 component: AVCDecoder format: 0x13 stride: 16 sliceHeight: 32 rotation: 0 crop: 0,0-15,31
I/OmxPlugin( 4348): mVideoSource ERROR 0x80000000
I/OmxPlugin( 4348): mVideoSource ERROR 0x80000000


* PASS! This 480p video DOES play:
https://people.mozilla.com/~cpeterson/videos/toodee-480p.mp4

I/MediaExtractor( 4348): Autodetected media content as 'video/mpeg4' with confidence 0.40
I/OMXCodec( 4348): OMXCodec::Create matchComponentName ((null)), flags (0)
D/OMXCodec( 4348): componentName=OMX.SEC.avcdec, quirks=73728, flags=0
I/OMXCodec( 4348): eColorFormat (19)
I/OmxPlugin( 4348): Unknown video color format: 0x2c
I/OmxPlugin( 4348): Falling back to software decoder
I/OMXCodec( 4348): OMXCodec::Create matchComponentName ((null)), flags (1)
I/OMXCodec( 4348): OMXCodec::Create matchComponentName ((null)), flags (0)
D/OMXCodec( 4348): componentName=OMX.SEC.aacdec, quirks=73728, flags=0
I/OMXCodec( 4348): start()
I/OMXCodec( 4348): init()
I/OMXCodec( 4348): [OMX.SEC.aacdec] allocating 5 buffers of size 8192 on input port
I/OMXCodec( 4348): [OMX.SEC.aacdec] allocating 9 buffers of size 8192 on output port
I/OmxPlugin( 4348): crop rect not available, assuming no cropping
I/OmxPlugin( 4348): rotation not available, assuming 0
I/OmxPlugin( 4348): width: 480 height: 270 component: AVCDecoder format: 0x13 stride: 480 sliceHeight: 270 rotation: 0 crop: 0,0-479,269
I/OMXCodec( 4348): [OMX.SEC.aacdec] allocating 9 buffers of size 8192 on output port
I/OMXCodec( 4348): [OMX.SEC.aacdec] allocating 9 buffers of size 8192 on output port
I/OmxPlugin( 4348): mVideoSource INFO_FORMAT_CHANGED
I/OmxPlugin( 4348): crop rect not available, assuming no cropping
I/OmxPlugin( 4348): rotation not available, assuming 0
I/OmxPlugin( 4348): width: 480 height: 272 component: AVCDecoder format: 0x13 stride: 480 sliceHeight: 272 rotation: 0 crop: 0,0-479,271

Comment 42

[Chris Peterson [:cpeterson]]

The 720p video's mVideoSource INFO_FORMAT_CHANGED from 1280x720 to 16x32 is quite odd.

Comment 43

[cajbir (:cajbir)]

The Note is showing the same thing as the SGS 2. Color format 0x2c unknown and falling back to the software decoder. 0x2c  is an undocumented format but seems to be the format that has garbage video data except for the first few bytes which in some manner points to the actual memory buffer. Edwin did some hacking on this format a while back which involved having to search through memory for the video buffer IIRC. Edwin, can you shed some light here? Do you still have that code?

Comment 44

[Edwin Flores [inactive from 2016-12-01] [:eflores] [:edwin]]

Unfortunately, I think I threw out that code long ago.

The frame has four pointers -- some permutation of {virtual,physical} * {input,output} buffers. There is a device (I forget the name) on which we can mmap the virtual output pointer and get the frame back in a nice format. We may need special permissions for it, however.

Comment 45

[cajbir (:cajbir)]

Comment on attachment 690159 [details] [diff] [review]
Patch 4 rev 4 - Gingerbread support

Review of attachment 690159 [details] [diff] [review]:
-----------------------------------------------------------------

::: media/omx-plugin/lib/gb/libstagefright_color_conversion/Makefile.in
@@ +53,5 @@
> +
> +# EXTRA_DSO_LDOPTS += \
> +# 		-L$(DEPTH)/media/omx-plugin/lib/gb/libutils \
> +# 		-lutils \
> +# 		$(NULL)

Delete this commented out code.

Comment 46

[cajbir (:cajbir)]

I've raised bug 820236 for the SGS 2 and Galaxy Note on Gingerbread video issues.

Comment 47

[Maire Reavy [:mreavy]]

(In reply to Edwin Flores [:eflores] [:edwin] from comment #37)
> Build for testing this:
> 
> https://people.mozilla.com/~eflores/builds/fennec-20.0a1.en-US.android-arm.
> apk

blassey -- here is the build for testing on an HTC Desire HD.  Thanks for your help with testing this!

Comment 48

[cajbir (:cajbir)]

I've tested on an HTC EVO Design 4G running Gingerbread 2.3.4. The bad news is stagefright playback fails due to differences in the DataSource class layout which I have to track down.

The good news is if I use the builtin Android DataSource class for reading from HTTP streams then playback works with video displaying correctly. This means the color conversion code for GB is working on this device.

I'll raise a bug for the 'bad news' issue.

Comment 49

[cajbir (:cajbir)]

Bug 820661 for the DataSource issue on the HTC EVO Design 4G. Fix for the issue is attached to that bug. With that fix, and the patches from this bug, video plays on the phone.

Comment 50

[cajbir (:cajbir)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/f7830f9b52d8
https://hg.mozilla.org/integration/mozilla-inbound/rev/544450118868
https://hg.mozilla.org/integration/mozilla-inbound/rev/b99888806337
https://hg.mozilla.org/integration/mozilla-inbound/rev/64e2e88fdf66
https://hg.mozilla.org/integration/mozilla-inbound/rev/2d36cae22daf

Comment 51

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/f7830f9b52d8
https://hg.mozilla.org/mozilla-central/rev/544450118868
https://hg.mozilla.org/mozilla-central/rev/b99888806337
https://hg.mozilla.org/mozilla-central/rev/64e2e88fdf66
https://hg.mozilla.org/mozilla-central/rev/2d36cae22daf