Mir

Merge lp://staging/~albaguirre/mir/screencast-capture-rate-limiter into lp://staging/mir

  1. screencast-capture-rate-limiter
  2. Merge into development-branch
Proposed by Alberto Aguirre
Status: Merged
Approved by: Alan Griffiths
Approved revision: no longer in the source branch.
Merged at revision: 1541
Proposed branch: lp://staging/~albaguirre/mir/screencast-capture-rate-limiter
Merge into: lp://staging/mir
Diff against target: 257 lines (+77/-31)
1 file modified
src/utils/screencast.cpp (+77/-31)
To merge this branch: bzr merge lp://staging/~albaguirre/mir/screencast-capture-rate-limiter
Reviewer Review Type Date Requested Status
PS Jenkins bot (community) continuous-integration Approve
Alan Griffiths Approve
Alexandros Frantzis (community) Approve
Kevin DuBois (community) Approve
Review via email: mp+214122@code.staging.launchpad.net

Commit message

Add capture rate limiting to mirscreencast (LP: #1294361, LP: #1280938)

Description of the change

Add capture rate limiting to mirscreencast (LP: #1294361, LP: #1280938)

To post a comment you must log in.
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

PASSED: Continuous integration, rev:1528
http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-ci/1243/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-trusty-i386-build/1481
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-trusty-amd64-build/1479
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-trusty-touch/1058
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-trusty-amd64-ci/975
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-trusty-amd64-ci/975/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-trusty-armhf-ci/980
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-trusty-armhf-ci/980/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-trusty-armhf/1059
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-trusty-armhf/1059/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/979
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/5609

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-team-mir-development-branch-ci/1243/rebuild

review: Approve (continuous-integration)
Revision history for this message
Kevin DuBois (kdub) wrote :

lgtm. Slight preference for '--max-fps' over '--fps' as the option name

review: Approve
Revision history for this message
Alexandros Frantzis (afrantzis) wrote :

Looks good, with a caveat: I am OK with the client being able to control its screencasting FPS, but the maximum FPS rate should be enforced at the server side. Otherwise, poorly coded or even malevolent clients could affect the overall rendering rate.

review: Approve
Revision history for this message
Daniel van Vugt (vanvugt) wrote :

I can't help but think maybe it's not the framerate we should specify but something more like a swap interval relative to the real compositor:
  1 -> 60 FPS
  2 -> 30 FPS
  3 -> 20 FPS

Revision history for this message
Alan Griffiths (alan-griffiths) wrote :

> lgtm. Slight preference for '--max-fps' over '--fps' as the option name

or --fps-limit

review: Approve
Revision history for this message
Alberto Aguirre (albaguirre) wrote :

> Looks good, with a caveat: I am OK with the client being able to control its
> screencasting FPS, but the maximum FPS rate should be enforced at the server
> side. Otherwise, poorly coded or even malevolent clients could affect the
> overall rendering rate.

Yeah I agree, but that will be a different MP.

Revision history for this message
Alberto Aguirre (albaguirre) wrote :

> I can't help but think maybe it's not the framerate we should specify but
> something more like a swap interval relative to the real compositor:
> 1 -> 60 FPS
> 2 -> 30 FPS
> 3 -> 20 FPS

Yeah I like that idea - I'll change it.

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

PASSED: Continuous integration, rev:1529
http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-ci/1271/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-trusty-i386-build/1524
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-trusty-amd64-build/1522
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-trusty-touch/1098
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-trusty-amd64-ci/1003
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-trusty-amd64-ci/1003/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-trusty-armhf-ci/1008
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-trusty-armhf-ci/1008/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-trusty-armhf/1099
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-trusty-armhf/1099/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/1017
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/5723

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-team-mir-development-branch-ci/1271/rebuild

review: Approve (continuous-integration)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches