1341533 - framerateMean, framerateStdDev, bitrateMean, and bitrateStdDev are sometimes wild numbers

Status: RESOLVED WONTFIX

(Core :: WebRTC, defect, P3)

Description

[Nico Grunbaum [:ng, @chew:mozilla.org]]

Sometimes framerateMean,  framerateStdDev, bitrateMean, bitrateStdDev are bizarre numbers. Here is a sample from tests (that are not yet in the tree) for 

TEST-UNEXPECTED-FAIL | dom/media/tests/mochitest/test_peerConnection_stats.html | inbound-rtp.framerateMean is sane. value=-2.3847388052893314e+124
TEST-UNEXPECTED-FAIL | dom/media/tests/mochitest/test_peerConnection_stats.html | outbound-rtp.bitrateMean is sane. value=-1.1673608800424e+210
TEST-UNEXPECTED-FAIL | dom/media/tests/mochitest/test_peerConnection_stats.html | outbound-rtp.bitrateStdDev is sane. value=-7.37533859961112e+281
TEST-UNEXPECTED-FAIL | dom/media/tests/mochitest/test_peerConnection_stats.html | outbound-rtp.framerateMean is sane. value=-532093946.8987685
TEST-UNEXPECTED-FAIL | dom/media/tests/mochitest/test_peerConnection_stats.html | outbound-rtp.framerateStdDev is sane. value=-1.4005287384974042e-64

See https://treeherder.mozilla.org/#/jobs?repo=try&revision=45677223211775f4d3c0b170c7489da5c9b8d4f9&selectedJob=79221146 Windows 7 opt, debug. OS X opt, etc.

Comment 1

[Nico Grunbaum [:ng, @chew:mozilla.org]]

Attachment: Bug 1341533 - intiailize framerate and bitrate stats;

Review commit: https://reviewboard.mozilla.org/r/114590/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/114590/

Comment 2

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

Comment on attachment 8840032 [details]
Bug 1341533 - intiailize framerate and bitrate stats;

https://reviewboard.mozilla.org/r/114590/#review116160

I'm gonna go with r- until I hear a reason why GetVideoEncoderStats() can't be fixed instead, as this seems like a wallpaper fix.

::: media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.cpp:3676
(Diff revision 1)
> -            double framerateMean;
> -            double framerateStdDev;
> -            double bitrateMean;
> -            double bitrateStdDev;
> -            uint32_t droppedFrames;
> +            double framerateMean = 0;
> +            double framerateStdDev = 0;
> +            double bitrateMean = 0;
> +            double bitrateStdDev = 0;
> +            uint32_t droppedFrames = 0;
>              if (mp.Conduit()->GetVideoEncoderStats(&framerateMean,
>                                                     &framerateStdDev,
>                                                     &bitrateMean,
>                                                     &bitrateStdDev,
>                                                     &droppedFrames)) {
>                s.mFramerateMean.Construct(framerateMean);
>                s.mFramerateStdDev.Construct(framerateStdDev);
>                s.mBitrateMean.Construct(bitrateMean);
>                s.mBitrateStdDev.Construct(bitrateStdDev);
>                s.mDroppedFrames.Construct(droppedFrames);
>              }

How can GetVideoEncoderStats() succeed without filling in all its out arguments?

If it can, then that sounds like a bug in GetVideoEncoderStats() that should be fixed there, not here.

Comment 3

[Nico Grunbaum [:ng, @chew:mozilla.org]]

Comment on attachment 8840032 [details]
Bug 1341533 - intiailize framerate and bitrate stats;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/114590/diff/1-2/

Comment 4

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

Comment on attachment 8840032 [details]
Bug 1341533 - intiailize framerate and bitrate stats;

https://reviewboard.mozilla.org/r/114590/#review117514

r=me once you fix GetVideoDecoderStats() the same way. It suffers from the exact same problem.

::: media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.cpp:3764
(Diff revision 2)
> -          double framerateMean;
> -          double framerateStdDev;
> -          double bitrateMean;
> -          double bitrateStdDev;
> -          uint32_t discardedPackets;
> +          double framerateMean = 0;
> +          double framerateStdDev = 0;
> +          double bitrateMean = 0;
> +          double bitrateStdDev = 0;
> +          uint32_t discardedPackets = 0;
>            if (mp.Conduit()->GetVideoDecoderStats(&framerateMean,
>                                                   &framerateStdDev,
>                                                   &bitrateMean,
>                                                   &bitrateStdDev,
>                                                   &discardedPackets)) {

Same problem in GetVideoDecoderStats().

Comment 5

[Nico Grunbaum [:ng, @chew:mozilla.org]]

Comment on attachment 8840032 [details]
Bug 1341533 - intiailize framerate and bitrate stats;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/114590/diff/2-3/

Comment 6

[Nico Grunbaum [:ng, @chew:mozilla.org]]

Comment on attachment 8840032 [details]
Bug 1341533 - intiailize framerate and bitrate stats;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/114590/diff/3-4/

Comment 7

[Emma Humphries ☕️🎸🧞‍♀️✨ (she/they) [:emceeaich] (Pacific Time) use needinfo]

This is an assigned P1 bug without activity in two weeks. 

If you intend to continue working on this bug for the current release/iteration/sprint, remove the 'stale-bug' keyword.

Otherwise we'll reset the priority of the bug back to '--' on Monday, August 28th.

Comment 8

[Bulk Bug Changes for mreavy's org]

Mass change P1->P2 to align with new Mozilla triage process

Comment 9

[Sylvestre Ledru [:Sylvestre]]

Moving to p3 because no activity for at least 1 year(s).
See https://github.com/mozilla/bug-handling/blob/master/policy/triage-bugzilla.md#how-do-you-triage for more information

Comment 10

[Mathew Hodson]

All four of these stats were removed from Firefox in bug 1367562.