Closed
Bug 1256626
Opened 9 years ago
Closed 9 years ago
[e10s] Benchmark result isn't stored when e10s is enabled
Categories
(Core :: Audio/Video: Playback, defect, P2)
Core
Audio/Video: Playback
Tracking
()
RESOLVED
FIXED
mozilla48
Tracking | Status | |
---|---|---|
e10s | + | --- |
firefox45 | --- | unaffected |
firefox46 | --- | unaffected |
firefox47 | --- | fixed |
firefox48 | --- | fixed |
People
(Reporter: jya, Assigned: jya)
References
Details
Attachments
(2 files)
(deleted),
text/x-review-board-request
|
jimm
:
review+
ritu
:
approval-mozilla-aurora+
|
Details |
(deleted),
text/x-review-board-request
|
jimm
:
review+
ritu
:
approval-mozilla-aurora+
|
Details |
Bug 1230265 added benchmarking VP9 in order to enable some features.
This do not work with e10s enabled.
Reason being Preferences::SetUint do not work when called from the content process
// static
nsresult
Preferences::SetInt(const char* aPref, int32_t aValue)
{
ENSURE_MAIN_PROCESS("Cannot SetInt from content process:", aPref);
NS_ENSURE_TRUE(InitStaticMembers(), NS_ERROR_NOT_AVAILABLE);
return PREF_SetIntPref(aPref, aValue, false);
}
Need to find another way to store the results.
Updated•9 years ago
|
tracking-e10s:
--- → +
Updated•9 years ago
|
Priority: -- → P3
Assignee | ||
Comment 1•9 years ago
|
||
This will be used to save video benchmark results. For now only VP9 is handled.
Review commit: https://reviewboard.mozilla.org/r/40713/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/40713/
Attachment #8731561 -
Flags: review?(jmathies)
Assignee | ||
Comment 2•9 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/40715/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/40715/
Attachment #8731562 -
Flags: review?(jmathies)
Assignee | ||
Comment 3•9 years ago
|
||
I don't see ContentParent.cpp aging very well if it's the unique mechanism/place to add a new service.
But seeing how small the new NotifyBenchmarkResult code is, I don't see a more elegant way to do that.
I thought of simply calling a static method in VP9Benchmark from ContentParent, but it's no better really.
Assignee | ||
Comment 4•9 years ago
|
||
this is blocking us from enabling webm/VP9 (as used by YouTube) for all users with machines fast enough
Priority: P3 → P2
Updated•9 years ago
|
Attachment #8731561 -
Flags: review?(jmathies) → review+
Comment 5•9 years ago
|
||
Comment on attachment 8731561 [details]
MozReview Request: Bug 1256626: P1. Add NotifyBenchmarkResult ipc methods. r?jimm
https://reviewboard.mozilla.org/r/40713/#review37683
Comment 6•9 years ago
|
||
Comment on attachment 8731562 [details]
MozReview Request: Bug 1256626: P2. Use NotifyBenchmarkResult to save VP9 result. r?jimm
https://reviewboard.mozilla.org/r/40715/#review37685
Attachment #8731562 -
Flags: review?(jmathies) → review+
Assignee | ||
Comment 7•9 years ago
|
||
Thanks for the reviews
Comment 10•9 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/b8375e4f9162
https://hg.mozilla.org/mozilla-central/rev/0ecd210e6b1e
https://hg.mozilla.org/mozilla-central/rev/84cb4926cca2
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
Assignee | ||
Comment 11•9 years ago
|
||
Comment on attachment 8731561 [details]
MozReview Request: Bug 1256626: P1. Add NotifyBenchmarkResult ipc methods. r?jimm
Patch request is for all changeset
Approval Request Comment
[Feature/regressing bug #]: 1256626
[User impact if declined]: We aim to enable VP9 to all machines fast enough to support it. It would also reduce issues seen with h264 hardware decoder.
[Describe test coverage new/current, TreeHerder]: in central for a few weeks.
[Risks and why]: Low, people may complain of a cpu usage spike as youtube prefers Vp9 over h264 if available. Overall playback quality should be greater however
[String/UUID change made/needed]: none
Attachment #8731561 -
Flags: approval-mozilla-aurora?
status-firefox47:
--- → affected
Hi Jean-Yves, Chris, I was under the impression that VP9 would not be enabled in Fx47. If that is still the case, do we still need to uplift this to Aurora 47? Please let me know.
Flags: needinfo?(jyavenard)
Flags: needinfo?(cpeterson)
Assignee | ||
Comment 13•9 years ago
|
||
I only saw Chris marking 46as won't fix.
We do want to enable vp9 in 47.
Flags: needinfo?(jyavenard)
Comment 14•9 years ago
|
||
e10s is not enabled in Beta 47, so we don't need to uplift this e10s fix to 47.
Comment on attachment 8731561 [details]
MozReview Request: Bug 1256626: P1. Add NotifyBenchmarkResult ipc methods. r?jimm
Media team wants to enable VP9 on faster machines in 47, I have been told this is mostly a staged rollout and quality is acceptable. Aurora47+
Attachment #8731561 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Attachment #8731562 -
Flags: approval-mozilla-aurora+
Comment 16•9 years ago
|
||
bugherder uplift |
You need to log in
before you can comment on or make changes to this bug.
Description
•