Closed Bug 782456 Opened 13 years ago Closed 12 years ago

Camera - Can't initialize HAL from content process

Categories

(Firefox OS Graveyard :: General, defect)

Product:
Firefox OS Graveyard
Component:
General
Platform:
x86_64
Linux
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(blocking-kilimanjaro:+, blocking-basecamp:+)

Status:
RESOLVED FIXED
Project Flags:
blocking-kilimanjaro +
blocking-basecamp +

People

(Reporter: dhylands, Assigned: cjones)

References

Details

(Whiteboard: [LOE:S])

Attachments

(4 files)

part 0: Add the cross-process ImageBridge glue
16.06 KB, patch
roc
: review+
Details | Diff | Splinter Review
part 1: Let clients of GeckoChildProcessHost specify privileges
7.79 KB, patch
bent.mozilla
: review+
Details | Diff | Splinter Review
part 2: Inherit privileges in apps that have permissions that require them. Sigh.
7.19 KB, patch
justin.lebar+bug
: review+
Details | Diff | Splinter Review
Fix process_util_bsd.cc bustage
2.65 KB, patch
Details | Diff | Splinter Review
When launching the camera app OOP, I see the controls at the bottom, but the preview never shows up (rest of screen stays black). I did notice the following on the console: [JavaScript Error: "TypeError: camera.capabilities.focusModes is null" {file: "app://camera.b2g.hylands.org/js/camera.js" line: 204}]
That error might be aborting the script and preventing the preview from starting. That said, 'focusModes' should not be null, so there's probably something up with OOP execution.
blocking-basecamp: ? → +
Assignee: nobody → mhabicher
It looks like we're failing to initialize the camera HAL entirely from content processes. This is a good thing security-wise, but I really wish we'd known about this sooner :( ...
Blocks: b2g-e10s-work
Summary: Camera app show black screen when launched OOP → Can't initialize camera HAL from content process
(In reply to Chris Jones [:cjones] [:warhammer] from comment #2) > It looks like we're failing to initialize the camera HAL entirely from > content processes. Can you grab/attach a logcat?
Status: NEW → ASSIGNED
Summary: Can't initialize camera HAL from content process → Camera - Can't initialize HAL from content process
adb logcat: "controlfd is invalid Permission denied" E/QualcommCamera( 383): Qint android::get_number_of_cameras(): E I/QualcommCameraHardware( 383): getCameraInfo: IN I/QualcommCameraHardware( 383): getCameraInfo: loading libqcamera at 0xb000db90 V/QualcommCameraHardware( 383): Storing the current target type as 3 E/mm-camera( 383): mm_camera_get_camera_info: controlfd is invalid Permission denied I/QualcommCameraHardware( 383): getCameraInfo: numOfCameras = 0 V/QualcommCameraHardware( 383): getCameraInfo: dlclose(libqcamera) I/QualcommCameraHardware( 383): getCameraInfo: OUT I/Gecko ( 383): getListOfCameras : get_number_of_cameras() returned 0 I/Gecko ( 383): mozilla::nsDOMCameraControl::nsDOMCameraControl(PRUint32, nsIThread*, nsICameraGetCameraCallback*, nsICameraErrorCallback*):109 : this=0x171b6e0 I/Gecko ( 383): mozilla::CameraControlImpl::CameraControlImpl(PRUint32, nsIThread*):65 : this=0x171b6f8 I/Gecko ( 383): mozilla::nsGonkCameraControl::nsGonkCameraControl(PRUint32, nsIThread*, mozilla::nsDOMCameraControl*, nsICameraGetCameraCallback*, nsICameraErrorCallback*):172 I/Gecko ( 383): InitGonkCameraControl::InitGonkCameraControl(mozilla::nsGonkCameraControl*, mozilla::nsDOMCameraControl*, nsICameraGetCameraCallback*, nsICameraErrorCallback*):138 : this=0x171b7f8 I/Gecko ( 383): ++++++++++++++++++++++++++++++++++++++++ I/Gecko ( 383): /home/mikeh/dev/mozilla/btg012/gecko/dom/camera/DOMCameraControl.cpp:38 : CAMREF-ADD(nsDOMCameraControl): this=0x171b6e0, mRefCnt=1 I/Gecko ( 383): /home/mikeh/dev/mozilla/btg012/gecko/dom/camera/DOMCameraControl.cpp:38 : CAMREF-ADD(nsDOMCameraControl): this=0x171b6e0, mRefCnt=2 I/Gecko ( 383): /home/mikeh/dev/mozilla/btg012/gecko/dom/camera/DOMCameraControl.cpp:38 : CAMREF-ADD(nsDOMCameraControl): this=0x171b6e0, mRefCnt=3 I/Gecko ( 383): /home/mikeh/dev/mozilla/btg012/gecko/dom/camera/DOMCameraControl.cpp:39 : CAMREF-REL(nsDOMCameraControl): this=0x171b6e0, mRefCnt=2 I/Gecko ( 383): /home/mikeh/dev/mozilla/btg012/gecko/dom/camera/DOMCameraControl.cpp:39 : CAMREF-REL(nsDOMCameraControl): this=0x171b6e0, mRefCnt=1 I/Gecko ( 383): static void mozilla::GonkCameraHardware::ReleaseHandle(PRUint32): aHwHandle = 0, hw = 0x0 (sHwHandle = 0) I/Gecko ( 383): mozilla::GonkCameraHardware::GonkCameraHardware(mozilla::GonkCamera*, PRUint32): this = 0x17b3fd0 (aTarget = 0x171b6f8) I/Gecko ( 383): void mozilla::GonkCameraHardware::Init(): this = 0x17b3fd0 I/ ( 383): Opening camera 0 E/QualcommCamera( 383): Qint android::camera_device_open(const hw_module_t*, const char*, hw_device_t**): E I/QualcommCameraHardware( 383): openCameraHardware: call createInstance E/QualcommCameraHardware( 383): openCameraHardware:Invalid camera ID 0 E/ ( 383): Could not open camera 0: -1 I/ ( 383): Destroying camera 0 I/Gecko ( 383): failed to initialize camera hardware I/Gecko ( 383): mozilla::GonkCameraHardware::~GonkCameraHardware():163 : this = 0x17b3fd0 I/Gecko ( 383): Initializing camera 0 (this = 0x171b6f8, mHwHandle = 0) I/Gecko ( 383): Pulling camera parameters I/Gecko ( 383): Camera preview formats: (null) I/Gecko ( 383): Pulling camera parameters I/Gecko ( 383): - minimum exposure compensation: -1.000000 I/Gecko ( 383): - exposure compensation step: -1.000000 I/Gecko ( 383): - maximum metering areas: -1 I/Gecko ( 383): - maximum focus areas: -1 I/Gecko ( 383): virtual InitGonkCameraControl::~InitGonkCameraControl():143 : this=0x171b7f8 I/Gecko ( 383): virtual nsresult GetCameraResult::Run() : this=0x17b80b8 -- BEFORE CALLBACK I/Gecko ( 383): virtual nsresult GetCameraResult::Run() : this=0x17b80b8 -- AFTER CALLBACK I/Gecko ( 383): /home/mikeh/dev/mozilla/btg012/gecko/dom/camera/DOMCameraControl.cpp:39 : CAMREF-REL(nsDOMCameraControl): this=0x171b6e0, mRefCnt=0 I/Gecko ( 383): ---------------------------------------- I/Gecko ( 383): virtual mozilla::nsGonkCameraControl::~nsGonkCameraControl():232 : this = 0x171b6f8, mHwHandle = 0 I/Gecko ( 383): static void mozilla::GonkCameraHardware::ReleaseHandle(PRUint32): aHwHandle = 0, hw = 0x0 (sHwHandle = 0) I/Gecko ( 383): virtual mozilla::nsGonkCameraControl::~nsGonkCameraControl():240
This is because the library is failing to open '/dev/msm_camera/control0'. / drwxr-xr-x root root 2012-08-21 11:38 dev /dev drwxr-xr-x root root 2012-08-21 11:38 msm_camera /dev/msm_camera crw-rw---- system camera 244, 0 2012-08-21 11:38 control0
Since the camera app is a relatively small amount of trusted code, and fixing this bug will require some IPC gymnastics, I would not hold release on OOP'ing this. However, the camera HAL is likely to be one of the most unstable, and any bugs there will take down the entire OS. So I *really* want to fix this.
blocking-basecamp: + → -
blocking-kilimanjaro: --- → +
I can confirm that the HAL loads when the content process is run as root (sigh). However, I see a crash shortly thereafter. I think that's an acceptable tradeoff for v1. It means that access to the raw camera stream must be a certified permission. I believe that was already the case, right?
Assignee: mhabicher → jones.chris.g
The intent was that camera streams would be exposed to privileged apps as well [1]. I believe this is has been a product requirement since Chris Lee wanted to allow 3rd party apps to be able to do camera interaction without switching out from the app. [1] https://docs.google.com/a/sicking.cc/spreadsheet/ccc?key=0Akyz_Bqjgf5pdENVekxYRjBTX0dCXzItMnRyUU1RQ0E#gid=0
blocking-basecamp: - → ?
Whiteboard: [blocked-on-input Chris Lee]
An alternative solution would be to allow opening the camera app in an <iframe>. But that doesn't allow instagram-style apps since they display a live preview while filters are applied, but that might be an ok tradeoff. We still would have to make some changes to the permission model to make that work though, and possibly some changes to the <iframe mozapp> implementation.
My understanding was that Instagram specifically didn't do live filtering. I haven't heard other product requirements around live stream, will let clee comment on that. We can't do nested content processes so <app camera> nested in another app doesn't work. That's harder than the "real" fix for this bug, remoting the camera stream.
The compromise/workaround here is very simple. The "correct" fix is considerably more work.
Whiteboard: [blocked-on-input Chris Lee] → [LOE:S][blocked-on-input Chris Lee]
The crash here is caused by allocator mismatch. Apparently we've enabled jemalloc in the meantime, but it's broken in content processes ...
The plugin-container process crashes much later with --disable-jemalloc, but something is causing a phone reboot when it crashes with gdb attached. Sigh.
Chris Lee: The question here is if taking pictures in-app is a product requirement. I.e. is it required that 3rd party applications can get access to the camera stream and display an in-app preview of what the camera view finder is seeing and then grab a picture from the camera hardware when the user wants. Or is it ok if whenever an app wants to get a picture, we'd switch to the camera app, the camera app would display all the preview-UI and take care of taking the picture. The picture would then be delivered back to the 3rd party app and we'd close the camera app and switch back to the 3rd party app. Doing the former is likely [LOE:L]. The latter is [LOE:S] Chris Jones: feel free to adjust my above estimates, or the problem description.
Whiteboard: [LOE:S][blocked-on-input Chris Lee] → [LOE:S][blocked-on-input Chris Lee, see comment 14]
That is accurate.
I've fully fixed this bug, but I'm not sure I'll be able to get reviewable patches up tonight.
We forgot to enable this in content processes. This is all the code needed to make it work there.
Attachment #655348 - Flags: review?(roc)
Whiteboard: [LOE:S][blocked-on-input Chris Lee, see comment 14] → [LOE:S]
This gets camera preview working on the Nexus S, but since it's broken in-process on otoro I can't test there.
Attachment #655349 - Flags: review?(bent.mozilla) → review+
Comment on attachment 655350 [details] [diff] [review] part 2: Inherit privileges in apps that have permissions that require them. Sigh. Ooooh, this is just about inheriting OS-level privileges, not Gecko-level privileges? Can you please comment that somewhere -- perhaps in a few places? That took me a few readings of this patch to understand. In fact, maybe we should just call all of these things "OS privileges". (ChildOSPrivileges, AppNeedsInheritedOSPrivileges, etc.) Sucks that we can't pre-load the camera app. You can't change the privileges after the process is created?
Attachment #655350 - Flags: review?(justin.lebar+bug) → review+
You could change the permissions after the fact if you prelaunched as root. Then you can change permissions downwards for apps that don't need them and leave them for apps that do. There is no way to change permissions upwards.
Yep, comment 22 is what I had in mind ... for a followup ;). Needs care.
Kanru, did bug 781892 change the camera frame queue size? I see very different performance characteristics for in-process vs. OOP camera. In-process stays smooth, but OOP is consistently very smooth for a short period of time then stalls; then is smooth again, then stalls, and so on. It almost looks like the queue size is different.
Depends on: 781892
(Should add, wasn't seeing any difference in behavior on Nexus S debug builds between in-/OOP before bug 781892.)
(In reply to Justin Lebar [:jlebar] from comment #21) > Comment on attachment 655350 [details] [diff] [review] > part 2: Inherit privileges in apps that have permissions that require them. > Sigh. > > Ooooh, this is just about inheriting OS-level privileges, not Gecko-level > privileges? > > Can you please comment that somewhere -- perhaps in a few places? That took > me a few readings of this patch to understand. In fact, maybe we should > just call all of these things "OS privileges". (ChildOSPrivileges, > AppNeedsInheritedOSPrivileges, etc.) > I changed all the occurrences in this patch. I'm going to go ahead and land this since camera is still blacklisted for OOP in gaia.
(In reply to Chris Jones [:cjones] [:warhammer] from comment #24) > Kanru, did bug 781892 change the camera frame queue size? I see very > different performance characteristics for in-process vs. OOP camera. > In-process stays smooth, but OOP is consistently very smooth for a short > period of time then stalls; then is smooth again, then stalls, and so on. > It almost looks like the queue size is different. The queue size wasn't changed but I also see the smooth-stall cycle after run camera in OOP.
Same smooth-stall cycle when OOP on otoro without patch from bug 781892
(In reply to Kan-Ru Chen [:kanru] from comment #28) > Same smooth-stall cycle when OOP on otoro without patch from bug 781892 Interesting. I wasn't able to get preview much at all without that.
Waiting on Chris Lee for a decisions on whether the camera app can run in process or if it needs to run in its own process.
Whiteboard: [LOE:S] → [LOE:M][blocked-on-input Chris Lee]
The goal of this bug is to move the current camera app OOP, for stability reasons (it's a memory hog and the camera HALs are crashy). The product decision about third-party apps getting raw streams has moved to bug 785592. clee and I spoke privately yesterday, will let him update that bug.
Whiteboard: [LOE:M][blocked-on-input Chris Lee] → [LOE:S]
Funnily, that broke bsd builds, since process_util_bsd.cc still uses SAME_PRIVILEGES_AS_PARENT which is now not defined anymore in process_util.h. What should it use instead ? PRIVILEGES_INHERIT ?
tentative fix, builds here.
Attachment #656611 - Flags: review?(jones.chris.g)
cjones says "borderline blocker". I'll weigh in and tip the scale to blocker.
blocking-basecamp: ? → +
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Whiteboard: [LOE:S] → [LOE:S][leave open for comment #35]
Please file new bugs for new issues, like bug 787279.
Status: REOPENED → RESOLVED
Closed: 12 years ago → 12 years ago
Depends on: 787279
Resolution: --- → FIXED
Comment on attachment 656611 [details] [diff] [review] Fix process_util_bsd.cc bustage r+'d in bug 787279.
Attachment #656611 - Flags: review?(jones.chris.g)
Whiteboard: [LOE:S][leave open for comment #35] → [LOE:S]
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: