<div dir="ltr"><p dir="ltr">Hey,</p>
<p dir="ltr">last week we started reworking the major patch [1] and could not solve one issue.<br>
At the moment, the softmix_bridge determines if interleaved stereo can be written by querying ast_format's attribute data if OPUS is used ([2]):</p>```<br>struct opus_attr *attr = ast_format_get_attribute_data(format);<br>if (attr != NULL) {<br>  if (attr->stereo == 1) {<br>     *sample_rate = attr->maxplayrate;<br>     return 1;<br>  }<br>}<div>```<br></div><div><p dir="ltr">In fact, this approach is conceptually flawed as it introduces a direct dependency between softmix_bridge and opus.<br>To remove this dependency, we tried to attach this information to ast_codec (channel_count uint; default 1), but did not find a useful and working method to write the newly introduced field.<br>We tried to set it in the res/res_format_attr_opus, ie, while parsing the fmtp for OPUS.<br>Here, the ast_codec is const and we thus cannot be change.</p><p dir="ltr">Any suggestions?<br>Are we approaching the issue from a "correct" perspective?</p><p dir="ltr">[1] <a href="https://gerrit.asterisk.org/#/c/3524/3">https://gerrit.asterisk.org/#/c/3524/3</a><br>[2] <a href="https://github.com/frahaase/Asterisk_Binaural/blob/master/asterisk_modifications/include/asterisk/interleaved_stereo.h">https://github.com/frahaase/Asterisk_Binaural/blob/master/asterisk_modifications/include/asterisk/interleaved_stereo.h</a></p><p dir="ltr">---<br></p><p dir="ltr">
Dennis Guse<br></p>
<p dir="ltr">On Sun, Sep 18, 2016 at 6:27 AM, Leif Madsen <<a href="mailto:leif@leifmadsen.com" target="_blank">leif@leifmadsen.com</a>> wrote:<br>
> This is getting to be an old thread, but the "magic" for adding multiple<br>
> changes is as follows:<br>
><br>
> * use your working branch<br>
> * make changes<br>
> * git commit -a --amend<br>
> * save...<br>
> * git review<br>
><br>
> This way, the same branch, with the initial commit, will be preserved along<br>
> with the same change ID.<br>
><br>
> If you want to build upon an initial review, then you can do:<br>
><br>
> * git checkout -b intial_changes<br>
> * ...make changes and git review...<br>
> * git checkout -b secondary_changes<br>
> * ...make changes, git commit, git review...<br>
><br>
> This will provide a second review, based upon the first one.<br>
><br>
> If you need to change the first review, then that's fine. Follow the initial<br>
> procedure with making changes, and then git commit -a --amend, along with<br>
> git review.<br>
><br>
> If upstream (master) changes, and you need to rebase, that's fine too.<br>
><br>
> * git checkout master<br>
> * git fetch upstream (assuming you've run a git remote add upstream <path to<br>
> upstream Asterisk master>)<br>
> * git rebase upstream/master<br>
> * git review<br>
><br>
> Then if you need to rebase the secondary patch, after rebasing your<br>
> initial_changes branch, just rebase against origin/initial_changes, and run<br>
> your git review again.<br>
><br>
> This way, you can chain changes, and not have to duplicate work. As the<br>
> reviews happen and you make changes, you just keep committing against the<br>
> same change ID (which happens when you git commit -a --amend; git review),<br>
> and eventually things get merged down from the initial branch to your<br>
> secondary, tertiary, etc branches.<br>
><br>
> Hope that helps. Let me know if you need any additional help...<br>
><br>
> Leif.<br>
><br>
> On Tue, Aug 23, 2016 at 5:45 AM, Joshua Colp <<a href="mailto:jcolp@digium.com" target="_blank">jcolp@digium.com</a>> wrote:<br>
>><br>
>> Dennis Guse wrote:<br>
>>><br>
>>> Hey,<br>
>>><br>
>>> we adjusted the patch set (fftw3 is now a ifdef-ed).<br>
>>> Precisely, these two commits have been modified (basically adding<br>
>>> HAVE_FFTW3)<br>
>>><br>
>>> <a href="https://gerrit.asterisk.org/#/c/3522/" target="_blank">https://gerrit.asterisk.org/#/<wbr>c/3522/</a><br>
>>> <a href="https://gerrit.asterisk.org/#/c/3524/" target="_blank">https://gerrit.asterisk.org/#/<wbr>c/3524/</a><br>
>>><br>
>>> Two questions:<br>
>>> 1. It seems that we were working outside the commit tree by adjusting<br>
>>> 3522 and 3524 - we pulled each one down using _git review -d XXXX_,<br>
>>> applied the modifications, and submitted using _git review_<br>
>>> Is it necessary to also "re-commit" the subsequent patches (or does<br>
>>> gerrit this automatically)?<br>
>>> And if so, how?<br>
>>> At the moment Gerrit does not try to rebuild the subsequent patches<br>
>>> (i.e., 3523 and 3525).<br>
>><br>
>><br>
>> Gerrit does not automatically do this, it's up to you to rebase the<br>
>> patches so that a review is rebased on the latest version of the one it is<br>
>> dependent on. You can either do this from the web interface using the Rebase<br>
>> button (if it will cleanly rebase) or you have to build up a patch series on<br>
>> your system. This can be done by creating a local branch and pulling down<br>
>> each patch into it using (git review -x <review>), making any changes<br>
>> needed, committing, and doing git review again. This provides the dependency<br>
>> information to Gerrit it needs.<br>
>><br>
>>> 2. How can we withdraw the OPUS patch from the patch set?<br>
>>> <a href="https://gerrit.asterisk.org/#/c/3526/" target="_blank">https://gerrit.asterisk.org/#/<wbr>c/3526/</a><br>
>><br>
>><br>
>> You can click "Abandon" from the web interface and it will abandon that<br>
>> specific review.<br>
>><br>
>> Cheers,<br>
>><br>
>> --<br>
>> Joshua Colp<br>
>> Digium, Inc. | Senior Software Developer<br>
>> 445 Jan Davis Drive NW - Huntsville, AL 35806 - US<br>
>> Check us out at: <a href="http://www.digium.com" target="_blank">www.digium.com</a> & <a href="http://www.asterisk.org" target="_blank">www.asterisk.org</a><br>
>><br>
>><br>
>> --<br>
>> ______________________________<wbr>______________________________<wbr>_________<br>
>> -- Bandwidth and Colocation Provided by <a href="http://www.api-digital.com" target="_blank">http://www.api-digital.com</a> --<br>
>><br>
>> asterisk-dev mailing list<br>
>> To UNSUBSCRIBE or update options visit:<br>
>> <a href="http://lists.digium.com/mailman/listinfo/asterisk-dev" target="_blank">http://lists.digium.com/<wbr>mailman/listinfo/asterisk-dev</a><br>
><br>
><br>
><br>
><br>
> --<br>
> Leif Madsen<br>
> <a href="http://www.oreilly.com/catalog/asterisk" target="_blank">http://www.oreilly.com/<wbr>catalog/asterisk</a><br>
> <a href="http://www.leifmadsen.com" target="_blank">http://www.leifmadsen.com</a><br>
><br>
> --<br>
> ______________________________<wbr>______________________________<wbr>_________<br>
> -- Bandwidth and Colocation Provided by <a href="http://www.api-digital.com" target="_blank">http://www.api-digital.com</a> --<br>
><br>
> asterisk-dev mailing list<br>
> To UNSUBSCRIBE or update options visit:<br>
> <a href="http://lists.digium.com/mailman/listinfo/asterisk-dev" target="_blank">http://lists.digium.com/<wbr>mailman/listinfo/asterisk-dev</a><br>
</p>
</div></div>