1050011 - For Flame builds, we should default to 480p for video recording

Status: RESOLVED WONTFIX

(Release Engineering :: General, defect)

Description

[Jason Smith [:jsmith]]

The QRD reference device relies primarily on 480p video recording. As such, I think it would be a good idea that our reference testing on Flame matches this specification, so let's change the default for Flame to record videos at 480p.

Comment 1

[Jason Smith [:jsmith]]

We really need this implemented to ensure that we're testing under the same environment that the QRD does.

Hema - Can someone on your team make the config change here?

Comment 2

[Diego Marcos [:dmarcos]]

OEM should adapt to our reference devices and not the other way around. We should not target specific devices from the application code. Camera always selects the highest resolution available for video and pictures. B2G should properly handle it as is the case for all production devices. 

If emulating QRD with flame is something we need now we can produce flame builds with a custom configuration profile for camera. Instructions here:

https://github.com/mozilla-b2g/gaia/tree/master/apps/camera#camera-configuration

You have to replace the following line:

exclude: ['high', '1080p']

With this one:

exclude: ['high', '1080p', '720p']

Comment 3

[Tony Chung [:tchung]]

(In reply to Diego Marcos [:dmarcos] from comment #2)
> OEM should adapt to our reference devices and not the other way around. We
> should not target specific devices from the application code. Camera always
> selects the highest resolution available for video and pictures. B2G should
> properly handle it as is the case for all production devices. 

Right, while i agree, the specs for 2.0 does call for 256mb at 480p.   and given we can't get our reference flame to 256mb, the next best thing is to tune it closest to what 480p can handle, which according to https://bugzilla.mozilla.org/show_bug.cgi?id=1027592#c70, that range is between 279 >x>283mb.  So we're wanting to try this out.
> 
> If emulating QRD with flame is something we need now we can produce flame
> builds with a custom configuration profile for camera. Instructions here:
> 
> https://github.com/mozilla-b2g/gaia/tree/master/apps/camera#camera-
> configuration
> 
> You have to replace the following line:
> 
> exclude: ['high', '1080p']
> 
> With this one:
> 
> exclude: ['high', '1080p', '720p']

Thanks Diego!

Comment 4

[Jason Smith [:jsmith]]

Nick - Could someone from rel eng look into this? We would like to change the default video recording to 480p for Flame devices, which requires us to change the Flame build configuration to use a customization to specify excluding of 1080p and 720p.

Comment 5

[Nick Thomas [:nthomas] (UTC+12)]

We're currently using './build.sh' (after config.sh and source pull), and it's not trivial or desirable to adjust the code after checking it out. Would it be possible to land the configuration change on the gaia 2.0 branch ? Or add an #ifdef to gaia/apps/camera/js/config/config.js so that we can set an environment variable in the ci builds ?

Comment 6

[Jason Smith [:jsmith]]

Clarified in IRC to Nick that we'll be wanting to use a customization here to set 480p as the default for Flame builds.

Diego - Nick is looking to get a sample customization demonstrating setting the default to 480p, as that will help him figure out what changes need to be on the rel eng side to match the customization needed here. Can you provide a sample customization demonstrating the 480p video recording default setting?

Comment 7

[Nick Thomas [:nthomas] (UTC+12)]

It would also be helpful to confirm if we can do
    ./build.sh GAIA_DISTRIBUTION_DIR=<somedir>  
where somedir only has a camera-config.js file, and that leaves other apps alone.

Comment 8

[Diego Marcos [:dmarcos]]

I attached to the bug a configuration file that excludes 720p recording profile: 

exclude: ['high', '1080p', '720p']

The custom profile has to be named camera-config.js

To build gaia with the configuration profile you have to do:

make GAIA_DISTRIBUTION_DIR=<somedir>

Comment 9

[Diego Marcos [:dmarcos]]

Attachment: camera-config.js

Custom configuration file that excludes 720p recording profile

Comment 10

[Jason Smith [:jsmith]]

That appears to work. I modified by customization here:

https://github.com/mozilla/qa-testcase-data/tree/gh-pages/customization/reference

To include that file & I managed to record a video on 319 MB via email attachment.

Nick - This means that when we build Gaia, we need to build with a folder that contains the camera-config.js file that Diego has included. Does that make sense?

Comment 11

[Nick Thomas [:nthomas] (UTC+12)]

Yes it does. We'll have to do some test builds in staging to integrate that with the full build. 

But how does your qa-testcase-data get used ? If you're applying it at test time do we need to modify the build ? Another way of asking that is 'are there any other consumers of 319MB builds?'.

Comment 12

[Jason Smith [:jsmith]]

(In reply to Nick Thomas [:nthomas] from comment #11)
> Yes it does. We'll have to do some test builds in staging to integrate that
> with the full build. 
> 
> But how does your qa-testcase-data get used ? If you're applying it at test
> time do we need to modify the build ? Another way of asking that is 'are
> there any other consumers of 319MB builds?'.

Can you be more specific about what you mean by used? Are you talking about how I'm creating a customized build using qa-testcase-data?

Comment 13

[Nick Thomas [:nthomas] (UTC+12)]

This is the first I heard of the qa-testcase-data repo, so apologies if I'm misinterpreting what it is for. I was guessing that you take a flame, fastboot it with the instruction to use 319MB, and apply qa-testcase-data. At that point I wasn't sure what the point of changing the flame builds coming off the CI is.

There's an issue keeping camera-config.json up to date with other changes, but we have that whether it's on your side or ours.

Comment 14

[Jason Smith [:jsmith]]

(In reply to Nick Thomas [:nthomas] from comment #13)
> This is the first I heard of the qa-testcase-data repo, so apologies if I'm
> misinterpreting what it is for. I was guessing that you take a flame,
> fastboot it with the instruction to use 319MB, and apply qa-testcase-data.
> At that point I wasn't sure what the point of changing the flame builds
> coming off the CI is.

Actually we do something different here - see https://wiki.mozilla.org/B2G/QA/Customizations#Build_Setup for the build command we use:

MOZILLA_OFFICIAL=1 GAIA_DISTRIBUTION_DIR=../qa-testcase-data/customization/reference make production

Comment 15

[Nick Thomas [:nthomas] (UTC+12)]

Oh, so that's a particular type of test where it was convenient to verify, but not necessarily what you want to use for. I've got a rush on for some other work this week, but will see if I can fit this into a gap.

Comment 16

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Please don't exclude 720p in all nightly pvtbuild -- people would love to enjoy the Flame with its full spec whatever QRD is capable of.

To support the QA need, we might want to create 319MB pvt builds or having QA flashing the build themselves against distribution dir.

Comment 17

[Jason Smith [:jsmith]]

(In reply to Tim Guan-tin Chien [:timdream] (MoCo-TPE) (please ni?) from comment #16)
> Please don't exclude 720p in all nightly pvtbuild -- people would love to
> enjoy the Flame with its full spec whatever QRD is capable of.
> 
> To support the QA need, we might want to create 319MB pvt builds or having
> QA flashing the build themselves against distribution dir.

I'd rather avoid generating another build here - we're already running low on build space, so adding another build is going to costly space-wise. We could look into seeing if it's possible to modify the flashing process to incorporate a customization by default though, so that's a potential option here.

Comment 18

[Tim Guan-tin Chien [:timdream] (please needinfo)]

(In reply to Jason Smith [:jsmith] from comment #17)
> I'd rather avoid generating another build here - we're already running low
> on build space, so adding another build is going to costly space-wise. We
> could look into seeing if it's possible to modify the flashing process to
> incorporate a customization by default though, so that's a potential option
> here.

Fine. If Camera developers agree with this, they can always switch off 720p with a runtime getFeature() flag.

Comment 19

[Nick Thomas [:nthomas] (UTC+12)]

This is blocked on agreement on how to proceed.

Comment 20

[Tim Guan-tin Chien [:timdream] (please needinfo)]

I still don't think we should exclude 720p outright since there are people outside Mozilla, paying money for Flame the reference device which we heavily promoted, and depend on our builds.

Comment 21

[Jason Smith [:jsmith]]

(In reply to Tim Guan-tin Chien [:timdream] (MoCo-TPE) (please ni?) from comment #20)
> I still don't think we should exclude 720p outright since there are people
> outside Mozilla, paying money for Flame the reference device which we
> heavily promoted, and depend on our builds.

I think we can come up with a solution here that does not interrupt dogfooders, but allows us to still ensure daily QA testing uses 480p - include a default customization for flashing a pvtbuild against that uses a 480p configuration. That would require us to do the following:

1. Serve a customization that uses 480p in a public Mozilla repository to pull from
2a. Modify the full flash script to allow for specifying an option to use a particular customization
2b. Modify the b2g flash tool to allow for specifying an option to use a particular customization from the public location

Nick - Do you have a preference on where we should put this customization?

Comment 22

[Nick Thomas [:nthomas] (UTC+12)]

Sorry for the slow respsonse. It seems to me all of this lives in a b2g repo somewhere (like the flash script itself), and probably a qa one since it's applicable to you most.

Can we make the customization only contain the one pref we want, and not need everything else ? It would help maintainability if that were so true, as it would be easy for gaia and the customization to diverge otherwise.