<p style="white-space: pre-wrap; word-wrap: break-word;">I don't think in off-nominal error cases the stats should be saved. I think it is only when a media session is terminated after being used that the stats in regards to it should be saved. As I previously said the way it is now can result in stats for RTP sessions that were never used, as well as stats for the same RTP session at different points in time.</p><p style="white-space: pre-wrap; word-wrap: break-word;">As a consumer of the functionality I think trying to use the data would be problematic in its current state. You would need to go through the data, correlate any duplicates, determine if any should be ignored, and then it would be useful. Without doing so it can lead you to wrong interpretations. For example if stats for an RTP instance that was never used were present and I saw the statistics in the vector I might wonder if there was a problem - but in reality there wasn't, because the stream was never actually used.</p><p>Patch set 5:<span style="border-radius: 3px; display: inline-block; margin: 0 2px; padding: 4px;background-color: #ffd4d4; color: #000000;">Code-Review -1</span></p><p><a href="https://gerrit.asterisk.org/10899">View Change</a></p><p>1 comment:</p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.asterisk.org/#/c/10899/5/include/asterisk/res_pjsip_session.h">File include/asterisk/res_pjsip_session.h:</a></p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/#/c/10899/5/include/asterisk/res_pjsip_session.h@144">Patch Set #5, Line 144:</a> </p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;"><pre style="font-family: monospace,monospace; white-space: pre-wrap;">        /*! \brief Related sip session */<br>     struct ast_sip_session *sip_session;<br></pre></blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">The reason I mentioned passing this in, instead of storing it is because it alters the API and ripples out to different places as you've done. It also changes the reference to be a bit circular (thankfully without a real reference). A media state is supposed to be isolated and not know/care about the session it is stored on.</p><p style="white-space: pre-wrap; word-wrap: break-word;">I think it is perfectly reasonable that the save function require a session to be passed in instead.</p></li></ul></li></ul><p>To view, visit <a href="https://gerrit.asterisk.org/10899">change 10899</a>. To unsubscribe, or for help writing mail filters, visit <a href="https://gerrit.asterisk.org/settings">settings</a>.</p><div itemscope itemtype="http://schema.org/EmailMessage"><div itemscope itemprop="action" itemtype="http://schema.org/ViewAction"><link itemprop="url" href="https://gerrit.asterisk.org/10899"/><meta itemprop="name" content="View Change"/></div></div>

<div style="display:none"> Gerrit-Project: asterisk </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-MessageType: comment </div>
<div style="display:none"> Gerrit-Change-Id: Ib25c2d3fc4da084aecfde2a82c1b1d733bd64fa5 </div>
<div style="display:none"> Gerrit-Change-Number: 10899 </div>
<div style="display:none"> Gerrit-PatchSet: 5 </div>
<div style="display:none"> Gerrit-Owner: sungtae kim <pchero21@gmail.com> </div>
<div style="display:none"> Gerrit-Reviewer: Friendly Automation (1000185) </div>
<div style="display:none"> Gerrit-Reviewer: Joshua C. Colp <jcolp@digium.com> </div>
<div style="display:none"> Gerrit-Reviewer: sungtae kim <pchero21@gmail.com> </div>
<div style="display:none"> Gerrit-Comment-Date: Fri, 08 Feb 2019 15:26:12 +0000 </div>
<div style="display:none"> Gerrit-HasComments: Yes </div>
<div style="display:none"> Gerrit-HasLabels: Yes </div>