-
Notifications
You must be signed in to change notification settings - Fork 477
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Report encode/decode time and implementation metric #2434
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,7 +2,7 @@ | |
// SPDX-License-Identifier: Apache-2.0 | ||
|
||
import Logger from '../logger/Logger'; | ||
import { SdkMetric } from '../signalingprotocol/SignalingProtocol.js'; | ||
import { SdkMetric, SdkStreamDimension } from '../signalingprotocol/SignalingProtocol.js'; | ||
import VideoStreamIndex from '../videostreamindex/VideoStreamIndex'; | ||
import Direction from './ClientMetricReportDirection'; | ||
import MediaType from './ClientMetricReportMediaType'; | ||
|
@@ -126,6 +126,35 @@ export default class ClientMetricReport { | |
return Number(metricReport.currentMetrics[metricName] * 1000); | ||
}; | ||
|
||
millisecondsPerSecond = (metricName?: string, ssrc?: number): number => { | ||
const metricReport = ssrc ? this.streamMetricReports[ssrc] : this.globalMetricReport; | ||
let intervalSeconds = (this.currentTimestampMs - this.previousTimestampMs) / 1000; | ||
if (intervalSeconds <= 0) { | ||
return 0; | ||
} | ||
if (this.previousTimestampMs <= 0) { | ||
intervalSeconds = 1; | ||
} | ||
const diff = | ||
metricReport.currentMetrics[metricName] - (metricReport.previousMetrics[metricName] || 0); | ||
if (diff <= 0) { | ||
return 0; | ||
} | ||
return (diff * 1000) / intervalSeconds; | ||
}; | ||
|
||
isHardwareImplementation = (metricName?: string, ssrc?: number): number => { | ||
const metricReport = this.streamMetricReports[ssrc]; | ||
const implName = String(metricReport.currentStringMetrics[metricName]); | ||
const hasHwName = | ||
implName.includes('ExternalDecoder') || | ||
implName.includes('ExternalEncoder') || | ||
implName.includes('EncodeAccelerator') || | ||
implName.includes('DecodeAccelerator'); | ||
const isFallback = implName.includes('fallback from'); | ||
return hasHwName && !isFallback ? 1 : 0; | ||
}; | ||
|
||
/** | ||
* Canonical and derived metric maps | ||
*/ | ||
|
@@ -252,6 +281,14 @@ export default class ClientMetricReport { | |
jitter: { | ||
transform: this.secondsToMilliseconds, | ||
}, | ||
totalEncodeTime: { | ||
transform: this.millisecondsPerSecond, | ||
type: SdkMetric.Type.VIDEO_ENCODE_MS, | ||
}, | ||
encoderImplementation: { | ||
transform: this.isHardwareImplementation, | ||
type: SdkMetric.Type.VIDEO_ENCODER_IS_HARDWARE, | ||
}, | ||
}; | ||
|
||
readonly videoDownstreamMetricMap: { | ||
|
@@ -261,7 +298,6 @@ export default class ClientMetricReport { | |
source?: string; | ||
}; | ||
} = { | ||
totalDecodeTime: { transform: this.identityValue, type: SdkMetric.Type.VIDEO_DECODE_MS }, | ||
packetsReceived: { transform: this.countPerSecond, type: SdkMetric.Type.VIDEO_RECEIVED_PPS }, | ||
packetsLost: { | ||
transform: this.packetLossPercent, | ||
|
@@ -297,6 +333,14 @@ export default class ClientMetricReport { | |
}, | ||
frameHeight: { transform: this.identityValue, type: SdkMetric.Type.VIDEO_DECODE_HEIGHT }, | ||
frameWidth: { transform: this.identityValue, type: SdkMetric.Type.VIDEO_DECODE_WIDTH }, | ||
totalDecodeTime: { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. should this name be changed? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This key is just about the field name in webrtc report where the metric is derived from, we cannot change that naming as it comes from webrtc.. |
||
transform: this.millisecondsPerSecond, | ||
type: SdkMetric.Type.VIDEO_DECODE_MS, | ||
}, | ||
decoderImplementation: { | ||
transform: this.isHardwareImplementation, | ||
type: SdkMetric.Type.VIDEO_DECODER_IS_HARDWARE, | ||
}, | ||
}; | ||
|
||
getMetricMap( | ||
|
@@ -329,6 +373,22 @@ export default class ClientMetricReport { | |
} | ||
} | ||
|
||
/** | ||
* Dimensions derived from metric | ||
*/ | ||
readonly streamDimensionMap: { | ||
[id: string]: SdkStreamDimension.Type; | ||
} = { | ||
encoderImplementation: SdkStreamDimension.Type.VIDEO_ENCODER_NAME, | ||
decoderImplementation: SdkStreamDimension.Type.VIDEO_DECODER_NAME, | ||
}; | ||
|
||
getStreamDimensionMap(): { | ||
[id: string]: SdkStreamDimension.Type; | ||
} { | ||
return this.streamDimensionMap; | ||
} | ||
|
||
/** | ||
* media Stream metrics | ||
*/ | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we name this to be clearer what it does? milliseconds per second is kinda confusing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the comment. Created #2443 to update function naming