[Camera] Camera launched from MMS app limits file size but still uses '480p' recording profile

VERIFIED FIXED in Firefox OS v2.2

Status

VERIFIED FIXED
4 years ago
3 years ago

People

(Reporter: mikeh, Assigned: aosmond)

Tracking

unspecified
2.2 S2 (19dec)
ARM
Gonk (Firefox OS)

Firefox Tracking Flags

(blocking-b2g:2.2+, b2g-v2.1 unaffected, b2g-v2.2 verified)

Details

Attachments

(2 attachments, 3 obsolete attachments)

I noticed this while investigating bug 1098028.

STR:
1. open the Messages app
2. tap on the pencil to compose a new message
3. tap on the paperclip to add an attachment
4 [review]. in the activities list, tap on Camera
5. switch to Video mode

Expected: the camera should pick the lowest resolution profile we support (on Flame, 'qcif') to record the video.

Observed: the camera picks the highest resolution profile, '480p'.

With extra logging in Gecko, logcat on the mode change to video shows:

11-13 19:47:28.060  3454  3481 I PRLog   : 28083520[b203a480]: [mikeh] aConfig.mMode=2
11-13 19:47:28.060  3454  3481 I PRLog   : 28083520[b203a480]: [mikeh] aConfig.mRecorderProfile='480p'
11-13 19:47:28.060  3454  3481 I PRLog   : 28083520[b203a480]: [mikeh] aConfig.mPreviewSize=864x480

(Here, mMode=2 means 'video' mode.)

With extra logging in Gaia (controllers/settings.js):

SettingsController.prototype.configureRecorderProfiles = function(sizes) {
  var setting = this.settings.recorderProfiles;
  var maxFileSize = setting.get('maxFileSizeBytes');
  var exclude = setting.get('exclude');
  var options = { exclude: exclude };
  var formatted = this.formatRecorderProfiles(sizes, options);

  // If a file size limit has been imposed,
  // pick the lowest-res (last) profile only.
  if (maxFileSize) {
    dump("[mikeh] maxFileSize=" + maxFileSize);
    formatted = [formatted[formatted.length - 1]];
    dump("[mikeh] formatted.length=" + formatted.length);
  } else {
    dump("[mikeh] !maxFileSize");
  }

  setting.resetOptions(formatted);
};

...logcat on the mode change shows:

11-13 19:47:25.430  3454  3454 I GeckoDump: [mikeh] !maxFileSize

What's interesting is that the Gecko logcat output also shows:

11-13 19:47:31.430  3454  3481 I PRLog   : 28083520[b203a480]: maxFileSizeBytes=302080
11-13 19:47:31.430  3454  3481 I PRLog   : 28083520[b203a480]: [../../../../../m-c/b2g-inbound-2/dom/camera/GonkRecorder.cpp:618]setParameters: max-filesize=302080
11-13 19:47:31.430  3454  3481 I PRLog   : 28083520[b203a480]: [../../../../../m-c/b2g-inbound-2/dom/camera/GonkRecorder.cpp:515]setParameter: key (max-filesize) => value (302080)
11-13 19:47:31.430  3454  3481 I PRLog   : 28083520[b203a480]: [../../../../../m-c/b2g-inbound-2/dom/camera/GonkRecorder.cpp:358]setParamMaxFileSizeBytes: 302080 bytes

...so the file-size limit is getting pushed to the recorder. (Accordingly, the video recording terminates about ~2 seconds at 480p. :)
Interestingly, after hitting the above error, if from the Retake/Select screen, I return to the Homescreen, then reopen the Messages app and tape Retake, maxFileSize is applied correctly:

11-13 19:50:06.280  3454  3454 I GeckoDump: [mikeh] maxFileSize=302080
11-13 19:50:06.280  3454  3454 I GeckoDump: [mikeh] formatted.length=1
  ...
11-13 19:50:06.760  3454  3663 I PRLog   : 28082936[b0e76680]: [mikeh] aConfig.mMode=2
11-13 19:50:06.760  3454  3663 I PRLog   : 28082936[b0e76680]: [mikeh] aConfig.mRecorderProfile='low'
11-13 19:50:06.760  3454  3663 I PRLog   : 28082936[b0e76680]: [mikeh] aConfig.mPreviewSize=864x480

...and I can record a ~15 second video without problem.
I suspect this is a regression because I'm pretty sure we had this working right before.

Setting qa wanted: does this bug reproduce in 2.1? If so we might want to get it fixed there ASAP.
Keywords: qawanted

Updated

4 years ago
blocking-b2g: 2.2? → 2.2+
Mike, how do you get the extra logging in Gecko?  I wasn't able to find it in the Developer menu.
Flags: needinfo?(mhabicher)
Chris, you need to modify /system/bin/b2g.sh on the DUT to add the following line somewhere before the line that actually launches b2g:

