Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1881933004:
Introduce an IsMutable method on VideoFrameBuffer. (Closed)
|
Created:
4 years, 8 months ago by nisse-webrtc Modified:
4 years, 8 months ago CC:
webrtc-reviews_webrtc.org, tterriberry_mozilla.com Base URL:
https://chromium.googlesource.com/external/webrtc.git@master Target Ref:
refs/pending/heads/master Project:
webrtc Visibility:
Public. |
DescriptionIntroduce an IsMutable method on VideoFrameBuffer.
Unlike HasOneRef, it can be overridden to always return false in
immutable subclasses.
I'm also investigating overiding it in PooledI420Buffer, to directly
inherit I420Buffer but ignore the reference from the pool. Still
unclear if that will work out.
BUG=webrtc:5682
Committed: https://crrev.com/6bd10f2c1ac912cbe5addd880e559d59274c60e6
Cr-Commit-Position: refs/heads/master@{#12365}
R=magjed@webrtc.org, pbos@webrtc.org, perkj@webrtc.org
Committed: https://chromium.googlesource.com/external/webrtc/+/47fe34c2bd38927d78925174390a9e07efc3752f
Patch Set 1 #
Total comments: 3
Patch Set 2 : Add default implementation of IsMutable. Intending to reland. #Patch Set 3 : Rebase. #
Messages
Total messages: 34 (15 generated)
nisse@webrtc.org changed reviewers: + magjed@webrtc.org, pbos@webrtc.org, perkj@webrtc.org
lgtm https://codereview.webrtc.org/1881933004/diff/1/webrtc/media/engine/webrtcvid... File webrtc/media/engine/webrtcvideoframe.cc (right): https://codereview.webrtc.org/1881933004/diff/1/webrtc/media/engine/webrtcvid... webrtc/media/engine/webrtcvideoframe.cc:115: bool WebRtcVideoFrame::IsExclusive() const { Should IsExlusive also be removed and a user forced to use video_frame_buffer()->IsMutable() ?
https://codereview.webrtc.org/1881933004/diff/1/webrtc/media/engine/webrtcvid... File webrtc/media/engine/webrtcvideoframe.cc (right): https://codereview.webrtc.org/1881933004/diff/1/webrtc/media/engine/webrtcvid... webrtc/media/engine/webrtcvideoframe.cc:115: bool WebRtcVideoFrame::IsExclusive() const { On 2016/04/14 06:34:51, perkj_webrtc wrote: > Should IsExlusive also be removed and a user forced to use > video_frame_buffer()->IsMutable() ? I think so.
lgtm
lgtm https://codereview.webrtc.org/1881933004/diff/1/webrtc/media/engine/webrtcvid... File webrtc/media/engine/webrtcvideoframe.cc (right): https://codereview.webrtc.org/1881933004/diff/1/webrtc/media/engine/webrtcvid... webrtc/media/engine/webrtcvideoframe.cc:115: bool WebRtcVideoFrame::IsExclusive() const { On 2016/04/14 06:42:28, nisse-webrtc wrote: > On 2016/04/14 06:34:51, perkj_webrtc wrote: > > Should IsExlusive also be removed and a user forced to use > > video_frame_buffer()->IsMutable() ? > > I think so. I agree.
The CQ bit was checked by nisse@webrtc.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1881933004/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1881933004/1
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: win_baremetal on tryserver.webrtc (JOB_TIMED_OUT, no build URL)
The CQ bit was checked by nisse@webrtc.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1881933004/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1881933004/1
Message was sent while issue was closed.
Committed patchset #1 (id:1)
Message was sent while issue was closed.
Description was changed from ========== Introduce an IsMutable method on VideoFrameBuffer. Unlike HasOneRef, it can be overridden to always return false in immutable subclasses. I'm also investigating overiding it in PooledI420Buffer, to directly inherit I420Buffer but ignore the reference from the pool. Still unclear if that will work out. BUG=webrtc:5682 ========== to ========== Introduce an IsMutable method on VideoFrameBuffer. Unlike HasOneRef, it can be overridden to always return false in immutable subclasses. I'm also investigating overiding it in PooledI420Buffer, to directly inherit I420Buffer but ignore the reference from the pool. Still unclear if that will work out. BUG=webrtc:5682 Committed: https://crrev.com/6bd10f2c1ac912cbe5addd880e559d59274c60e6 Cr-Commit-Position: refs/heads/master@{#12365} ==========
Message was sent while issue was closed.
Patchset 1 (id:??) landed as https://crrev.com/6bd10f2c1ac912cbe5addd880e559d59274c60e6 Cr-Commit-Position: refs/heads/master@{#12365}
Message was sent while issue was closed.
A revert of this CL (patchset #1 id:1) has been created in https://codereview.webrtc.org/1885943004/ by guidou@webrtc.org. The reason for reverting is: This is breaking all FYI bots. The new virtual method is not implemented on the Chromium side yet..
Message was sent while issue was closed.
Description was changed from ========== Introduce an IsMutable method on VideoFrameBuffer. Unlike HasOneRef, it can be overridden to always return false in immutable subclasses. I'm also investigating overiding it in PooledI420Buffer, to directly inherit I420Buffer but ignore the reference from the pool. Still unclear if that will work out. BUG=webrtc:5682 Committed: https://crrev.com/6bd10f2c1ac912cbe5addd880e559d59274c60e6 Cr-Commit-Position: refs/heads/master@{#12365} ========== to ========== Introduce an IsMutable method on VideoFrameBuffer. Unlike HasOneRef, it can be overridden to always return false in immutable subclasses. I'm also investigating overiding it in PooledI420Buffer, to directly inherit I420Buffer but ignore the reference from the pool. Still unclear if that will work out. BUG=webrtc:5682 Committed: https://crrev.com/6bd10f2c1ac912cbe5addd880e559d59274c60e6 Cr-Commit-Position: refs/heads/master@{#12365} ==========
The CQ bit was checked by nisse@webrtc.org
The patchset sent to the CQ was uploaded after l-g-t-m from magjed@webrtc.org, pbos@webrtc.org, perkj@webrtc.org Link to the patchset: https://codereview.webrtc.org/1881933004/#ps20001 (title: "Add default implementation of IsMutable. Intending to reland.")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1881933004/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1881933004/20001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: win_baremetal on tryserver.webrtc (JOB_TIMED_OUT, no build URL)
The CQ bit was checked by nisse@webrtc.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1881933004/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1881933004/20001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: win_baremetal on tryserver.webrtc (JOB_TIMED_OUT, no build URL)
The CQ bit was checked by nisse@webrtc.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1881933004/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1881933004/20001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: win_baremetal on tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/win_baremetal/builds/10550)
Message was sent while issue was closed.
Description was changed from ========== Introduce an IsMutable method on VideoFrameBuffer. Unlike HasOneRef, it can be overridden to always return false in immutable subclasses. I'm also investigating overiding it in PooledI420Buffer, to directly inherit I420Buffer but ignore the reference from the pool. Still unclear if that will work out. BUG=webrtc:5682 Committed: https://crrev.com/6bd10f2c1ac912cbe5addd880e559d59274c60e6 Cr-Commit-Position: refs/heads/master@{#12365} ========== to ========== Introduce an IsMutable method on VideoFrameBuffer. Unlike HasOneRef, it can be overridden to always return false in immutable subclasses. I'm also investigating overiding it in PooledI420Buffer, to directly inherit I420Buffer but ignore the reference from the pool. Still unclear if that will work out. BUG=webrtc:5682 Committed: https://crrev.com/6bd10f2c1ac912cbe5addd880e559d59274c60e6 Cr-Commit-Position: refs/heads/master@{#12365} R=magjed@webrtc.org, pbos@webrtc.org, perkj@webrtc.org Committed: https://crrev.com/47fe34c2bd38927d78925174390a9e07efc3752f Cr-Commit-Position: refs/heads/master@{#12404} ==========
Message was sent while issue was closed.
Patchset 3 (id:??) landed as https://crrev.com/47fe34c2bd38927d78925174390a9e07efc3752f Cr-Commit-Position: refs/heads/master@{#12404}
Message was sent while issue was closed.
Description was changed from ========== Introduce an IsMutable method on VideoFrameBuffer. Unlike HasOneRef, it can be overridden to always return false in immutable subclasses. I'm also investigating overiding it in PooledI420Buffer, to directly inherit I420Buffer but ignore the reference from the pool. Still unclear if that will work out. BUG=webrtc:5682 Committed: https://crrev.com/6bd10f2c1ac912cbe5addd880e559d59274c60e6 Cr-Commit-Position: refs/heads/master@{#12365} R=magjed@webrtc.org, pbos@webrtc.org, perkj@webrtc.org Committed: https://crrev.com/47fe34c2bd38927d78925174390a9e07efc3752f Cr-Commit-Position: refs/heads/master@{#12404} ========== to ========== Introduce an IsMutable method on VideoFrameBuffer. Unlike HasOneRef, it can be overridden to always return false in immutable subclasses. I'm also investigating overiding it in PooledI420Buffer, to directly inherit I420Buffer but ignore the reference from the pool. Still unclear if that will work out. BUG=webrtc:5682 Committed: https://crrev.com/6bd10f2c1ac912cbe5addd880e559d59274c60e6 Cr-Commit-Position: refs/heads/master@{#12365} R=magjed@webrtc.org, pbos@webrtc.org, perkj@webrtc.org Committed: https://chromium.googlesource.com/external/webrtc/+/47fe34c2bd38927d789251743... ==========
Message was sent while issue was closed.
Committed patchset #3 (id:40001) manually as 47fe34c2bd38927d78925174390a9e07efc3752f (presubmit successful). |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
