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

Issue 740794 link

Starred by 2 users

Issue metadata

Status: Verified
Owner:
Closed: Jul 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , Chrome , Mac
Pri: 3
Type: Feature

Blocking:
issue 736517



Sign in to add a comment

VideoFrame::AddDestructionObserver() [1] should be defined with a move-only OnceCallback argument.

Project Member Reported by mcasas@chromium.org, Jul 11 2017

Issue description

VideoFrame::AddDestructionObserver() [1] should be defined with a move-only
OnceCallback argument.

Found this while trying to pass a mojo move-only callback as destruction
observer (in [2,3]), but it makes sense: the destruction callback should
be called only once, upon destruction, it doesn't make any sense if a 
VideoFrame gets destructed twice :-)

[1] https://cs.chromium.org/chromium/src/media/base/video_frame.h?l=374
[2] https://chromium-review.googlesource.com/c/558846/
[3] https://bugs.chromium.org/p/chromium/issues/detail?id=736517
 

Comment 1 by mcasas@chromium.org, Jul 11 2017

Description: Show this description

Comment 2 Deleted

Comment 3 by mcasas@chromium.org, Jul 12 2017

Cc: tzik@chromium.org
+tzik@ FYI and since you seems to be doing similar work in 
https://bugs.chromium.org/p/chromium/issues/detail?id=714018
Project Member

Comment 4 by bugdroid1@chromium.org, Jul 13 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/bd4a286c40bc98c1c2e9ee210fec484ee38eaf31

commit bd4a286c40bc98c1c2e9ee210fec484ee38eaf31
Author: tzik <tzik@chromium.org>
Date: Thu Jul 13 02:21:56 2017

Use OnceCallback on VideoFrame::AddDestructionObserver

Bug:  740794 
Change-Id: I7e4de04dfd2230b1eef2d7e85d7874055122b6dd
Reviewed-on: https://chromium-review.googlesource.com/567902
Reviewed-by: Xiaohan Wang <xhwang@chromium.org>
Reviewed-by: Miguel Casas <mcasas@chromium.org>
Commit-Queue: Taiju Tsuiki <tzik@chromium.org>
Cr-Commit-Position: refs/heads/master@{#486224}
[modify] https://crrev.com/bd4a286c40bc98c1c2e9ee210fec484ee38eaf31/media/base/video_frame.cc
[modify] https://crrev.com/bd4a286c40bc98c1c2e9ee210fec484ee38eaf31/media/base/video_frame.h

Comment 5 by mcasas@chromium.org, Jul 13 2017

Blocking: 736517

Comment 6 by mcasas@chromium.org, Jul 13 2017

Owner: tzik@chromium.org
Status: Fixed (was: Available)
Should be fixed by #4.
Status: Verified (was: Fixed)

Sign in to add a comment