1067442 - H.264 OMX video isn't encoded in Webrtc on b2g 2.2

Status: RESOLVED FIXED

(Core :: WebRTC: Audio/Video, defect)

Description

[Oscar Patiño González]

I'm working with two mobiles. If both devices have Master branch installed, when running loop I can't see any video on screen, although I can hear audio.
I don't know yet if video is not being tx, or decoding is failing, or maybe both.

Comment 1

[Oscar Patiño González]

Randell, do you know if there has been any change that could affect video decoding or tx in Master branch?
Thanks

Comment 2

[Randell Jesup [:jesup] (needinfo me)]

There is a problem that was causing loss of chroma (color info); depending on how it manifests itself this could cause black video - though I can't say that is the cause here.

See bug 1066410, which landed on inbound today, and likely will be in nightly tomorrow.

Comment 3

[Oscar Patiño González]

I tested flame with last version against fire-e with v2.0. No video on any side. I think this bug is a duplicate of https://bugzilla.mozilla.org/show_bug.cgi?id=1068394

Comment 4

[Randell Jesup [:jesup] (needinfo me)]

This turned out to be a different bug than bug 10638394 - that is a decoder bug, this is an issue with img->IsValid() failing for PlanarYCrCb buffers which were created with SetDataNoCopy(data)

This is a regression from the landing of bug 997593

Comment 5

[Randy Lin [:rlin] - Be Mozillian on 2015/01/26]

Hi John, 
Could you help this?

Comment 6

[John Lin [:jhlin][:jolin]]

My response about InValid() failing in bug 1068394 comment 30 is in fact not correct! Sorry about that.
Turns out that although SetDataNoCopy() doesn't set mBufferSize, calling AllocateAndGetNewBuffer() at [1] does it.

Anyway, I built latest m-c and did some tests. On my flames those camera images were rejected at [2] and never were sent to encoder. Randell, do you know about this?

[1] http://dxr.mozilla.org/mozilla-central/source/content/media/webrtc/MediaEngineWebRTCVideo.cpp#1015
[2] http://dxr.mozilla.org/mozilla-central/source/media/webrtc/signaling/src/mediapipeline/MediaPipeline.cpp#982

Comment 7

[John Lin [:jhlin][:jolin]]

(In reply to John Lin[:jolin][:jhlin] from comment #6)
> My response about InValid() failing in bug 1068394 comment 30 is in fact not
> correct! Sorry about that.
> Turns out that although SetDataNoCopy() doesn't set mBufferSize, calling
> AllocateAndGetNewBuffer() at [1] does it.

Oops! Just found that [1] is where it fails. I'll upload the patch later.

[1] http://dxr.mozilla.org/mozilla-central/source/media/webrtc/signaling/src/media-conduit/WebrtcOMXH264VideoCodec.cpp#940

Comment 8

[John Lin [:jhlin][:jolin]]

Attachment: Validate input image using Y channel pointer in mData.

Comment 9

[Randell Jesup [:jesup] (needinfo me)]

Comment on attachment 8494421 [details] [diff] [review]
Validate input image using Y channel pointer in mData.

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

::: content/media/omx/OMXCodecWrapper.cpp
@@ +396,5 @@
>      NS_ENSURE_TRUE(aWidth == size.width, NS_ERROR_INVALID_ARG);
>      NS_ENSURE_TRUE(aHeight == size.height, NS_ERROR_INVALID_ARG);
>      if (format == ImageFormat::PLANAR_YCBCR) {
> +      const PlanarYCbCrData* yuv = static_cast<PlanarYCbCrImage*>(img)->GetData();
> +      NS_ENSURE_TRUE(yuv->mYChannel, NS_ERROR_INVALID_ARG);

Add comment "// test that works with SetDataNoCopy() on empty image" or equivalent

::: media/webrtc/signaling/src/media-conduit/WebrtcOMXH264VideoCodec.cpp
@@ +936,5 @@
>                                     (yuvData.mYSize.height + 1) / 2);
>    yuvData.mPicSize = yuvData.mYSize;
>    yuvData.mStereoMode = StereoMode::MONO;
>    layers::PlanarYCbCrImage img(nullptr);
> +  // SetDataNoCopy() w/o AllocateAndGetNewBuffer() only works with OMXVideoEncoder.

There are other users of SetDataNoCopy() without AllocateAndGetNewBuffer() (GStreamerReader, MediaData).  Probably say OMXVideoEncoder doesn't need AllocateAndGetNewBuffer() when SetDataNoCopy is used.

Comment 10

[Randell Jesup [:jesup] (needinfo me)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/941da0d74928

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/941da0d74928

Comment 12

[Bobby Chien]

remove from nomination.