Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2607933002:
RTCMediaStreamTrackStats.framesDropped collected by RTCStatsCollector. (Closed)
|
Created:
3 years, 11 months ago by hbos Modified:
3 years, 11 months ago CC:
webrtc-reviews_webrtc.org, interface-changes_webrtc.org, video-team_agora.io, yujie_mao (webrtc), zhengzhonghou_agora.io, stefan-webrtc, tterriberry_mozilla.com, qiang.lu, niklas.enbom, peah-webrtc, the sun, mflodman Target Ref:
refs/heads/master Project:
webrtc Visibility:
Public. |
DescriptionRTCMediaStreamTrackStats.framesDropped collected by RTCStatsCollector.
Spec: https://w3c.github.io/webrtc-stats/#dom-rtcmediastreamtrackstats-framesdropped
Implemented as frames_received - frames_rendered.
Part of this CL is adding frames_rendered to VideoReceiveStream::Stats
and updating it at ReceiveStatisticsProxy::OnRenderedFrame.
BUG=webrtc:6757, chromium:659137, chromium:627816
NOTRY=True
Review-Url: https://codereview.webrtc.org/2607933002
Cr-Commit-Position: refs/heads/master@{#16213}
Committed: https://chromium.googlesource.com/external/webrtc/+/50cfe1fda7f2b4a6a449fe7234f4c1aa2a475c61
Patch Set 1 #Patch Set 2 : Rebase after RTCMediaStreamTrackStats being per-attachment #Patch Set 3 : Added unittests for webrtcvideoengine2 and receive_statistics_proxy #
Total comments: 3
Patch Set 4 : Rebase with master and unittest listing "rendered: 13, decoded: 14" #Patch Set 5 : Rebase with master after webrtc/api renamed to webrtc/pc #
Messages
Total messages: 53 (34 generated)
The CQ bit was checked by hbos@webrtc.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...
hbos@webrtc.org changed reviewers: + deadbeef@webrtc.org, hta@webrtc.org
Please take a look, hta and deadbeef.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: ios32_sim_dbg on master.tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/ios32_sim_dbg/builds/13743) ios_rel on master.tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/ios_rel/builds/20067) linux_libfuzzer_rel on master.tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/linux_libfuzzer_rel/bui...)
On 2016/12/29 14:50:25, hbos wrote: > Please take a look, hta and deadbeef. Patch failure on bots having to do with dependent patchset, not sure what's going on there, I'll rebase after land.
lgtm
On 2017/01/03 08:12:12, hta-webrtc wrote: > lgtm What about the issue you raised about tracks switching between SSRCs? The counts would be for the current SSRC only. I believe we have this problem for all of my recent RTCMediaStreamTrackStats CLs, except maybe it doesn't matter on the frames per second and frame width/height since these don't accumulate. Do we land (better than nothing) and file a bug to replace the code or do we hold off landing altogether for the counts ones?
On 2017/01/03 09:03:51, hbos wrote: > On 2017/01/03 08:12:12, hta-webrtc wrote: > > lgtm > > What about the issue you raised about tracks switching between SSRCs? The counts > would be for the current SSRC only. I believe we have this problem for all of my > recent RTCMediaStreamTrackStats CLs, except maybe it doesn't matter on the > frames per second and frame width/height since these don't accumulate. Do we > land (better than nothing) and file a bug to replace the code or do we hold off > landing altogether for the counts ones? Let's land what we can.
The CQ bit was checked by hbos@webrtc.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...
Description was changed from ========== RTCMediaStreamTrackStats.framesDropped collected by RTCStatsCollector. Implemented as frames_received - frames_rendered. BUG=webrtc:6757, chromium:659137, chromium:627816 ========== to ========== RTCMediaStreamTrackStats.framesDropped collected by RTCStatsCollector. Spec: https://w3c.github.io/webrtc-stats/#dom-rtcmediastreamtrackstats-framesdropped Implemented as frames_received - frames_rendered. BUG=webrtc:6757, chromium:659137, chromium:627816 ==========
Description was changed from ========== RTCMediaStreamTrackStats.framesDropped collected by RTCStatsCollector. Spec: https://w3c.github.io/webrtc-stats/#dom-rtcmediastreamtrackstats-framesdropped Implemented as frames_received - frames_rendered. BUG=webrtc:6757, chromium:659137, chromium:627816 ========== to ========== RTCMediaStreamTrackStats.framesDropped collected by RTCStatsCollector. Spec: https://w3c.github.io/webrtc-stats/#dom-rtcmediastreamtrackstats-framesdropped Implemented as frames_received - frames_rendered. Part of this CL is adding frames_rendered to VideoReceiveStream::Stats and updating it at ReceiveStatisticsProxy::OnRenderedFrame. BUG=webrtc:6757, chromium:659137, chromium:627816 ==========
The CQ bit was checked by hbos@webrtc.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...
hbos@webrtc.org changed reviewers: + asapersson@webrtc.org, pbos@webrtc.org
Please take a look, deadbeef, pbos and asapersson.
deadbeef: *stats*
pbos: webrtc/media
webrtc/video_receive_stream.h
asapersson: webrtc/video
On 2017/01/03 17:02:02, hta-webrtc wrote:
> On 2017/01/03 09:03:51, hbos wrote:
> > On 2017/01/03 08:12:12, hta-webrtc wrote:
> > > lgtm
> >
> > What about the issue you raised about tracks switching between SSRCs? The
> counts
> > would be for the current SSRC only. I believe we have this problem for all
of
> my
> > recent RTCMediaStreamTrackStats CLs, except maybe it doesn't matter on the
> > frames per second and frame width/height since these don't accumulate. Do we
> > land (better than nothing) and file a bug to replace the code or do we hold
> off
> > landing altogether for the counts ones?
>
> Let's land what we can.
Now that the track stats are collected per-attachment to the peer connection
(per SSRC) the counts are correct.
webrtc/video: lgtm
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
lgtm
webrtc/media lgtm https://codereview.webrtc.org/2607933002/diff/40001/webrtc/media/engine/webrt... File webrtc/media/engine/webrtcvideoengine2_unittest.cc (right): https://codereview.webrtc.org/2607933002/diff/40001/webrtc/media/engine/webrt... webrtc/media/engine/webrtcvideoengine2_unittest.cc:3307: stats.frames_rendered = 12; 14, but nit/whatever
https://codereview.webrtc.org/2607933002/diff/40001/webrtc/media/engine/webrt... File webrtc/media/engine/webrtcvideoengine2_unittest.cc (right): https://codereview.webrtc.org/2607933002/diff/40001/webrtc/media/engine/webrt... webrtc/media/engine/webrtcvideoengine2_unittest.cc:3307: stats.frames_rendered = 12; On 2017/01/19 17:14:03, pbos-webrtc wrote: > 14, but nit/whatever It would be reasonable to have a DCHECK for "frames_rendered < frames_decoded" though, so if this is changed it should probably be decoded=14, rendered=13.
Patchset #4 (id:60001) has been deleted
The CQ bit was checked by hbos@webrtc.org
The patchset sent to the CQ was uploaded after l-g-t-m from hta@webrtc.org, deadbeef@webrtc.org, asapersson@webrtc.org, pbos@webrtc.org Link to the patchset: https://codereview.webrtc.org/2607933002/#ps80001 (title: "Rebase with master")
The CQ bit was unchecked by hbos@webrtc.org
Patchset #4 (id:80001) has been deleted
https://codereview.webrtc.org/2607933002/diff/40001/webrtc/media/engine/webrt... File webrtc/media/engine/webrtcvideoengine2_unittest.cc (right): https://codereview.webrtc.org/2607933002/diff/40001/webrtc/media/engine/webrt... webrtc/media/engine/webrtcvideoengine2_unittest.cc:3307: stats.frames_rendered = 12; On 2017/01/19 17:23:08, Taylor Brandstetter wrote: > On 2017/01/19 17:14:03, pbos-webrtc wrote: > > 14, but nit/whatever > > It would be reasonable to have a DCHECK for "frames_rendered < frames_decoded" > though, so if this is changed it should probably be decoded=14, rendered=13. Changed the order so that it's ..., 12, 13 (rendered), 14 (decoded).
The CQ bit was checked by hbos@webrtc.org
The patchset sent to the CQ was uploaded after l-g-t-m from deadbeef@webrtc.org, hta@webrtc.org, asapersson@webrtc.org, pbos@webrtc.org Link to the patchset: https://codereview.webrtc.org/2607933002/#ps100001 (title: "Rebase with master and unittest listing "rendered: 13, decoded: 14"")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: win_clang_dbg on master.tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/win_clang_dbg/builds/10173) win_clang_rel on master.tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/win_clang_rel/builds/10081) win_dbg on master.tryserver.webrtc (JOB_FAILED, no build URL) win_rel on master.tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/win_rel/builds/22398) win_x64_clang_dbg on master.tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/win_x64_clang_dbg/build...) win_x64_clang_rel on master.tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/win_x64_clang_rel/build...) win_x64_dbg on master.tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/win_x64_dbg/builds/5501) win_x64_rel on master.tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/win_x64_rel/builds/21904)
Description was changed from ========== RTCMediaStreamTrackStats.framesDropped collected by RTCStatsCollector. Spec: https://w3c.github.io/webrtc-stats/#dom-rtcmediastreamtrackstats-framesdropped Implemented as frames_received - frames_rendered. Part of this CL is adding frames_rendered to VideoReceiveStream::Stats and updating it at ReceiveStatisticsProxy::OnRenderedFrame. BUG=webrtc:6757, chromium:659137, chromium:627816 ========== to ========== RTCMediaStreamTrackStats.framesDropped collected by RTCStatsCollector. Spec: https://w3c.github.io/webrtc-stats/#dom-rtcmediastreamtrackstats-framesdropped Implemented as frames_received - frames_rendered. Part of this CL is adding frames_rendered to VideoReceiveStream::Stats and updating it at ReceiveStatisticsProxy::OnRenderedFrame. BUG=webrtc:6757, chromium:659137, chromium:627816 TBR=True ==========
Description was changed from ========== RTCMediaStreamTrackStats.framesDropped collected by RTCStatsCollector. Spec: https://w3c.github.io/webrtc-stats/#dom-rtcmediastreamtrackstats-framesdropped Implemented as frames_received - frames_rendered. Part of this CL is adding frames_rendered to VideoReceiveStream::Stats and updating it at ReceiveStatisticsProxy::OnRenderedFrame. BUG=webrtc:6757, chromium:659137, chromium:627816 TBR=True ========== to ========== RTCMediaStreamTrackStats.framesDropped collected by RTCStatsCollector. Spec: https://w3c.github.io/webrtc-stats/#dom-rtcmediastreamtrackstats-framesdropped Implemented as frames_received - frames_rendered. Part of this CL is adding frames_rendered to VideoReceiveStream::Stats and updating it at ReceiveStatisticsProxy::OnRenderedFrame. BUG=webrtc:6757, chromium:659137, chromium:627816 NOTRY=True ==========
The CQ bit was checked by hbos@webrtc.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: presubmit on master.tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/presubmit/builds/12541)
Description was changed from ========== RTCMediaStreamTrackStats.framesDropped collected by RTCStatsCollector. Spec: https://w3c.github.io/webrtc-stats/#dom-rtcmediastreamtrackstats-framesdropped Implemented as frames_received - frames_rendered. Part of this CL is adding frames_rendered to VideoReceiveStream::Stats and updating it at ReceiveStatisticsProxy::OnRenderedFrame. BUG=webrtc:6757, chromium:659137, chromium:627816 NOTRY=True ========== to ========== RTCMediaStreamTrackStats.framesDropped collected by RTCStatsCollector. Spec: https://w3c.github.io/webrtc-stats/#dom-rtcmediastreamtrackstats-framesdropped Implemented as frames_received - frames_rendered. Part of this CL is adding frames_rendered to VideoReceiveStream::Stats and updating it at ReceiveStatisticsProxy::OnRenderedFrame. BUG=webrtc:6757, chromium:659137, chromium:627816 ==========
The CQ bit was checked by hbos@webrtc.org
The patchset sent to the CQ was uploaded after l-g-t-m from deadbeef@webrtc.org, hta@webrtc.org, asapersson@webrtc.org, pbos@webrtc.org Link to the patchset: https://codereview.webrtc.org/2607933002/#ps120001 (title: "Rebase with master after webrtc/api renamed to webrtc/pc")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: win_baremetal on master.tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/win_baremetal/builds/17906) win_clang_dbg on master.tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/win_clang_dbg/builds/10182) win_clang_rel on master.tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/win_clang_rel/builds/10090) win_dbg on master.tryserver.webrtc (JOB_FAILED, no build URL) win_rel on master.tryserver.webrtc (JOB_FAILED, no build URL) win_x64_clang_dbg on master.tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/win_x64_clang_dbg/build...) win_x64_clang_rel on master.tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/win_x64_clang_rel/build...) win_x64_dbg on master.tryserver.webrtc (JOB_FAILED, no build URL) win_x64_rel on master.tryserver.webrtc (JOB_FAILED, no build URL)
Description was changed from ========== RTCMediaStreamTrackStats.framesDropped collected by RTCStatsCollector. Spec: https://w3c.github.io/webrtc-stats/#dom-rtcmediastreamtrackstats-framesdropped Implemented as frames_received - frames_rendered. Part of this CL is adding frames_rendered to VideoReceiveStream::Stats and updating it at ReceiveStatisticsProxy::OnRenderedFrame. BUG=webrtc:6757, chromium:659137, chromium:627816 ========== to ========== RTCMediaStreamTrackStats.framesDropped collected by RTCStatsCollector. Spec: https://w3c.github.io/webrtc-stats/#dom-rtcmediastreamtrackstats-framesdropped Implemented as frames_received - frames_rendered. Part of this CL is adding frames_rendered to VideoReceiveStream::Stats and updating it at ReceiveStatisticsProxy::OnRenderedFrame. BUG=webrtc:6757, chromium:659137, chromium:627816 NOTRY=True ==========
The CQ bit was checked by hbos@webrtc.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...
CQ is committing da patch.
Bot data: {"patchset_id": 120001, "attempt_start_ts": 1485184772560890,
"parent_rev": "9c3d4c4d881be595dd8e68e2130f23d9f4fab057", "commit_rev":
"50cfe1fda7f2b4a6a449fe7234f4c1aa2a475c61"}
Message was sent while issue was closed.
Description was changed from ========== RTCMediaStreamTrackStats.framesDropped collected by RTCStatsCollector. Spec: https://w3c.github.io/webrtc-stats/#dom-rtcmediastreamtrackstats-framesdropped Implemented as frames_received - frames_rendered. Part of this CL is adding frames_rendered to VideoReceiveStream::Stats and updating it at ReceiveStatisticsProxy::OnRenderedFrame. BUG=webrtc:6757, chromium:659137, chromium:627816 NOTRY=True ========== to ========== RTCMediaStreamTrackStats.framesDropped collected by RTCStatsCollector. Spec: https://w3c.github.io/webrtc-stats/#dom-rtcmediastreamtrackstats-framesdropped Implemented as frames_received - frames_rendered. Part of this CL is adding frames_rendered to VideoReceiveStream::Stats and updating it at ReceiveStatisticsProxy::OnRenderedFrame. BUG=webrtc:6757, chromium:659137, chromium:627816 NOTRY=True Review-Url: https://codereview.webrtc.org/2607933002 Cr-Commit-Position: refs/heads/master@{#16213} Committed: https://chromium.googlesource.com/external/webrtc/+/50cfe1fda7f2b4a6a449fe723... ==========
Message was sent while issue was closed.
Committed patchset #5 (id:120001) as https://chromium.googlesource.com/external/webrtc/+/50cfe1fda7f2b4a6a449fe723... |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
