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)
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 | ||
Updated•11 years ago
|
Assignee: nobody → dmarcos
Updated•11 years ago
|
Whiteboard: [fxos:media]
Target Milestone: --- → 1.4 S1 (14feb)
Assignee | ||
Comment 1•11 years ago
|
||
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)
Assignee | ||
Comment 2•11 years ago
|
||
Assignee | ||
Comment 3•11 years ago
|
||
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
Assignee | ||
Updated•11 years ago
|
Flags: needinfo?(mhabicher)
Assignee | ||
Updated•11 years ago
|
Flags: needinfo?(mhabicher)
Reporter | ||
Comment 4•11 years ago
|
||
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)
Assignee | ||
Comment 5•11 years ago
|
||
Attachment #8370894 -
Flags: review?(wilsonpage)
Assignee | ||
Updated•11 years ago
|
Attachment #8370894 -
Flags: review?(jdarcangelo)
Assignee | ||
Comment 6•11 years ago
|
||
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)
Assignee | ||
Comment 7•11 years ago
|
||
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
Reporter | ||
Comment 8•11 years ago
|
||
Thanks for the feedback--I'm investigating.
Flags: needinfo?(mhabicher)
Target Milestone: 1.4 S1 (14feb) → ---
Reporter | ||
Comment 9•11 years ago
|
||
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)
Assignee | ||
Comment 10•11 years ago
|
||
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
Assignee | ||
Updated•11 years ago
|
Flags: needinfo?(mhabicher)
Reporter | ||
Comment 11•11 years ago
|
||
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 12•11 years ago
|
||
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-
Updated•11 years ago
|
Target Milestone: --- → 1.4 S1 (14feb)
Reporter | ||
Comment 13•11 years ago
|
||
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.
Assignee | ||
Updated•11 years ago
|
Attachment #8370894 -
Flags: review- → review?(wilsonpage)
Flags: needinfo?(dmarcos)
Assignee | ||
Comment 14•11 years ago
|
||
Attachment #8370513 -
Attachment is obsolete: true
Attachment #8373159 -
Flags: feedback?(wilsonpage)
Assignee | ||
Comment 15•11 years ago
|
||
Attachment #8373159 -
Attachment is obsolete: true
Attachment #8373159 -
Flags: feedback?(wilsonpage)
Attachment #8373160 -
Flags: feedback?(wilsonpage)
Updated•11 years ago
|
Attachment #8370894 -
Flags: review?(wilsonpage) → review+
Assignee | ||
Comment 16•11 years ago
|
||
Comment 17•11 years ago
|
||
Bug 909542 was backed out from b2g-inbound, so I reverted this as well since my understanding is that they go together.
Master: 6a8978b847b530e038b4f819463d6c5bceabe7b9
Assignee | ||
Comment 18•11 years ago
|
||
Attachment #8373160 -
Attachment is obsolete: true
Attachment #8373160 -
Flags: feedback?(wilsonpage)
Attachment #8375234 -
Flags: review?(wilsonpage)
Comment 19•11 years ago
|
||
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-
Assignee | ||
Comment 20•11 years ago
|
||
Landed in master:
https://github.com/mozilla-b2g/gaia/commit/3d01dbcfe42d6f57c794855b8c4bcc462e4e38d1
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Comment 21•11 years ago
|
||
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 → ---
Assignee | ||
Comment 22•11 years ago
|
||
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)
Comment 23•11 years ago
|
||
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)
Assignee | ||
Comment 24•11 years ago
|
||
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)
Comment 25•11 years ago
|
||
I have nothing to do with that part of the process. You're better off talking to ahal or jgriffin.
Flags: needinfo?(ryanvm)
Comment 26•11 years ago
|
||
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.
Assignee | ||
Comment 27•11 years ago
|
||
Status: REOPENED → RESOLVED
Closed: 11 years ago → 11 years ago
Resolution: --- → FIXED
Comment 28•11 years ago
|
||
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.
Description
•