EricssonResearch / scream

SCReAM - Mobile optimised congestion control algorithm
BSD 2-Clause "Simplified" License
174 stars 53 forks source link

RTCP CC Feedback payload type value? #17

Open saikrishnareddykovvuri opened 5 years ago

saikrishnareddykovvuri commented 5 years ago

In mentioned version 2 of avtcore-cc-feedback draft the PT in feed back is mentioned as 205.

But in code gstreamer plugin code is creating the feedback as 205 which was proper. But the streamRX file outside the gstreamer module creating feedback with PT 207.

https://github.com/EricssonResearch/scream/blob/master/code/ScreamRx.cpp

Are the files present outside the gstreamer module uptodate? Can we use the files outside the gstreamer to integrate in the application?

sdroege commented 5 years ago

From what I can see, the only difference is the RTCP packet type. 205 (RTPFB) vs. 207 (XR). The code outside the gstreamer module already uses the correct type (XR).

RFC8298 mentions usage of XR, and so does draft-ietf-rmcat-scream-cc-13. draft-ietf-avtcore-cc-feedback-message-03 indeed mentions RTPFB but I don't think the code here currently implements the avtcore-cc-feedback-message signalling, or draft-holmer-rmcat-transport-wide-cc-extensions-01.

So from what I understand, the GStreamer code using RTPFB is wrong and it should be XR like in the code outside the GStreamer module until the generalized the generalized feedback messages are implemented.

saikrishnareddykovvuri commented 5 years ago

From my observation. Even we are generating XR(207) packet, it is not included the Packet Receipt Times Report header. when i captured the pcap and verified the feedback XR structure it was showing as malformed.

Screenshot 2019-05-31 at 2 48 55 PM

When i looked in the streamRX.cpp

stream->getStandardizedFeedback(time_ntp, &buf[ptr], size_stream); is generating block payload without header and appending directly to the RTCP header.

As we are trying to note, the reception times of RTP packets. According to XR standard https://tools.ietf.org/html/rfc3611#section-4.3

all the timestamps which we are transmitting in XR block type 3 from above are 32 bit long. But in screamRX they are in 16bit.

Is the screamRX, generating feedback, XR standard?

IngJohEricsson commented 5 years ago

Hi Thanks for the review.

The code implements https://tools.ietf.org/html/draft-ietf-avtcore-cc-feedback-message-02 , and this implies PT=205 (RTPFB). I see that the PT in https://github.com/EricssonResearch/scream/blob/master/code/gscream/gst-gscreamrx/gst-plugin/src/ScreamRx.cpp use the correct PT value. The code in https://github.com/EricssonResearch/scream/blob/master/code/gscream/gst-gscreamrx/gst-plugin/src/gstgscreamrx.cpp#L381 also use RTPFB. One possible error is that I have not set FMT correctly , it is currently zero.

With that said.. https://github.com/EricssonResearch/scream/blob/master/code/ScreamRx.cpp actually use PT=207, which is wrong. I will fix this. You may notice that these modules are duplicates, I will merge these later on to reduce the risk of confusion.

/Ingemar

saikrishnareddykovvuri commented 5 years ago

Hi Thanks for the information.

As we are using PT=205(RTPFB). According to RFC 4585 FMT value is defined as 1 for Generic NACK.

As scream implemented https://tools.ietf.org/html/draft-ietf-avtcore-cc-feedback-message-02. It has mention FMT as CCFB, what is the value for CCFB. i searched for the value but unable to find anywhere.

iampeteandrews commented 5 years ago

@saikrishnareddykovvuri or @sdroege /

can you share the gstreamer pipes (tx/rx) and network configs that you're testing with? i think my problem (in the thread i started) may be related to this issue.

thanks!

-pete

saikrishnareddykovvuri commented 5 years ago

Hi @iampeteandrews

Unfortunately i am not using gstreamer. I am working with only the necessary files, integrated into another application and testing. So i can not provide any info related gstreamer. Can you point me to the thread which you started so that i can have a look.

iampeteandrews commented 5 years ago

@saikrishnareddykovvuri - thanks...here's my thread: https://github.com/EricssonResearch/scream/issues/16

-pete

IngJohEricsson commented 5 years ago

Hi

CCFB is to be defined by the IANA but AFAIK, this has not happened yet.

https://www.iana.org/assignments/rtp-parameters/rtp-parameters.xhtml#rtp-parameters-8 https://www.iana.org/assignments/rtp-parameters/rtp-parameters.xhtml#rtp-parameters-8

I guess the best alternative for now to use one of the unassigned values, now it is zero which may perhaps be parsed badly.

/Ingemar

From: saikrishnareddykovvuri notifications@github.com Sent: den 3 juni 2019 19:39 To: EricssonResearch/scream scream@noreply.github.com Cc: Ingemar Johansson S ingemar.s.johansson@ericsson.com; Comment comment@noreply.github.com Subject: Re: [EricssonResearch/scream] RTCP CC Feedback payload type value? (#17)

Hi Thanks for the information.

As we are using PT=205(RTPFB). According to RFC 4585 FMT value is defined as 1 for Generic NACK.

As scream implemented https://tools.ietf.org/html/draft-ietf-avtcore-cc-feedback-message-02. It has mention FMT as CCFB, what is the value for CCFB. i searched for the value but unable to find anywhere.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://protect2.fireeye.com/url?k=1e0fe57a-42dcf512-1e0fa5e1-8691b328a8b8-3bd6ac6b12f9b375&q=1&u=https%3A%2F%2Fgithub.com%2FEricssonResearch%2Fscream%2Fissues%2F17%3Femail_source%3Dnotifications%26email_token%3DACRZ2GH7WUNS4K5HKWW7GP3PYVJKRA5CNFSM4HQOTCRKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODW2ETYY%23issuecomment-498354659 , or mute the thread https://protect2.fireeye.com/url?k=5b0c501e-07df4076-5b0c1085-8691b328a8b8-ae615ca0cb562fa9&q=1&u=https%3A%2F%2Fgithub.com%2Fnotifications%2Funsubscribe-auth%2FACRZ2GF37QJXXPT7UYWHYPLPYVJKRANCNFSM4HQOTCRA .