New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 607272 link

Starred by 1 user

Issue metadata

Status: WontFix
Owner:
Closed: Feb 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 2
Type: Feature

Blocking:
issue 492839



Sign in to add a comment

Add FrameSubscriber interface to ui::Layer

Project Member Reported by m...@chromium.org, Apr 27 2016

Issue description

Recently, a change was made to desktop/window capture on CrOS (and the same code is also used for Browser Window capture on Win/Linux) to fix a "freezing" problem.  https://codereview.chromium.org/1922143004/  This solution is a hack, in that it is mis-using a compositor animation observer callback to achieve guaranteed frame captures.  This solution is also very poor for performance across-the-board, whether you're considering CPU/GPU/Power usage or quality/smoothness of animating content.

What we need is something analogous to FrameSubscriber, the event-driven mechanism between DelegatedFrameHost and WebContentsVideoCaptureDevice used by tab capture.  Reference: https://code.google.com/p/chromium/codesearch#chromium/src/content/public/browser/render_widget_host_view_frame_subscriber.h&rcl=1461755891&l=30

Here's what I was thinking this might look like:

  // ui::Compositor
  class Compositor {
   public:
    // Clients implement this interface in order to receive notifications about
    // damage to a target (e.g., a ui::Layer) that is about to be resolved by
    // draw/commit.
    //
    // Example usage: When copying textures for screen capture, a client might
    // conditionally add a ui::CopyOutputRequest to a layer, according to its
    // configured sampling rate and current environmental conditions.
    class UpdateSubscriber {
     public:
      // Called when a damaged target will be composited. |damage_rect| is the
      // region within the target that it dirty and |present_time| is when the
      // result will be presented on the display.
      virtual bool WillCommitUpdate(const gfx::Rect& damage_rect,
                                    base::TimeTicks present_time) = 0;
    };

    ...

    void AddUpdateSubscriberForLayer(ui::Layer* layer,
                                     UpdateSubscriber* subscriber);
    void RemoveUpdateSubscriberForLayer(ui::Layer* layer,
                                        UpdateSubscriber* subscriber);

    ...
  };

Then, example usage by content::AuraWindowCaptureMachine might be:

  class CaptureSubscriber : public ui::Compositor::UpdateSubscriber {
    void WillCommitUpdate(damage_rect, present_time) override {
      if (damage_rect.IsEmpty())
        return;

      scoped_refptr<media::VideoFrame> output_frame;
      base::Callback<...> done_cb;
      if (oracle_->ObserveEventAndDecideCapture(damage_rect, present_time,
                                                &output_frame, &done_cb)) {
        target_->RequestCopyOfOutput(
            cc::CopyOutputRequest::CreateRequest(base::Bind(
                &AuraWindowCaptureMachine::DidCopyOutput,
                machine_, output_frame, present_time, done_cb)));
      }
    }

   private:
    ui::Layer* target_;
    base::WeakPtr<AuraWindowCaptureMachine> machine_;
    scoped_refptr<media::ThreadSafeCaptureOracle> oracle_;
  }

  ...

  // In AuraWindowCaptureMachine::InternalStart():
  window_->GetHost()->compositor()->AddUpdateSubscriberForLayer(window_->layer());

 

Comment 1 by m...@chromium.org, Apr 27 2016

Blocking: 492839
Owner: qiangchen@chromium.org
Cc: jbau...@chromium.org
Unfortunately the browser compositor won't do a commit if only a child surface changes (from a child renderer or OOPIF).

I'm not sure of the best interface that works across all parts of the stack. Maybe we need a long term CopyOutputRequest/FrameSubscriber hybrid, one that isn't removed from the layer after one copy, but instead stays on there until removed manually. It could also veto capture if necessary, though that would be less efficient than not having one attached at all.

Comment 4 by yuweih@chromium.org, May 12 2016

Cc: yuweih@chromium.org
Project Member

Comment 5 by sheriffbot@chromium.org, Jul 12 2016

Labels: -M-53 M-54 MovedFrom-53
Moving this nonessential bug to the next milestone.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Status: Assigned (was: Available)
qiangchen@ is this gonna make it in M54?
Nope. As no functionality is currently broken, low priority for this bug.
Labels: -M-54 -MovedFrom-53
Labels: -Type-Bug-Regression Type-Feature
It looks to me that this is not a regression or even a bug, so I'm changing the type to Feature. Please correct if that's wrong.

Comment 10 by m...@chromium.org, Feb 7 2018

Status: WontFix (was: Assigned)
FrameSubscriber was just deleted (as part of the old tab capture implementation). And, Aura Window capture will soon be replaced, tracked in  bug 806366 .

Sign in to add a comment