918186 - crash in java.lang.NullPointerException: at org.webrtc.videoengine.VideoCaptureAndroid.onPreviewFrame(VideoCaptureAndroid.java)

Status: RESOLVED FIXED

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

Description

[Jason Smith [:jsmith]]

This bug was filed from the Socorro interface and is 
report bp-9cc974fc-ab81-407d-a700-7ffec2130918.
=============================================================

java.lang.NullPointerException
	at org.webrtc.videoengine.VideoCaptureAndroid.onPreviewFrame(VideoCaptureAndroid.java:341)
	at android.hardware.Camera$EventHandler.handleMessage(Camera.java:791)
	at android.os.Handler.dispatchMessage(Handler.java:99)
	at android.os.Looper.loop(Looper.java:137)
	at android.app.ActivityThread.main(ActivityThread.java:5303)
	at java.lang.reflect.Method.invokeNative(Native Method)
	at java.lang.reflect.Method.invoke(Method.java:525)
	at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:739)
	at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:555)
	at de.robv.android.xposed.XposedBridge.main(XposedBridge.java:112)
	at dalvik.system.NativeStart.main(Native Method)

https://crash-stats.mozilla.com/report/list?signature=java.lang.NullPointerException%3A+at+org.webrtc.videoengine.VideoCaptureAndroid.onPreviewFrame%28VideoCaptureAndroid.java%29&product=FennecAndroid&query_type=contains&range_unit=weeks&process_type=any&hang_type=any&date=2013-09-19+03%3A00%3A00&range_value=4

Comment 1

[Jason Smith [:jsmith]]

Looks like we're crashing here:

https://mxr.mozilla.org/mozilla-release/source/media/webrtc/trunk/webrtc/modules/video_capture/android/java/org/webrtc/videoengine/VideoCaptureAndroid.java#341

Which implies that data is a null value.

Comment 2

[Doug Turner (:dougt)]

jason, want to patch it? :)

Comment 3

[Jason Smith [:jsmith]]

Attachment: Null check data to prevent NullPointerException v1

Comment 4

[Gian-Carlo Pascutto [:gcp]]

Comment on attachment 811631 [details] [diff] [review]
Null check data to prevent NullPointerException v1

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

At first sight I was afraid this was plastering over another error in our WebRTC code and potentially making it harder to find, but this function is in fact only ever called from Android system code, so it's not our fault as far as I can tell.

We should probably look if this is specific to certain devices and/or file an upstream bug. Note that we have other code that's assuming data can't be null there:
http://dxr.mozilla.org/mozilla-central/source/netwerk/protocol/device/CameraStreamImpl.cpp#l38

From a bit of Googling, this might be a bug in certain ROMs.

Comment 5

[Jason Smith [:jsmith]]

(In reply to Gian-Carlo Pascutto (:gcp) from comment #4)
> Comment on attachment 811631 [details] [diff] [review]
> Null check data to prevent NullPointerException v1
> 
> Review of attachment 811631 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> At first sight I was afraid this was plastering over another error in our
> WebRTC code and potentially making it harder to find, but this function is
> in fact only ever called from Android system code, so it's not our fault as
> far as I can tell.
> 
> We should probably look if this is specific to certain devices and/or file
> an upstream bug. Note that we have other code that's assuming data can't be
> null there:
> http://dxr.mozilla.org/mozilla-central/source/netwerk/protocol/device/
> CameraStreamImpl.cpp#l38
> 
> From a bit of Googling, this might be a bug in certain ROMs.

Crash stats over the past 28 days shows this only reproducing on a LG Optimus G twice. It probably is device-specific. We probably should file an upstream bug as well.

For those of us who don't know about the upstream process - What needs to be done here for this patch to get upstreamed (e.g. where would I file a bug)?

Comment 6

[Gian-Carlo Pascutto [:gcp]]

>It probably is device-specific. We probably should file an upstream bug as well.

Well, your analysis sortof reduces the urgency of doing so greatly, as this looks like a bug in a single device and maybe even a single non-standard ROM :)

>For those of us who don't know about the upstream process - What needs to be done here for this patch to get upstreamed (e.g. where would I file a bug)?

For Android bugs: http://code.google.com/p/android/issues/list  [New Issue] and pray (I've seldom or never seen a reaction)

For WebRTC.org upstream: First check if upstream has the same code, i.e. if the bug isn't in a modification on our side: 
https://code.google.com/p/webrtc/source/checkout

Then file here: http://code.google.com/p/webrtc/issues/list  Upstream is generally responsive and receptive to patches.

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/b5b45b437292

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/b5b45b437292

Comment 9

[Gian-Carlo Pascutto [:gcp]]

It looks like Bug 932112 accidentally backed this out.

Comment 10

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

Looks like just the dataLengthStr in the log message; the rest appears the same.