Closed
Bug 951496
Opened 11 years ago
Closed 10 years ago
WebRTC codec statistics - fps, bit rate and frame drops
Categories
(Core :: WebRTC: Audio/Video, defect, P2)
Core
WebRTC: Audio/Video
Tracking
()
RESOLVED
FIXED
mozilla32
People
(Reporter: slee, Assigned: jib)
References
(Blocks 1 open bug)
Details
(Whiteboard: [p=1])
Attachments
(5 files, 15 obsolete files)
8.06 KB,
patch
|
jib
:
review+
|
Details | Diff | Splinter Review |
12.63 KB,
patch
|
jib
:
review+
|
Details | Diff | Splinter Review |
23.57 KB,
patch
|
jib
:
review+
|
Details | Diff | Splinter Review |
7.77 KB,
patch
|
jib
:
review+
|
Details | Diff | Splinter Review |
17.25 KB,
text/html
|
Details |
We need the statistics data for checking the status of codec.
Attachment #8349152 -
Flags: review?(rjesup)
Reporter | ||
Updated•11 years ago
|
Attachment #8349152 -
Flags: feedback?(cku)
Reporter | ||
Comment 1•11 years ago
|
||
Sorry, I uploaded wrong patch. This is the correct one.
Attachment #8349152 -
Attachment is obsolete: true
Attachment #8349152 -
Flags: review?(rjesup)
Attachment #8349152 -
Flags: feedback?(cku)
Attachment #8349286 -
Flags: review?(rjesup)
Reporter | ||
Updated•11 years ago
|
Flags: needinfo?(cku)
Comment on attachment 8349152 [details] [diff] [review]
webrtcLog.patch
Review of attachment 8349152 [details] [diff] [review]:
-----------------------------------------------------------------
::: media/webrtc/signaling/src/media-conduit/VideoConduit.h
@@ +41,5 @@
> class WebrtcVideoConduit:public VideoSessionConduit
> ,public webrtc::Transport
> ,public webrtc::ExternalRenderer
> + ,public webrtc::ViEEncoderObserver
> + ,public webrtc::ViEDecoderObserver
1. Implement webrtc::ViEEncoderObserver and webrtc::ViEDecoderObserver inherited concrete classes, and aggregate these two observers in WebrtcVideoConduit.
2. Bypass the data you need from WebrtcVideoConduit, e.g. mPtrViECodec, into those observers.
Reporter | ||
Comment 3•11 years ago
|
||
(In reply to C.J. Ku[:CJKu] from comment #2)
> 1. Implement webrtc::ViEEncoderObserver and webrtc::ViEDecoderObserver
> inherited concrete classes, and aggregate these two observers in
> WebrtcVideoConduit.
> 2. Bypass the data you need from WebrtcVideoConduit, e.g. mPtrViECodec, into
> those observers.
ViEEncoderObserver and ViEDecoderObserver are 2 pure virtual classes, why not just inherited them? There should be only few overhead. Implementing them as objects, we need extra-memory to save them and the related pointer, such as mPtrViECodec.
(In reply to StevenLee[:slee] from comment #3)
> (In reply to C.J. Ku[:CJKu] from comment #2)
> > 1. Implement webrtc::ViEEncoderObserver and webrtc::ViEDecoderObserver
> > inherited concrete classes, and aggregate these two observers in
> > WebrtcVideoConduit.
> > 2. Bypass the data you need from WebrtcVideoConduit, e.g. mPtrViECodec, into
> > those observers.
> ViEEncoderObserver and ViEDecoderObserver are 2 pure virtual classes, why
> not just inherited them? There should be only few overhead. Implementing
> them as objects, we need extra-memory to save them and the related pointer,
> such as mPtrViECodec.
I don't think it's a good idea to put whole thing into a single class. It's not relative to memory overhead or any implementation limitation, it's just a design logic: keep a single object handle as less things as possible
Flags: needinfo?(cku)
Reporter | ||
Comment 5•11 years ago
|
||
After discussing with CJ, I added some more statistics data, standard deviation, minimum and maximum values.
Attachment #8349286 -
Attachment is obsolete: true
Attachment #8349286 -
Flags: review?(rjesup)
Updated•11 years ago
|
Blocks: 964161
Summary: WbeRTC codec statistics - fps, bit rate and frame drops → WebRTC codec statistics - fps, bit rate and frame drops
Updated•11 years ago
|
Assignee: nobody → slee
Updated•11 years ago
|
Updated•11 years ago
|
Assignee: slee → jib
Updated•11 years ago
|
Priority: -- → P2
Whiteboard: [p=2]
Target Milestone: --- → mozilla32
Comment 6•10 years ago
|
||
Isn't this going into the v2.0 release? if so, I think we should start tracking this.
Comment 7•10 years ago
|
||
We are tracking this as part of the WebRTC Platform release for Fx 32 (which aligns with v2.0). Please see the following if you want more details: https://wiki.mozilla.org/Media/WebRTC#Sprint:_mozilla32_WebRTC_Platform
Assignee | ||
Comment 8•10 years ago
|
||
Unbitrotted.
- Added mDroppedFrames and mDiscardedPackets (both accumulators).
Notes to self: This patch gathers
- encoder outgoing framerate, bitrate and dropped frames
- incoming framerate, bitrate, and discarded packets
For the rate-values, it calculates min, max, mean, std.deviation, and stores history. I have some concern about this scaling and am looking into calculating std.deviation without needing to hold captured values forever.
We should also figure out which of theses values to expose to stats and telemetry (all of them?)
Flags: needinfo?(rjesup)
Assignee | ||
Updated•10 years ago
|
Attachment #8350551 -
Attachment is obsolete: true
Comment 9•10 years ago
|
||
Comment on attachment 8431097 [details] [diff] [review]
Part 1: statistics data for checking the status of codec (2)
Review of attachment 8431097 [details] [diff] [review]:
-----------------------------------------------------------------
::: media/webrtc/signaling/src/media-conduit/CodecStatistics.cpp
@@ +1,1 @@
> +/* This Source Code Form is subject to the terms of the Mozilla Public
mode lines per style guide
@@ +2,5 @@
> + * License, v. 2.0. If a copy of the MPL was not distributed with this file,
> + * You can obtain one at http://mozilla.org/MPL/2.0/. */
> +
> +#include "CSFLog.h"
> +#include "CodecStatistics.h"
swap order here per style guide
@@ +15,5 @@
> + mChannel(channel),
> + mSentRawFrames(0),
> + mPtrViECodec(codec),
> + mEncoderDroppedFrames(0),
> + mDecoderDiscardedPackets(0) {
{ on left edge (for all functions)
@@ +16,5 @@
> + mSentRawFrames(0),
> + mPtrViECodec(codec),
> + mEncoderDroppedFrames(0),
> + mDecoderDiscardedPackets(0) {
> + mPtrViECodec->RegisterEncoderObserver(mChannel, *this);
Add MOZ_ASSERT(mPtrViECodec);
@@ +26,5 @@
> + mPtrViECodec->DeregisterDecoderObserver(mChannel);
> +}
> +
> +void VideoCodecStatistics::OutgoingRate(const int video_channel,
> + const unsigned int framerate, const unsigned int bitrate) {
indents
@@ +27,5 @@
> +}
> +
> +void VideoCodecStatistics::OutgoingRate(const int video_channel,
> + const unsigned int framerate, const unsigned int bitrate) {
> + unsigned int keyFrames, deltaFrames;
concrete size (uint32_t)
@@ +28,5 @@
> +
> +void VideoCodecStatistics::OutgoingRate(const int video_channel,
> + const unsigned int framerate, const unsigned int bitrate) {
> + unsigned int keyFrames, deltaFrames;
> + mPtrViECodec->GetSendCodecStastistics(video_channel, keyFrames, deltaFrames);
Typo (Statistics)
@@ +31,5 @@
> + unsigned int keyFrames, deltaFrames;
> + mPtrViECodec->GetSendCodecStastistics(video_channel, keyFrames, deltaFrames);
> + uint32_t dropped = mSentRawFrames - (keyFrames + deltaFrames);
> + CSFLogDebug(logTag,
> + "encoder statistics - framerate: %d, bitrate: %d, dropped frames: %d",
Indent; Probably all %u's
@@ +39,5 @@
> + mEncoderDroppedFrames += dropped;
> +}
> +
> +void VideoCodecStatistics::IncomingCodecChanged(const int video_channel,
> + const webrtc::VideoCodec& video_codec) {
indent
@@ +40,5 @@
> +}
> +
> +void VideoCodecStatistics::IncomingCodecChanged(const int video_channel,
> + const webrtc::VideoCodec& video_codec) {
> + CSFLogDebug(logTag, "channel %d change codec to %d ", video_channel, video_codec);
(int) video_codec perhaps
@@ +44,5 @@
> + CSFLogDebug(logTag, "channel %d change codec to %d ", video_channel, video_codec);
> +}
> +
> +void VideoCodecStatistics::IncomingRate(const int video_channel,
> + const unsigned int framerate, const unsigned int bitrate) {
indent
@@ +45,5 @@
> +}
> +
> +void VideoCodecStatistics::IncomingRate(const int video_channel,
> + const unsigned int framerate, const unsigned int bitrate) {
> + unsigned int discarded = mPtrViECodec->GetDiscardedPackets(video_channel);
unsigned int is correct here
@@ +48,5 @@
> + const unsigned int framerate, const unsigned int bitrate) {
> + unsigned int discarded = mPtrViECodec->GetDiscardedPackets(video_channel);
> + CSFLogDebug(logTag,
> + "decoder statistics - framerate: %d, bitrate: %d, discarded packets %d",
> + framerate, bitrate, discarded);
Indent; %u %u %u
::: media/webrtc/signaling/src/media-conduit/CodecStatistics.h
@@ +1,1 @@
> +/* This Source Code Form is subject to the terms of the Mozilla Public
add mode lines per style guide
@@ +7,5 @@
> +#include "nsISupportsImpl.h"
> +
> +#include "webrtc/video_engine/include/vie_codec.h"
> +
> +#include <math.h>
<> includes before mozilla includes unless you can't for some reason
@@ +9,5 @@
> +#include "webrtc/video_engine/include/vie_codec.h"
> +
> +#include <math.h>
> +
> +class webrtc::ViECodec;
Either remove this, or remove the include of vie_codec.h
@@ +12,5 @@
> +
> +class webrtc::ViECodec;
> +
> +namespace mozilla {
> +class Statistics {
Feels like this should live somewhere else (Telemetry? mfbt?), and be a template (allowing more than just uint32_t data to be accumulated). At least add a comment to that effect, but I'd prefer it simply move into it's own header file.
@@ +18,5 @@
> + Statistics() : mMin(0)
> + ,mMax(0)
> + ,mSum(0)
> + {
> + }
{}
@@ +25,5 @@
> + if (mData.Length() == 0) {
> + mMin = data;
> + mMax = data;
> + }
> + mData.AppendElement(data);
This means it will hold an infinite amount of data, just in order to calculate StandardDeviation. Probably not a great tradeoff, unless we know the size is bounded and/or will not be held for long.
I suggest http://en.wikipedia.org/wiki/Algorithms_for_calculating_variance#Incremental_Algorithm
@@ +38,5 @@
> +
> + mSum += data;
> + }
> +
> + float Mean() {
const
@@ +42,5 @@
> + float Mean() {
> + return (float)(mSum/mData.Length());
> + }
> +
> + float StandardDeviation() {
const
@@ +45,5 @@
> +
> + float StandardDeviation() {
> + float mean = Mean();
> + float variance = 0.0;
> + for (uint32_t i = 0; i < mData.Length(); i++) {
see above about algorithm
@@ +51,5 @@
> + }
> + return sqrtf(variance/mData.Length());
> + }
> +
> + uint32_t Min() {
const
@@ +55,5 @@
> + uint32_t Min() {
> + return mMin;
> + }
> +
> + uint32_t Max() {
const
@@ +65,5 @@
> + uint32_t mMax;
> + uint32_t mSum;
> +};
> +
> +class VideoCodecStatistics : public webrtc::ViEEncoderObserver
comments?
@@ +71,5 @@
> +{
> +public:
> + VideoCodecStatistics(int channel, webrtc::ViECodec* vieCodec);
> + ~VideoCodecStatistics();
> + void InsertOneFrame();
SentFrame() perhaps? InsertOneFrame is kinda the wrong name...
@@ +73,5 @@
> + VideoCodecStatistics(int channel, webrtc::ViECodec* vieCodec);
> + ~VideoCodecStatistics();
> + void InsertOneFrame();
> + virtual void OutgoingRate(const int video_channel,
> + const unsigned int framerate, const unsigned int bitrate);
be consistent about indents (in this case, have it line up with previous argument)
@@ +84,5 @@
> + const unsigned int bitrate);
> +
> + virtual void RequestNewKeyFrame(const int video_channel);
> + void Dump();
> + NS_INLINE_DECL_REFCOUNTING(VideoCodecStatistics);
move up to after ~foo()
However, I don't see this as needed; it lives in one struct (VideoConduit); and it could be an nsAutoPtr<> there instead of a nsRefPtr<>
::: media/webrtc/signaling/src/media-conduit/VideoConduit.cpp
@@ +28,5 @@
> #include <algorithm>
> #include <math.h>
>
> +using namespace webrtc;
> +
Let's not add this for this patch; this file has lots of webrtc::'s in it already
@@ +119,5 @@
> mVideoEngine = nullptr;
> } else {
> // We can't delete the VideoEngine until all these are released!
> // And we can't use a Scoped ptr, since the order is arbitrary
> + mVideoCodecStat = nullptr;
This is slightly different than the rest; put it before the comment with it's own comment about releasing reference to the ViECodec - or even better perhaps right before setting mPtrViECodec to nullptr.
Assignee | ||
Comment 10•10 years ago
|
||
> > +void VideoCodecStatistics::IncomingCodecChanged(const int video_channel,
> > + const webrtc::VideoCodec& video_codec) {
> > + CSFLogDebug(logTag, "channel %d change codec to %d ", video_channel, video_codec);
>
> (int) video_codec perhaps
I'll do video_codec.mPName.
> > +class Statistics {
>
> Feels like this should live somewhere else (Telemetry? mfbt?), and be a
> template (allowing more than just uint32_t data to be accumulated). At
> least add a comment to that effect, but I'd prefer it simply move into it's
> own header file.
Discussed with jesup and keeping it here for now while we adapt it for our needs.
> This means it will hold an infinite amount of data, just in order to
> calculate StandardDeviation. Probably not a great tradeoff, unless we know
> the size is bounded and/or will not be held for long.
>
> I suggest
> http://en.wikipedia.org/wiki/
> Algorithms_for_calculating_variance#Incremental_Algorithm
This was my concern as well, I'm looking into this. Agree with all remaining comments. Thanks!
Flags: needinfo?(rjesup)
Assignee | ||
Comment 11•10 years ago
|
||
Broke out typo fix into separate patch.
Attachment #8433628 -
Flags: review?(rjesup)
Assignee | ||
Comment 12•10 years ago
|
||
Unbitrot and updated to work with 3.50. Addressed most feedback, except:
Working on adding MovingAverage and a Standard deviation without accumulating history next.
Attachment #8431097 -
Attachment is obsolete: true
Updated•10 years ago
|
Attachment #8433628 -
Flags: review?(rjesup) → review+
Assignee | ||
Comment 13•10 years ago
|
||
- Replaced Statistics class with RunningStat from
http://www.johndcook.com/standard_deviation.html
- Dropping min/max. Doing doing mean and std deviation.
I hope this algorithm is good enough for both on-demand stats and PER_CALL telemetry at the end.
Working on tying it to our getStats code in part 3.
Attachment #8433631 -
Attachment is obsolete: true
Attachment #8434451 -
Flags: review?(rjesup)
Assignee | ||
Comment 14•10 years ago
|
||
Whoops, wrong patch. This is the right one.
Attachment #8434451 -
Attachment is obsolete: true
Attachment #8434451 -
Flags: review?(rjesup)
Attachment #8434454 -
Flags: review?(rjesup)
Comment 15•10 years ago
|
||
Comment on attachment 8434454 [details] [diff] [review]
Part 2: statistics data for checking the status of codec (5)
Review of attachment 8434454 [details] [diff] [review]:
-----------------------------------------------------------------
r+ but will need license/copyright clarification before landing, or recoding the algorithm.
::: media/webrtc/signaling/src/media-conduit/CodecStatistics.cpp
@@ +11,5 @@
> +
> +// use the same tag as VideoConduit
> +static const char* logTag ="WebrtcVideoSessionConduit";
> +
> +void VideoCodecStatistics::RunningStat::Push(double x)
Move to file...
@@ +14,5 @@
> +
> +void VideoCodecStatistics::RunningStat::Push(double x)
> +{
> + mN++;
> +
spaces (here and later)
::: media/webrtc/signaling/src/media-conduit/CodecStatistics.h
@@ +15,5 @@
> +
> +class VideoCodecStatistics : public webrtc::ViEEncoderObserver
> + , public webrtc::ViEDecoderObserver
> +{
> + // From "Accurately computing running variance - John D. Cook"
Include URL & copyright status
Should be broken into a separate file if not pure PD. even then, probably.
@@ +78,5 @@
> + int current_delay_ms,
> + int target_delay_ms,
> + int jitter_buffer_ms,
> + int min_playout_delay_ms,
> + int render_delay_ms) OVERRIDE {}
MOZ_OVERRIDE
@@ +83,5 @@
> + void Dump();
> +private:
> + int mChannel;
> + uint32_t mSentRawFrames;
> + webrtc::ViECodec* mPtrViECodec;
Ok, we need to be careful with these. Raw ptrs == dangerous. ScopedCustomReleasePtr?
::: media/webrtc/signaling/src/media-conduit/VideoConduit.cpp
@@ +120,5 @@
> // We can't delete the VideoEngine until all these are released!
> // And we can't use a Scoped ptr, since the order is arbitrary
> mPtrViEBase = nullptr;
> mPtrViECapture = nullptr;
> + mVideoCodecStat = nullptr; // has ptr to mPtrViECodec
move this to one end or the other; out of the list of ScopedCustomReleasePtrs
More explicit comment as to why we have to null it here.
Attachment #8434454 -
Flags: review?(rjesup) → review+
Assignee | ||
Comment 16•10 years ago
|
||
Updated commit msg.
Attachment #8433628 -
Attachment is obsolete: true
Attachment #8434671 -
Flags: review+
Assignee | ||
Comment 17•10 years ago
|
||
Incorporated all feedback. Carrying forward r=jesup.
Attachment #8434454 -
Attachment is obsolete: true
Attachment #8434673 -
Flags: review+
Assignee | ||
Comment 18•10 years ago
|
||
Work in progress.
- Encoder settings work. Decoder side not appearing yet (not inited properly)
Next is hooking up about:webrtc and PER_CALL telemetry.
Assignee | ||
Comment 19•10 years ago
|
||
Comment 20•10 years ago
|
||
Comment on attachment 8435321 [details] [diff] [review]
Part 3: codec getStats WIP
Review of attachment 8435321 [details] [diff] [review]:
-----------------------------------------------------------------
Overall looks good; will need DOM peer of course
::: media/webrtc/signaling/src/media-conduit/AudioConduit.h
@@ +181,5 @@
> + bool GetVideoEncoderStats(double* framerateMean,
> + double* framerateStdDev,
> + double* bitrateMean,
> + double* bitrateStdDev,
> + uint32_t* droppedFrames);
Just { return false; } here for not-relevant base-class funcs
Comment 21•10 years ago
|
||
Comment on attachment 8434671 [details] [diff] [review]
Part 1: fix Stastistics typo in vie_codec (2) r=jesup
Review of attachment 8434671 [details] [diff] [review]:
-----------------------------------------------------------------
This patch should be submitted to upstream (after uplift)
Updated•10 years ago
|
Blocks: webrtc_upstream_bugs
Assignee | ||
Comment 22•10 years ago
|
||
- Works. Had to put a stent in the VideoStatistics constructor to not
register both encoder and decoder observer unless needed, as it was
competing with other instances for this single-observer callback.
Hopefully not too ugly.
- Added to about:webrtc
- Webidl needs dom review (should be trivial, just dictionary changes).
Attachment #8435321 -
Attachment is obsolete: true
Attachment #8436219 -
Flags: review?(rjesup)
Attachment #8436219 -
Flags: review?(bugs)
Assignee | ||
Comment 23•10 years ago
|
||
Attachment #8435323 -
Attachment is obsolete: true
Updated•10 years ago
|
Attachment #8436219 -
Flags: review?(rjesup) → review+
Assignee | ||
Comment 24•10 years ago
|
||
That should be it. Try - https://tbpl.mozilla.org/?tree=Try&rev=52291a4eeab4
Attachment #8436269 -
Flags: review?(rjesup)
Comment 25•10 years ago
|
||
Comment on attachment 8436269 [details] [diff] [review]
Part 4: codec telemetry
Review of attachment 8436269 [details] [diff] [review]:
-----------------------------------------------------------------
::: toolkit/components/telemetry/Histograms.json
@@ +5340,5 @@
> },
> + "WEBRTC_VIDEO_ENCODER_BITRATE_AVG_PER_CALL_KBPS": {
> + "expires_in_version": "never",
> + "kind": "exponential",
> + "high": 100000,
Is this too high? Highest I expect is 2Mbps (2000 here, low will be 200 typ; perhaps 100ish if a drop the lower bound a bit); of course it's exponential... Perhaps this is fine.
Attachment #8436269 -
Flags: review?(rjesup) → review+
Comment 26•10 years ago
|
||
Comment on attachment 8436219 [details] [diff] [review]
Part 3: codec getStats (2)
Review of attachment 8436219 [details] [diff] [review]:
-----------------------------------------------------------------
Why does this violate our no-vendor-prefixes policy?
Attachment #8436219 -
Flags: feedback-
Assignee | ||
Comment 27•10 years ago
|
||
(In reply to :Ms2ger from comment #26)
> Why does this violate our no-vendor-prefixes policy?
As discussed on IRC and per https://wiki.mozilla.org/WebAPI/ExposureGuidelines I'll change the patch to make these additions chrome-only for now, so we can land it for about:webrtc and telemetry.
Assignee | ||
Comment 28•10 years ago
|
||
- Made new moz-prefixed stats dictionary members internal-only for now.
- I'll take the first DOM review I can get. ;-)
Attachment #8436219 -
Attachment is obsolete: true
Attachment #8436219 -
Flags: review?(bugs)
Attachment #8436318 -
Flags: review?(peterv)
Attachment #8436318 -
Flags: review?(bugs)
Assignee | ||
Comment 29•10 years ago
|
||
Addressed nits. Carrying forward r=jesup.
- Reduced bitrate ceilings in histogram by an order of 10.
Attachment #8436269 -
Attachment is obsolete: true
Attachment #8436320 -
Flags: review+
Comment 30•10 years ago
|
||
Comment on attachment 8436219 [details] [diff] [review]
Part 3: codec getStats (2)
un-obsoleting for further consideration
Attachment #8436219 -
Attachment is obsolete: false
Comment 31•10 years ago
|
||
Comment on attachment 8436219 [details] [diff] [review]
Part 3: codec getStats (2)
Reinstating r? to smaug
Per discussion in #content, this stats interface is under active development in the WG and both we and google are experimenting with uses. It lives behind a moz-prefixed API (mozRTCPeerConnection), so this doesn't expose a new moz API to the web. Also, per the discussion in SpecialCases in the policy, this is important to FxOS apps being developed, where about:webrtc isn't practical to use or ask users to use and cut-and-paste (and apps want to report back bandwidth and framerates).
Attachment #8436219 -
Flags: review?(bugs)
Updated•10 years ago
|
Attachment #8436219 -
Flags: review?(peterv)
Comment 32•10 years ago
|
||
Comment on attachment 8436219 [details] [diff] [review]
Part 3: codec getStats (2)
I don't really see reasons to use prefixes here.
If we think the API is stable enough, just add non-prefixed stuff, and in the
other case we need to be ready to change the API anyway, whether or not
we have prefixes.
(We should try hard to not add more prefixed stuff to web, and
mozRTCPeerConnection is already prefixed. So if prefixes bring in anything good,
that should be enough.)
Attachment #8436219 -
Flags: review?(peterv)
Attachment #8436219 -
Flags: review?(bugs)
Attachment #8436219 -
Flags: review-
Updated•10 years ago
|
Attachment #8436318 -
Flags: review?(peterv)
Attachment #8436318 -
Flags: review?(bugs)
Assignee | ||
Comment 33•10 years ago
|
||
Got r+ from smaug on IRC in #developers an hour ago. Carrying forward r+ from jesup.
- Removed 'moz' prefixes from new stats dictionary members.
- Removed chrome-only from new stats dictionary members.
Attachment #8436219 -
Attachment is obsolete: true
Attachment #8436318 -
Attachment is obsolete: true
Attachment #8436383 -
Flags: review+
Assignee | ||
Comment 34•10 years ago
|
||
Unbitrot (removed 'moz' prefixes from new stats). Carrying forward r+ from jesup.
Attachment #8436320 -
Attachment is obsolete: true
Attachment #8436384 -
Flags: review+
Assignee | ||
Comment 35•10 years ago
|
||
Unbitrot (removed 'moz' prefixes from new stats).
Attachment #8436220 -
Attachment is obsolete: true
Assignee | ||
Updated•10 years ago
|
Keywords: checkin-needed
Whiteboard: [p=2] → [p=1]
Comment 36•10 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/99dc3a6c8432
https://hg.mozilla.org/integration/mozilla-inbound/rev/6dd8777a4453
https://hg.mozilla.org/integration/mozilla-inbound/rev/e30a256cb1fc
https://hg.mozilla.org/integration/mozilla-inbound/rev/b8f0896469c7
Keywords: checkin-needed
Comment 38•10 years ago
|
||
ms2ger: see comment 33 - verbal r+ from smaug on IRC given removing the moz prefixes; he didn't go back and update the bug
Flags: needinfo?(jib)
Comment 39•10 years ago
|
||
well, r+ patch with moz-prefix would be tiny bit odd.
But my understanding was that we're ok exposing the API as such, and prefixes aren't needed.
So, r+.
Comment 40•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/99dc3a6c8432
https://hg.mozilla.org/mozilla-central/rev/6dd8777a4453
https://hg.mozilla.org/mozilla-central/rev/e30a256cb1fc
https://hg.mozilla.org/mozilla-central/rev/b8f0896469c7
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Updated•6 years ago
|
No longer blocks: webrtc_upstream_bugs
You need to log in
before you can comment on or make changes to this bug.
Description
•