Client report #385

lukasIO · 2023-05-22T10:34:30Z

No description provided.

…o lukas/client-report

livekit_models.proto

davidzhao · 2023-05-23T05:53:32Z

livekit_models.proto

+
+  message RTCSenderStats {
+    // none, cpu, bandwidth, or other
+    string qualityLimitationReason = 1;


I think quality limitation might be more useful to keep track as events.. since there's a clear start & finish point (and a clear cause). The webrtc-internals display is only reflecting current state, so sending it as a stat would require us to send the entire payload whenever that field is detected to have changed.

we have qualitylimitationDurations directly from the stats, without dedicated events and only reading from stats, I guess that's the best we can do for now. We'll have the current limitation reason regularly whenever we send a report.

davidzhao · 2023-05-23T05:55:10Z

livekit_models.proto

+  }
+
+  message RTCReceiverStats {
+    int32 jitter = 1;


is this looking at total jitter?

~~it was meant as a snapshot. but maybe makes sense to sample multiple values over time until the next report gets sent out?~~

it's total jitter as defined here

livekit_models.proto

davidzhao · 2023-05-23T05:57:22Z

livekit_models.proto

+  }
+
+  message DeviceInfo {
+    string ua = 1;


ua is already captured by the server during the initial session establishment (even though we only store a parsed version of it). since concurrency is unlikely to change, could that also be passed up at the same time?

makes sense.
thinking more about this, it seems to me like DeviceInfo would also conceptually be a one-off message as it won't change over time. Even if we include other properties here, those will be hardware bound, so adding them one time when connection establishes probably makes the most sense.
Do you agree to remove this entirely from here and rather add concurrency to the already existing ClientInfo?

livekit_models.proto

…o lukas/client-report

davidzhao · 2023-08-07T06:22:15Z

livekit_models.proto

+
+  message RTCAudioReceiverStats {
+    RTCReceiverStats common_stats = 1;
+    uint32 concealed_samples = 2;


I think we'll also need total_samples for concealed samples to be useful.

davidzhao · 2023-08-07T06:24:42Z

livekit_models.proto

+  message RTCVideoSenderStats {
+   RTCSenderStats common_stats = 1;
+    // outbound-rtp
+    map<string, uint32> quality_limitation_durations = 2;


I'm not sure if webrtc-internals tracks this differently between layers. for some reasons I remember the duration being the same across layers.

Is RTCVideoSenderStats per track? or per simulcast layer?

it's per sender, which should be equivalent to per track.
I think you're right, that it will not track different layers, but how would we do that in the first place?

Regarding the type, it's a map that includes reason and duration and not different layers.

davidzhao · 2023-08-07T06:25:32Z

livekit_models.proto

+    // local-candidate
+    string networkType = 3;
+    // outbound-rtp
+    uint64 packetsSent = 4;


would suggest snake_case for field names.

though I'm not sure if network level stats would be helpful, since we do get these on the server side. For v1, let's start with minimal stats?

missed snake case here. Will remove packets_sent entirely then.

would you also want to exclude nack_count for now as we see that on the server, too?

davidzhao · 2023-08-07T06:26:20Z

livekit_models.proto

 enum SubscriptionError {
  SE_UNKNOWN = 0;
  SE_CODEC_UNSUPPORTED = 1;
  SE_TRACK_NOTFOUND = 2;
 }
+
+message ClientReport {


perhaps we should move this outside of livekit_models? since it's used by a very specific request.. maybe into RoomService?

will move it to RoomService as a last step, so that keeping track of the discussions is easier until then

davidzhao · 2023-08-07T06:44:33Z

livekit_models.proto

+
+  message RTCSenderStats {
+    // ice-candidate-pair
+    uint32 available_outgoing_bitrate = 1;


this seems like at the transport layer, and doesn't fit too well within each sender

makes sense, is your suggestion to leave it out entirely or to move it to a higher level, that's not sender related?

davidzhao · 2023-08-07T06:45:22Z

livekit_models.proto

+    string quality_limitation_reason = 3;
+    uint32 quality_limitation_resolution_changes = 4;
+    string scalability_mode = 5;
+    uint32 frame_height = 6;


which layer would dimension stats be referencing? I think we also already have these pieces of data on the server side.

the original dimension. Makes sense to omit them, if we have them already.

davidzhao · 2023-08-07T06:52:26Z

livekit_models.proto

+  message VideoPublicationReport {
+    string sid = 1;
+    RTCVideoSenderStats rtc_stats = 2;
+    int32 ready_state = 3;


trying to understand these a bit:

ready_state - I think this is just live or ended. do we expect to be ended?

label - this seems a better fit as an event, since we do not expect this to change?

muted - is this tracking MediaStreamTrack.muted property ? i.e. has data, or is it looking at our mute property? would be good to use a diff name here if it's former.

we don't expect it to be ended, but it might be good debugging insight to know that for sure?

not sure why I wanted to include this in the first place, I guess we can drop it for v1 at least

it's looking at our mute property.

davidzhao · 2023-08-07T06:53:29Z

livekit_models.proto

+    int32 ready_state = 3;
+    SubscriptionStatus subscription_status = 4;
+    bool allowed = 5;
+    uint32 width = 6;


I think this should move into RTCVideoReceiverStats ? since the width and height of receiver can change dynamically.

though I'm also not sure what value it is to keep this on the client.. since the server knows exactly what width/height is sent down

you're right, we would have dimension in RTC stats as well. Seems like leaving them out is the way to go!

davidzhao · 2023-08-07T06:54:09Z

livekit_models.proto

+  message VideoSubscriptionReport {
+    string sid = 1;
+    RTCVideoReceiverStats rtc_stats = 2;
+    int32 ready_state = 3;


same question as above

davidzhao · 2023-08-07T06:56:13Z

livekit_models.proto

+    uint32 packetsLost = 5;
+    uint32 nack_count = 6;    
+    uint32 retransmitted_packets_sent = 7;
+    uint64 timestamp = 8;


which timestamp is this referencing? I only noticed it in the sender stats.. do we need a similar timestamp in the receiver side too? maybe this should be moved into the *Report message itself?

timestamp is referring to

A DOMHighResTimeStamp object indicating the time at which the sample was taken for this statistics object.

For receiver stats I included last_packet_received_timestamp, but probably makes sense to add the processing timestamp, too!

lukasIO and others added 12 commits May 22, 2023 12:34

wip client report

8982b9a

generated protobuf

257358e

add description to API error

1e14eca

Merge branch 'lukas/client-report' of github.com:livekit/protocol int…

f7d4a52

…o lukas/client-report

generated protobuf

d87229f

whitespace

a472ee6

Merge branch 'lukas/client-report' of github.com:livekit/protocol int…

c904440

…o lukas/client-report

move width and height into subscription report

c142198

generated protobuf

3dc0c00

flatten

d16646d

Merge branch 'lukas/client-report' of github.com:livekit/protocol int…

bf57eca

…o lukas/client-report

generated protobuf

0fce8fa

davidzhao reviewed May 23, 2023

View reviewed changes

lukasIO and others added 10 commits August 3, 2023 18:51

update rtc sender stats

3c0c83e

Merge branch 'lukas/client-report' of github.com:livekit/protocol int…

c88eb0b

…o lukas/client-report

generated protobuf

da97b3b

subscription reports

26d8f19

re-add jitter

862d71a

add concurrency to clientInfo

6d7c692

fix field number

6aeaacd

generated protobuf

5606a74

merge main

f7776b8

update models

8b70793

lukasIO marked this pull request as ready for review August 4, 2023 14:50

lukasIO requested a review from davidzhao August 4, 2023 14:50

davidzhao reviewed Aug 7, 2023

View reviewed changes

address comments

704dc4f

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Client report #385

Client report #385

lukasIO commented May 22, 2023

davidzhao May 23, 2023

lukasIO Aug 4, 2023

davidzhao May 23, 2023

lukasIO Jul 18, 2023 •

edited

Loading

lukasIO Aug 4, 2023

davidzhao May 23, 2023

lukasIO Jul 18, 2023

davidzhao Aug 7, 2023

davidzhao Aug 7, 2023

davidzhao Aug 7, 2023

lukasIO Aug 7, 2023

lukasIO Aug 7, 2023

davidzhao Aug 7, 2023

lukasIO Aug 7, 2023

lukasIO Aug 7, 2023

davidzhao Aug 7, 2023

lukasIO Aug 7, 2023

davidzhao Aug 7, 2023

lukasIO Aug 7, 2023 •

edited

Loading

davidzhao Aug 7, 2023

lukasIO Aug 7, 2023

davidzhao Aug 7, 2023

lukasIO Aug 7, 2023

davidzhao Aug 7, 2023

lukasIO Aug 7, 2023

davidzhao Aug 7, 2023

davidzhao Aug 7, 2023

lukasIO Aug 7, 2023

Client report #385

Are you sure you want to change the base?

Client report #385

Conversation

lukasIO commented May 22, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lukasIO Jul 18, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lukasIO Aug 7, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lukasIO Jul 18, 2023 •

edited

Loading

lukasIO Aug 7, 2023 •

edited

Loading