Closed
Bug 853349
Opened 12 years ago
Closed 11 years ago
[camera] open in record mode when trigger camera with record activity by video type
Categories
(Firefox OS Graveyard :: Gaia::Camera, defect)
Tracking
(blocking-b2g:leo+, b2g18+ fixed)
RESOLVED
FIXED
blocking-b2g | leo+ |
People
(Reporter: gasolin, Assigned: gasolin)
References
Details
(Whiteboard: TaipeiWW)
Attachments
(3 files)
Camera should support 'record' with 'video' type to fulfill the need of new video.app spec https://www.dropbox.com/s/h89kj0xrwygfj1n/Sharing-videos-via-bluetooth.pdf from Bug 805333 video.app will triger 'take a shot' action to camera.app. Though there's no way to turn back to video.app from camera.app. It might be need some extra UX work to clarify the transition between video.app and camera.app
Assignee | ||
Comment 1•12 years ago
|
||
Sorry spec is from bug 838028. We could check video related integration issue here
Flags: needinfo?(abc)
Assignee | ||
Comment 2•12 years ago
|
||
Another criteria might be 'when trigger camera with video type, open record mode instead of default shot mode'
Comment 3•12 years ago
|
||
re: comment 2 Yes, it's a good idea, but we just need to make sure that if we take this route then it would still work for other scenarios that are similar — it's a tricky UX problem to solve. Is there a meta bug that tracks all places where such an interaction between apps that exists? Cheers, Arun
Flags: needinfo?(abc)
Assignee | ||
Updated•12 years ago
|
blocking-b2g: --- → leo?
Summary: [camera] support 'record' activity with 'video' type → [camera] open in record mode when trigger camera with video app
Comment 4•12 years ago
|
||
This is a nice to have. Please only work on this after other blockers have been resolved, and nominate for uplift when resolved.
blocking-b2g: leo? → -
tracking-b2g18:
--- → +
Assignee | ||
Comment 5•11 years ago
|
||
make title more specific
Summary: [camera] open in record mode when trigger camera with video app → [camera] open in record mode when trigger camera with record activity by video type
Comment 6•11 years ago
|
||
Its confusing that https://bugzilla.mozilla.org/show_bug.cgi?id=879881 is leo+ and this isnt
Updated•11 years ago
|
Assignee: nobody → dale
Comment 8•11 years ago
|
||
There is a few notes for this commit, 1 apologies for the amss change from this.captureMode to this._captureMode, it was a bug that has always existed however it never previously caused problems as we always manually set this.captureMode. 1. When we record a video, we get a black screen with the confirmation to retake or select the image, I though this would be less confusing than a live preview screen (and pausing currently doesnt work), Ideally this should be a preview of the video that can be played 2. The duration of the current duration isnt shown as it is in the same place as the cancel button, we can either hide the cancel button during recording or move the timer to the right Both things should be fixed, but I though given the time contraints and size of change they should be done in follow ups
Comment 9•11 years ago
|
||
Comment on attachment 759576 [details]
Pointer to GH
The code looks good, but I can't figure out how to test it.
When I add an attachment in the Messaging app, it seems to be doing a pick activity with a wildcard for the type, so all apps that can handle picks are listed in the menu. When I select the camera app, the type is not "video/*" so the camera app lets me pick a still photo, but doesn't let me pick a video.
I fear that in order to support MMS, the camera app will have to support three kinds of picks:
1) still photos only (we already had that)
2) videos only (what this patch gives)
3) either one, displaying the switch button so the user can choose.
If I'm misunderstanding, just explain and set r? again.
Attachment #759576 -
Flags: review?(dflanagan) → review-
Assignee | ||
Comment 10•11 years ago
|
||
nominate since this issue block bug 868225 and bug 879881 (both leo+)
blocking-b2g: - → leo?
Updated•11 years ago
|
blocking-b2g: leo? → leo+
Updated•11 years ago
|
Comment 12•11 years ago
|
||
I was testing by manually changing the wallpaper to ask for video/*, but you are right there is no way to capture 'video/*' 'images/*' right now, I think adding the ability for the camera to switch between modes when the activity is asking for the particular type of media shouldnt be too intrusive a patch, will come up with one now
Assignee | ||
Comment 13•11 years ago
|
||
Dale, sorry I just saw you just start working on the following work. hope it not late... I'm working on bug 868225 (video attachment support on MMS), so I followed your work yesterday and come out a patch. Your previous patch did most great effort, but only support 'pick' activity, which not fulfill the origin purpose of this bug. This patch works for * support 'record' activity (open video app -> press record button in left bottom side) * displaying the switch button while multiple types are declared. I'm working based on your patch. If it works you can r? to djf. thanks
Attachment #760727 -
Flags: review?(dale)
Comment 14•11 years ago
|
||
Comment on attachment 760727 [details]
pull request redirect to github
Hey Fred, no worries on taking over, so there is a few issues with this.
I will needinfo David for this, but currently we are using the 'record' activity to trigger the plain camera, and 'pick' activity to pick media from the camera to be returned
This currently breaks the normally launched camera, the capture button doesnt get enabled
The code to pick the types is confusing, there is lots of branching etc, hopefully sorting out the pick vs record will help, I also put a quick code example up there that I think makes it a little clearer.
David mentioned to me recently that the addVideo call to the filmstrip currently contains the code that generated the preview for the video that the gallery uses, so that code needs to be extracted / invoked
Attachment #760727 -
Flags: review?(dale) → review-
Comment 15•11 years ago
|
||
Actually I dont really need to needinfo david, reusing the 'record' activity name will break existing usage (launch from gallery), the SMS app has to use the 'pick' activity
Flags: needinfo?(dflanagan)
Assignee | ||
Comment 17•11 years ago
|
||
Comment on attachment 760727 [details]
pull request redirect to github
Thanks dale, I've adopt your code example,
and the capture button doesn't get enabled issue is fixed now.
find some issues about video preview and video,
I think its not related to this issue statement, and can be done in follow ups.
Attachment #760727 -
Flags: review?(dale)
Comment 18•11 years ago
|
||
Comment on attachment 760727 [details] pull request redirect to github The r? seemed to be on the wrong user so clearing that, leaving my r- for now as I would like another look at this with some ammendments before it lands, but almost there. 1. The cancel button gets disabled when switching between video / camera 2. I mentioned previously that https://github.com/mozilla-b2g/gaia/blob/master/apps/camera/js/filmstrip.js#L617 is required for saved videos to be displayed in the gallery, it isnt a major issue but probably best fixed now (while we remember) Few other comments on GH, cheers
Attachment #760727 -
Flags: review?(dale)
Updated•11 years ago
|
Assignee: dale → gasolin
Comment 19•11 years ago
|
||
Also last comment, the branch needs rebased, it hits against a few merge conflicts, cheers
Assignee | ||
Comment 20•11 years ago
|
||
I feel that summarizing some test criteria will help goes through the following work: record open video app -> press record button in left bottom side -> camera opened in record mode pick 1) photos only open homescreen > long click to trigger webactivity menu -> camera opened in camera mode 2) videos only manual modify some app(ex video) to test this action. 3) either one, displaying the switch button so the user can choose. open message app > click left bottom paper clip sign > select camera -> open in camera mode, can switch back and forth between camera/record mode, always can see the cancel pick button.
Assignee | ||
Comment 21•11 years ago
|
||
1. passed test criteria in comment 20 2. append blob when return pick result (thus message app will show the compose view with file size). I tried to call Filmstrip.addVideo(videofile) in stop recording function, but doing that will start preview mode, which not the expect behavior. So I'd leave it black in this stage.
Attachment #762593 -
Flags: review?(dflanagan)
Comment 22•11 years ago
|
||
Fred, Please also try this test case: - Open message app, attach a video to a message, then go to the gallery app and see if the video is displayed properly. Without calling Filmstrip.addVideo() for each new video, the poster image that gallery depends on will not be written. If you can't call that (and just hide the filmstrip in pick mode) then you'll need to refactor so that the poster image is generated in camera.js I haven't had a chance to read the code enough for this to be a review but I thought I'd at least mention this part.
Flags: needinfo?(gasolin)
Assignee | ||
Comment 23•11 years ago
|
||
Comment on attachment 762593 [details]
2nd pull request redirect to github
Thanks for helping tackle down the gallery-related test case,
the poster image is not written without calling Filmstrip.addVideo().
I'd try to fix it then request review again.
Attachment #762593 -
Flags: review?(dflanagan)
Assignee | ||
Updated•11 years ago
|
Flags: needinfo?(gasolin)
Assignee | ||
Comment 24•11 years ago
|
||
Comment on attachment 762593 [details]
2nd pull request redirect to github
will add thumb to gallery, and not trigger preview while in pick mode
Attachment #762593 -
Flags: review?(dflanagan)
Comment 25•11 years ago
|
||
Comment on attachment 762593 [details]
2nd pull request redirect to github
r- see github for details.
Also, you have not modified the webapp.manifest file to list video/* and video/3gpp as supported types for the activity. Without that, the activity won't work for apps that want to pick video but not still images.
You may need to create a UITest pane as part of this patch to demonstrate that the app can work in all possible cases:
- photos only
- videos only
- photos and videos, both specified
- no types specified, allow both (?)
Attachment #762593 -
Flags: review?(dflanagan) → review-
Assignee | ||
Comment 26•11 years ago
|
||
Comment on attachment 762593 [details]
2nd pull request redirect to github
Thanks for review, fixed comment in github and add UITests for pick video, pick photo/video cases.
Works fine with those uitest.
Attachment #762593 -
Flags: review?(dflanagan)
Updated•11 years ago
|
Whiteboard: TaipeiWW
Assignee | ||
Comment 27•11 years ago
|
||
Comment on attachment 762593 [details]
2nd pull request redirect to github
squashed commits and ask dale for review
Attachment #762593 -
Flags: review?(dflanagan) → review?(dale)
Comment 28•11 years ago
|
||
Comment on attachment 762593 [details] 2nd pull request redirect to github Thanks, tested this and is working well Merged in: https://github.com/mozilla-b2g/gaia/commit/baea36ed4e1034af3b58e815d515b0415d80a2ce
Attachment #762593 -
Flags: review?(dale) → review+
Updated•11 years ago
|
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Comment 29•11 years ago
|
||
Uplifted to v1-train: https://github.com/mozilla-b2g/gaia/commit/2dfdafa42e4e8f9b978c85100582ed628d56cb91
Updated•11 years ago
|
status-b2g18:
--- → fixed
You need to log in
before you can comment on or make changes to this bug.
Description
•