[jitsi-dev] [jitsi-videobridge] add handling in the rest api for header extensions field (#74)


#1

REST api is missing a few features compared to the xmpp api, not sure if there's a bigger update in the works but I added support for header extensions here.
You can view, comment on, or merge this pull request online at:

  https://github.com/jitsi/jitsi-videobridge/pull/74

-- Commit Summary --

  * add handling in the rest api for header extensions field

-- File Changes --

    M src/org/jitsi/videobridge/rest/JSONDeserializer.java (44)
    M src/org/jitsi/videobridge/rest/JSONSerializer.java (7)

-- Patch Links --

https://github.com/jitsi/jitsi-videobridge/pull/74.patch
https://github.com/jitsi/jitsi-videobridge/pull/74.diff

···

---
Reply to this email directly or view it on GitHub:
https://github.com/jitsi/jitsi-videobridge/pull/74


#2

Oops, realized this included more than I had planned. I'll try and get it cleaned up.

···

---
Reply to this email directly or view it on GitHub:
https://github.com/jitsi/jitsi-videobridge/pull/74#issuecomment-112609421


#3

Hi Brian,

Thank you for your contribution. The RTP header extensions support for REST is something that we'd like to merge. Before we can do that we need you to sign our contributor agreement as either an [individual](https://jitsi.org/icla) or a [corporation](https://jitsi.org/ccla).
Also, there are a couple of minor changes necessary for the code, so that it follows our style guide: opening braces should be on new lines, a Logger instance should be used instead of System.err for logging errors.

Thanks again,
Boris

···

---
Reply to this email directly or view it on GitHub:
https://github.com/jitsi/jitsi-videobridge/pull/74#issuecomment-115242508


#4

Cool! Thanks Boris. I'll work on getting this cleaned up.

···

---
Reply to this email directly or view it on GitHub:
https://github.com/jitsi/jitsi-videobridge/pull/74#issuecomment-115418707


#5

Hey Boris, still figuring out the contract piece, but I've tried to make the appropriate style changes here to be consistent with the rest of the file.

···

---
Reply to this email directly or view it on GitHub:
https://github.com/jitsi/jitsi-videobridge/pull/74#issuecomment-115827393


#6

@bgrozev just got the contributor agreement signed and submitted

···

---
Reply to this email directly or view it on GitHub:
https://github.com/jitsi/jitsi-videobridge/pull/74#issuecomment-116772488


#7

@bgrozev let me know if there's anything else you need from me on this.

···

---
Reply to this email directly or view it on GitHub:
https://github.com/jitsi/jitsi-videobridge/pull/74#issuecomment-119291189


#8

Sorry, Brian, it's been a busy couple of days. I'll review and merge when I get a chance.

···

---
Reply to this email directly or view it on GitHub:
https://github.com/jitsi/jitsi-videobridge/pull/74#issuecomment-119604993


#9

Merged #74.

···

---
Reply to this email directly or view it on GitHub:
https://github.com/jitsi/jitsi-videobridge/pull/74#event-351227525


#10

@brianh5 are you able to specify rtcp-fb: ccm fir, nack pli, and goog-remb via the rest api?

···

---
Reply to this email directly or view it on GitHub:
https://github.com/jitsi/jitsi-videobridge/pull/74#issuecomment-124602643


#11

@markreg yes, we're able to set that. i do notice, however, that when you access the conference state via the rest api, that information will not show up. it looks like RtpChannel::describe (which serializes the bridge data into json to respond to rest requests) only does the ssrc--so if you're worried your rtcp-fb information isn't showing up in rest, i think that's why.

···

---
Reply to this email directly or view it on GitHub:
https://github.com/jitsi/jitsi-videobridge/pull/74#issuecomment-124661230


#12

Videobridge currently doesn't use any of the rtcp-fb fields. It doesn't even store the information, which is why it doesn't show up in replies (either XMPP or REST)

···

---
Reply to this email directly or view it on GitHub:
https://github.com/jitsi/jitsi-videobridge/pull/74#issuecomment-124758526


#13

So, is it not used because its features are implicit in Videobridge or are the rtcp-fb features seen as unnecessary?

···

---
Reply to this email directly or view it on GitHub:
https://github.com/jitsi/jitsi-videobridge/pull/74#issuecomment-126547898


#14

Would you accept a pull request with rtcp-fb support?

···

---
Reply to this email directly or view it on GitHub:
https://github.com/jitsi/jitsi-videobridge/pull/74#issuecomment-127083635


#15

Do you mean rtcp-fb support in COLIBRI and the REST interface, or somehow handling the specific rtcp-fb extensions set on a channel? I think we would accept the first, since we can use it to detect FIR, NACK, REMB support for endpoints.
Do you have a specific use-case where you need this?

···

---
Reply to this email directly or view it on GitHub:
https://github.com/jitsi/jitsi-videobridge/pull/74#issuecomment-127367448


#16

Sure, no problem! Wanted to make sure everything with the contributor
agreement was right.

···

On Wed, Jul 8, 2015 at 7:39 AM, bgrozev <notifications@github.com> wrote:

Sorry, Brian, it's been a busy couple of days. I'll review and merge when
I get a chance.


Reply to this email directly or view it on GitHub
<https://github.com/jitsi/jitsi-videobridge/pull/74#issuecomment-119604993>
.

_______________________________________________
dev mailing list
dev@jitsi.org
Unsubscribe instructions and other list options:
http://lists.jitsi.org/mailman/listinfo/dev