Chromium Code Reviews

Issue 1420963005: Add QP stats for received video streams to StatsReport.

Created:
5 years, 2 months ago by åsapersson
Modified:
4 years, 7 months ago
Reviewers:
CC:
webrtc-reviews_webrtc.org, interface-changes_webrtc.org, rwolff_gocast.it, yujie_mao (webrtc), Andrew MacDonald, stefan-webrtc, tterriberry_mozilla.com, qiang.lu, niklas.enbom, peah-webrtc, the sun, perkj_webrtc, andresp
Base URL:
https://chromium.googlesource.com/external/webrtc.git@master
Target Ref:
refs/pending/heads/master
Project:
webrtc
Visibility:
Public.

Description

Add QP stats for received video streams to StatsReport for tracking video quality. A running average of the QP (per-frame value) for a received VP8 video stream is reported. QP range is 0-127. BUG=

Patch Set 1 : #

Total comments: 1

Patch Set 2 : filter qp #

Patch Set 3 : add unit test #

Unified diffs Side-by-side diffs Stats (+33 lines, -2 lines)
M talk/app/webrtc/statscollector.cc View 1 chunk +1 line, -0 lines 0 comments
M talk/app/webrtc/statstypes.h View 1 chunk +1 line, -0 lines 0 comments
M talk/app/webrtc/statstypes.cc View 1 chunk +2 lines, -0 lines 0 comments
M talk/media/base/mediachannel.h View 2 chunks +4 lines, -1 line 0 comments
M talk/media/webrtc/webrtcvideoengine2.cc View 1 chunk +1 line, -0 lines 0 comments
M talk/media/webrtc/webrtcvideoengine2_unittest.cc View 1 chunk +11 lines, -0 lines 0 comments
M webrtc/video/receive_statistics_proxy.h View 2 chunks +2 lines, -0 lines 0 comments
M webrtc/video/receive_statistics_proxy.cc View 3 chunks +9 lines, -1 line 0 comments
M webrtc/video_receive_stream.h View 1 chunk +2 lines, -0 lines 0 comments

Messages

Total messages: 23 (17 generated)
åsapersson
5 years, 2 months ago (2015-10-23 12:39:30 UTC) #6
perkj_webrtc
I have not reviewed the whole cl. https://codereview.webrtc.org/1420963005/diff/40001/talk/app/webrtc/statstypes.cc File talk/app/webrtc/statstypes.cc (right): https://codereview.webrtc.org/1420963005/diff/40001/talk/app/webrtc/statstypes.cc#newcode560 talk/app/webrtc/statstypes.cc:560: return "googQpReceived"; ...
5 years, 1 month ago (2015-10-27 20:50:35 UTC) #8
mflodman
On 2015/10/27 20:50:35, perkj1 wrote: > I have not reviewed the whole cl. > > ...
5 years, 1 month ago (2015-10-28 07:16:03 UTC) #9
åsapersson
Adding hta as a reviewer.
5 years, 1 month ago (2015-10-28 12:17:31 UTC) #12
hta-webrtc
General guidance is to not add more goog* stats. Instead, what we should do is: ...
5 years, 1 month ago (2015-10-29 14:50:54 UTC) #13
åsapersson
5 years ago (2015-11-25 12:39:52 UTC) #21 
On 2015/10/29 14:50:54, hta-webrtc wrote:
> General guidance is to not add more goog* stats.
> Instead, what we should do is:
> - Make the case for collecting this stat
> - Figure out a name we can live with if everyone implements it
> - Publish a proposal for that stats value
> - Implement
> 
> We don't want to promise to wait for acceptance before we implement, but we
want
> the proposal to be public.
> 
> I have hesitations about this one. QP is a per-frame value, and is strongly
> influenced by rate control; spot-picking QP values from "the last frame that
was
> received before getStats was called" seems like it's collecting a number
rather
> than collecting something that's useful for statistics.
> 
> What's the use case that drives this proposal?

I have updated the CL so a running average of the QP is reported to GetStats.

The stats would correspond to:
The quantizer value used (running average of per-frame value) for encoding the
sent video stream (the highest resolution stream is reported). The QP range is
0-127 for VP8.

The quantizer value used (running average of per-frame value) for encoding the
received video stream. The QP range is 0-127 for VP8.

Powered by Google App Engine