export NSPR_LOG_MODULES=Camera:4

...then reboot. When the system restarts, you'll have extra camera logging enabled. (I haven't had a chance to add it to the Developer menu yet.)

You might need to run |adb root| and/or |adb remount| before editing b2g.sh.
Flags: needinfo?(mhabicher)
Mike adding the requested line to the b2g.sh prevents the phone from booting up.  Is there more that needs to be added or changed in order to handle the extra export?
Flags: needinfo?(mhabicher)
Jayme, please attach your modified b2g.sh to this bug and I'll take a look.
Flags: needinfo?(mhabicher) → needinfo?(jmercado)
Created attachment 8526193 [details]
modified b2g.sh

Here it is, pulled from the devices on the 2.1 build we were using, modified to add the requested line, and pushed back to the phone.
Flags: needinfo?(jmercado)
Created attachment 8526262 [details]
modified b2g.sh

Jayme, this shouldn't make any difference, but please try this version.

If that doesn't work either (it works here), can you please attach the logcat output (let it run for a minute or so) from the DUT?
Attachment #8526193 - Attachment is obsolete: true
Attachment #8526262 - Flags: feedback?(jmercado)
Created attachment 8526291 [details]
wont_load_logcat.txt

I've uploaded a logcat obtained on trying to load up the phone after pushing your file to the phone.
Attachment #8526262 - Flags: feedback?(jmercado) → feedback-
Jayme, you need to make sure the version of b2g.sh you push to your phone is executable.

# adb push b2g.sh /system/bin/b2g.sh
# adb shell chmod 0775 /system/bin/b2g.sh

Otherwise, b2g won't start.
Flags: needinfo?(jmercado)
Mike I had the same problem after using your commands to push your b2g.sh and make it executable.
Flags: needinfo?(jmercado)
Hi Mike - we are still waiting on additional feedback from you on getting the additional logging working to check 2.1 (and satisfy the QA-Wanted Request)
Flags: needinfo?(mhabicher)
(Assignee)

Comment 13

4 years ago
Created attachment 8537336 [details] [review]
Gaia pull request, v1 -- master

Clear ni? since I don't think we need the logcat anymore. This was likely a regression due to making camera startup faster (event orders changed) but was always theoretically possible.

Thoughts? It is sort of messy but I'm not sure how else to do it without changing how settings works. Is it possible to ensure we get the activity event earlier at application start instead of after newcamera?
Assignee: nobody → aosmond
Attachment #8526262 - Attachment is obsolete: true
Attachment #8526291 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Flags: needinfo?(mhabicher)
Attachment #8537336 - Flags: review?(jdarcangelo)
striking qa-wanted keyword based on comment 13 "Clear ni? since I don't think we need the logcat anymore."
Keywords: qawanted
(Assignee)

Updated

4 years ago
Target Milestone: --- → 2.2 S2 (19dec)
(Assignee)

Updated

4 years ago
status-b2g-v2.1: ? → unaffected
Comment on attachment 8537336 [details] [review]
Gaia pull request, v1 -- master

Tested on Flame. LGTM!
Attachment #8537336 - Flags: review?(jdarcangelo) → review+
(Assignee)

Updated

4 years ago
Keywords: checkin-needed
(Assignee)

Comment 16

4 years ago
Wait there is a lint issue I need to fix first ;).
Keywords: checkin-needed
(Assignee)

Updated

4 years ago
Keywords: checkin-needed
Master: https://github.com/mozilla-b2g/gaia/commit/1ab5b205c79551cee576ed2e915a7b57ccfa705b
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
status-b2g-v2.2: affected → fixed
Keywords: checkin-needed
Resolution: --- → FIXED
Created attachment 8631472 [details]
Flame_v2.2.3gp

This bug has been verified as pass on latest build of Flame v2.2 by the STR in Comment 0.

Actual results: The camera pick the lowest resolution profile,'176x144'.

See attachment:Flame_v2.2.3gp

Reproduce rate: 0/10

Device: Flame 2.2 build(Pass)
Build ID               20150708162504
Gaia Revision          d6d6a35b9407ffaf3131bc244cabc8ee2b09c4df
Gaia Date              2015-07-08 19:35:16
Gecko Revision         https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/629912eb8659
Gecko Version          37.0
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150708.195227
Firmware Date          Wed Jul  8 19:52:38 EDT 2015
Bootloader             L1TC000118D0
Status: RESOLVED → VERIFIED
QA Whiteboard: [MGSEI-Triage+]
status-b2g-v2.2: fixed → verified
You need to log in before you can comment on or make changes to this bug.