Closed Bug 958200 Opened 11 years ago Closed 11 years ago

[Camera] Update Camera app to use new CameraControl API

Categories

(Firefox OS Graveyard :: Gaia::Camera, defect)

ARM
Gonk (Firefox OS)
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
1.4 S1 (14feb)

People

(Reporter: mikeh, Assigned: dmarcos)

References

Details

(Whiteboard: [fxos:media])

Attachments

(3 files, 3 obsolete files)

The new CameraControl API has changes which will break the current version of the Camera app: 1. The preview MediaStream is now returned as part of the CameraControl object in the getCamera() onsuccess callback. This means fewer trips through JS: function getCamera_onSuccess(camera, config) { this.viewfinder.mozSrcObject = camera; // ... } 2. getPreviewStream() and getPreviewStreamVideoMode() are gone; both are replaced with a single setConfiguration() call that takes either 'picture' or 'video' as an argument. 3. getCamera() takes an additional new argument that is the same configuration argument that would be passed to setConfiguration() -- this allows a single call to get a camera with a configured preview stream that can be used right away. 4. As shown in (1) above, the getCamera() onsuccess callback gets a new (second) argument that is the configuration passed to getCamera(); the passed-back version may be modified by the platform to account for any necessary changes (e.g. if the requested preview size is not supported, the returned object will reflect the closest preview size selected by the platform). To prevent breakage, the Camera app may need to support both versions of the API for a short time. There is no API versioning (perhaps there should be?) but you should be able to query navigator.mozCameras.getCamera.length to see which version of the function is supported by the platform: 3 for the old version, 4 for the new one.
Assignee: nobody → dmarcos
Whiteboard: [fxos:media]
Target Milestone: --- → 1.4 S1 (14feb)
Got a working patch on Hamachi. Everything works as expected except for the orientation flag. I'm passing the phone orientation in degrees as part of the configuration object for the takePicture gecko API. This hasn't changed with respect to the old API: In camera js/camera.js I do: var config = { orientation: orientation.get(), dateTime: Date.now() / 1000, fileFormat: 'jpeg' }; camera.takePicture(config, onSuccess, onError); I tried to hardcode different values in the orientation attribute and didn't have any effect. All the pictures returned from the camera have an orientation of -90 degrees (90 degrees counter clockwise) This is the branch with the code I've written: https://github.com/dmarcos/gaia/tree/bug958200 This is the commit: https://github.com/dmarcos/gaia/commit/a0949dd89469c80d093a7a89306a9661ea8acd8e This is the patch: https://github.com/dmarcos/gaia/commit/a0949dd89469c80d093a7a89306a9661ea8acd8e.diff $ cd your-gaia-clone $ wget -O bug958200.patch https://github.com/dmarcos/gaia/commit/a0949dd89469c80d093a7a89306a9661ea8acd8e.diff $ git apply bug958200.patch
Flags: needinfo?(mhabicher)
Attached file Pull Request (obsolete) (deleted) —
The new API doesn't seem to select the front camera preview. Passing to the API: navigator.mozCameras.getCamera(0,{}, done); navigator.mozCameras.getCamera(1,{}, done); In both cases I'm getting back the preview of the back camera
Flags: needinfo?(mhabicher)
Flags: needinfo?(mhabicher)
Don't pass in a number, pass in the name of the camera as returned by getListOfCameras(): { "back", "front" }. If the API can't make sense of what you're passing it, it selects the first camera, almost always the "back" one.
Flags: needinfo?(mhabicher)
Attached file Pull Request (deleted) —
Attachment #8370894 - Flags: review?(wilsonpage)
Attachment #8370894 - Flags: review?(jdarcangelo)
I'm testing the front/back camera selection on the new API and I'm getting the same object no matter what parameter I pass: this.requestCameraHardware("front", configureCamera); this.requestCameraHardware("back", configureCamera); They are both returning the front camera object.
Flags: needinfo?(mhabicher)
I misunderstood the behavior of my own code. When the application starts I can select back or front camera getting the expected preview. When I toggle cameras and I request the second camera the application crashes on the call below. The configureCamera callback is never called. navigator.mozCameras.getCamera(selectedCamera, {}, configureCamera); I pushed to the PR the code I used to reproduce the problem. I'm testing on a helix device
Thanks for the feedback--I'm investigating.
Flags: needinfo?(mhabicher)
Target Milestone: 1.4 S1 (14feb) → ---
How are you building/flashing for your Helix? When I try to flash b2g-inbound and master-gaia onto my Helix, it gets into a start-up loop and the main screen never appears.
Flags: needinfo?(dmarcos)
Do you have the last partner image installed? Use the sdcard method to flash your phone. Instructions here: https://intranet.mozilla.org/B2G_Team/Helix#Flashing_via_SD_Card It also explains where to get the latest image. After that I applied your patch with the mercurial queue tool (qimport) on my checkout of b2g-inbound and build it ./config.sh helix ./build.sh gecko ./flash.sh gecko Finally I flashed gaia with the branch of this bug. Do ./make reset-gaia from your gaia checkout just to make sure that you start fresh I'm going to have something to eat and be back online shortly. I can help you later
Flags: needinfo?(mhabicher)
Unfortunately I can't seem to get gdb working on Helix: see bug 969127. With camera logging enabled, logcat shows the crash happens somewhere after the viewfinder returns its first frame: 11-01 09:26:06.249 1773 1781 I Gecko : android::CameraGraphicBuffer::CameraGraphicBuffer(android::GonkNativeWindow*, uint32_t, uint32_t, mozilla::layers::SurfaceDescriptor):316 : this=0x44c58650 11-01 09:26:06.249 1558 1827 I QCameraHWI: processPreviewChannelEvent: X 11-01 09:26:06.249 1558 1827 I QCameraHWI: processChannelEvent: X 11-01 09:26:06.249 1773 1781 I Gecko : OnNewPreviewFrame: we have 1 preview frame listener(s) 11-01 09:26:06.249 1558 1827 E QCameraHWI: App_cb Notify 0x0, datacb=0x0 11-01 09:26:06.249 1558 1827 I QCameraHWI: processEvent: X 11-01 09:26:06.249 1773 1781 I Gecko : OnNewPreviewFrame: got 1280 x 720 frame 11-01 09:26:06.249 1773 1781 I Gecko : GonkNativeWindow::returnBuffer: slot=0 (generation=1) 11-01 09:26:06.249 1773 1781 I Gecko : virtual android::CameraGraphicBuffer::~CameraGraphicBuffer():321 : this=0x44c58650 11-01 09:26:06.269 1576 1586 I Gecko : [Parent 1576] WARNING: pipe error (112): Connection reset by peer: file /home/mikeh/dev/mozilla/m-c/b2g-inbound/ipc/chromium/src/chrome/common/ipc_channel_posix.cc, line 445 11-01 09:26:06.269 1576 1576 I Gecko : 11-01 09:26:06.269 1576 1576 I Gecko : ###!!! [Parent][MessageChannel] Error: Channel error: cannot send/recv
Flags: needinfo?(mhabicher)
Comment on attachment 8370894 [details] Pull Request Coupling of camera hardware with viewfinder element seems to go against initial architectural decisions. Comments on Github.
Attachment #8370894 - Flags: review?(wilsonpage) → review-
Target Milestone: --- → 1.4 S1 (14feb)
Diego, I've noticed that when using the front (user-facing) camera, landscape photos are upside down. Make sure the new camera code includes the negative-rotation fix from bug 947956.
Attachment #8370894 - Flags: review- → review?(wilsonpage)
Flags: needinfo?(dmarcos)
Attached file WIP - Pull Request on top of settings patch (obsolete) (deleted) —
Attachment #8370513 - Attachment is obsolete: true
Attachment #8373159 - Flags: feedback?(wilsonpage)
Attached file WIP - Pull Request on top of settings patch (obsolete) (deleted) —
Attachment #8373159 - Attachment is obsolete: true
Attachment #8373159 - Flags: feedback?(wilsonpage)
Attachment #8373160 - Flags: feedback?(wilsonpage)
Attachment #8370894 - Flags: review?(wilsonpage) → review+
Bug 909542 was backed out from b2g-inbound, so I reverted this as well since my understanding is that they go together. Master: 6a8978b847b530e038b4f819463d6c5bceabe7b9
Attached file Pull Request over settings branch (deleted) —
Attachment #8373160 - Attachment is obsolete: true
Attachment #8373160 - Flags: feedback?(wilsonpage)
Attachment #8375234 - Flags: review?(wilsonpage)
Comment on attachment 8375234 [details] Pull Request over settings branch Video profile selection should the job of the settings, not camera hardware. End selection is dependant on app config, persisted settings and incoming activity parameters.
Attachment #8375234 - Flags: review?(wilsonpage) → review-
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Reverted for Gaia unit test perma-fail on TBPL. https://github.com/mozilla-b2g/gaia/commit/e86a1d8a35d68c00267edb087b230d82115dac8c https://tbpl.mozilla.org/php/getParsedLog.php?id=34669659&tree=B2g-Inbound gaia-unit-tests TEST-UNEXPECTED-FAIL | camera/test/unit/lib/sounds_test.js | Sounds Sounds#createAudio() Should set the mozAudioChannel type | expected false to be truthy gaia-unit-tests TEST-UNEXPECTED-FAIL | camera/test/unit/controllers/hud_test.js | controllers/hud "before each" hook | The constructor should be a function. TypeError: The constructor should be a function.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
This is very frustrating. What kind of environment is TBPL? The unit tests run perfectly in both my local machines and in travis builds.
Flags: needinfo?(ryanvm)
From the log: b2g_ubuntu64_vm b2g-inbound opt test gaia-unit on 2014-02-13 23:38:26 PST for push b0053c8d0836 slave: tst-linux64-spot-439 Linux64 desktop b2g build.
Flags: needinfo?(ryanvm)
How can I make sure that the unit tests work on tbpl? Why do test pass on travis and our local machines but not in tbpl? What is different about the environment? There's too many things to look at and wait for. Can we simplify the process?
Flags: needinfo?(ryanvm)
I have nothing to do with that part of the process. You're better off talking to ahal or jgriffin.
Flags: needinfo?(ryanvm)
See https://developer.mozilla.org/en-US/Firefox_OS/TBPL#Running_TBPL_Try_jobs_against_Gaia_builds for a way to test your changes on TBPL. James Lal and Release Engineering are working on a way to make this saner, but this is the compromise we have for now.
Status: REOPENED → RESOLVED
Closed: 11 years ago11 years ago
Resolution: --- → FIXED
Comment on attachment 8370894 [details] Pull Request Cancelling this re-review since the patch has already landed.
Attachment #8370894 - Flags: review?(jdarcangelo)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: