[Asterisk-code-review] media: Add experimental support for RTCP feedback. (asterisk[master])

[Lorenzo Miniero]

Lorenzo Miniero has posted comments on this change. ( https://gerrit.asterisk.org/4614 )

Change subject: media: Add experimental support for RTCP feedback.
......................................................................


Patch Set 1:

(2 comments)

Hi guys, since the last time I made changes I formatted my laptop and so have lost the setup I had made for Gerrit. Considering I very much prefer passing for a dummy, rather than risking making the same mess I did with my first Gerrit interactions, which are the steps to restore this exact branch here on my local setup and make the changes we discussed, without resulting in yet another Change page or other problems? I cloned the base https://gerrit.asterisk.org/asterisk repo but not sure what I should do at this stage... Thanks.

https://gerrit.asterisk.org/#/c/4614/1/res/res_rtp_asterisk.c
File res/res_rtp_asterisk.c:

PS1, Line 4334: 				memcpy(report_block, rtcp_report->report_block[0], sizeof(struct ast_rtp_rtcp_report_block));
> Although it appears we are not currently handling more than one report at t
Ack, will do, although you'll then have to remember wrapping that into a for/while to copy them all and not just the last one.


PS1, Line 4335: 				rtcp_report2 = (struct ast_rtp_rtcp_report *)rtp->f.data.ptr;
              : 				rtcp_report2->report_block[0] = report_block;
> Is this code needed (I could be misreading it)? It appears to get the rtcp_
Answered the same question (or very similar) Joshua Colp made on an older revision of the code here:
https://gerrit.asterisk.org/#/c/4521/2/res/res_rtp_asterisk.c@4334
The reason basically is that the block property in ast_rtp_rtcp_report is a pointer, not the block itself. As such, in order to share it all together, we allocate room for pointer+block, copy the block where it needs to be (right after the pointer), and set the pointer to where the block is.


-- 
To view, visit https://gerrit.asterisk.org/4614
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ifd6aa77fb4a7ff546c6025900fc2baf332c31857
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Owner: Lorenzo Miniero <lminiero at gmail.com>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Kevin Harwell <kharwell at digium.com>
Gerrit-Reviewer: Lorenzo Miniero <lminiero at gmail.com>
Gerrit-Reviewer: Matthew Fredrickson <creslin at digium.com>
Gerrit-HasComments: Yes