Hi! We've been using PJMEDIA as part of a media server for voip calls. When support for G722 was added between endpoints we noticed that the sound level was much lower compared to G711a. After some investigation we found that the clip detector in the G722 decoder kicked in immediately when the decoding started, and that it seemed to be due to negative signal levels being fed into the clip detector. The current implementation of the g722_dec_decoder return values in the range [-16384, 16383]. The way the current clip detector is constructed it will always detect clipping on negative values because of the bit representation of negative values. With a small modification in how the signal is compared with the PCM clip mask we avoid clip detection by negative values. The result is that the sound level of the decoded G722 signal is on par with G711a. The attached patch (git format-patch) show the fix we use. P.S. A PCM shift of 2 has sometimes in our environment resulted in distortion, probably due to clipping, and because of this we use a PCM shift of 1. BR, Martin Navne Telavox

Hi Martin, You're right, wow, the bug has been around for quite a long time! It seems that the encoder part is buggy too, so tried to update your patch for that. And in the middle of it, realizing that bit shifting operations seem to complicate things (were meant optimization perhaps, but now we are about to add absolute and more checks anyway), so tried to replace them with multiplication/division operations. Also, instead of immediately stop the shifting, perhaps reducing it step by step will be better (more adaptive perhaps, as you mentioned about setting pcm shift to 1). Attached is the updated patch, please check and let us know if you have any feedback. Thank you for the report, the analysis, and the patch. BR, nanang On Tue, Sep 3, 2019 at 8:18 PM Martin Navne <martin.navne@telavox.se> wrote: > Hi! > > We've been using PJMEDIA as part of a media server for voip calls. When > support for G722 was added between endpoints we noticed that the sound > level was much lower compared to G711a. > > After some investigation we found that the clip detector in the G722 > decoder kicked in immediately when the decoding started, and that it > seemed to be due to negative signal levels being fed into the clip > detector. The current implementation of the g722_dec_decoder return > values in the range [-16384, 16383]. The way the current clip detector > is constructed it will always detect clipping on negative values because of > the bit representation of negative values. > > With a small modification in how the signal is compared with the PCM > clip mask we avoid clip detection by negative values. The result is that > the sound level of the decoded G722 signal is on par with G711a. > > The attached patch (git format-patch) show the fix we use. > > P.S. > A PCM shift of 2 has sometimes in our environment resulted in distortion, > probably due to clipping, and because of this we use a PCM shift of 1. > > BR, > Martin Navne > Telavox > _______________________________________________ > Visit our blog: http://blog.pjsip.org > > pjsip mailing list > pjsip@lists.pjsip.org > http://lists.pjsip.org/mailman/listinfo/pjsip_lists.pjsip.org >

Hi Nanang! Thanks for the update; I did a pen-and-paper test of the code and it seems to do what it should. :) Good idea with the adaptive approach! BR, Martin On Thu, 5 Sep 2019 at 13:07, Nanang Izzuddin <nanang@pjsip.org> wrote: > Hi Martin, > > You're right, wow, the bug has been around for quite a long time! > > It seems that the encoder part is buggy too, so tried to update your patch > for that. And in the middle of it, realizing that bit shifting operations > seem to complicate things (were meant optimization perhaps, but now we are > about to add absolute and more checks anyway), so tried to replace them > with multiplication/division operations. Also, instead of immediately stop > the shifting, perhaps reducing it step by step will be better (more > adaptive perhaps, as you mentioned about setting pcm shift to 1). Attached > is the updated patch, please check and let us know if you have any feedback. > > Thank you for the report, the analysis, and the patch. > > BR, > nanang > > > On Tue, Sep 3, 2019 at 8:18 PM Martin Navne <martin.navne@telavox.se> > wrote: > >> Hi! >> >> We've been using PJMEDIA as part of a media server for voip calls. When >> support for G722 was added between endpoints we noticed that the sound >> level was much lower compared to G711a. >> >> After some investigation we found that the clip detector in the G722 >> decoder kicked in immediately when the decoding started, and that it >> seemed to be due to negative signal levels being fed into the clip >> detector. The current implementation of the g722_dec_decoder return >> values in the range [-16384, 16383]. The way the current clip detector >> is constructed it will always detect clipping on negative values because >> of >> the bit representation of negative values. >> >> With a small modification in how the signal is compared with the PCM >> clip mask we avoid clip detection by negative values. The result is that >> the sound level of the decoded G722 signal is on par with G711a. >> >> The attached patch (git format-patch) show the fix we use. >> >> P.S. >> A PCM shift of 2 has sometimes in our environment resulted in distortion, >> probably due to clipping, and because of this we use a PCM shift of 1. >> >> BR, >> Martin Navne >> Telavox >> _______________________________________________ >> Visit our blog: http://blog.pjsip.org >> >> pjsip mailing list >> pjsip@lists.pjsip.org >> http://lists.pjsip.org/mailman/listinfo/pjsip_lists.pjsip.org >> > _______________________________________________ > Visit our blog: http://blog.pjsip.org > > pjsip mailing list > pjsip@lists.pjsip.org > http://lists.pjsip.org/mailman/listinfo/pjsip_lists.pjsip.org >

Thanks for the pen-and-paper test :) Just commited the patch into SVN trunk, ticket is https://trac.pjsip.org/repos/ticket/2228. BR, nanang On Thu, Sep 5, 2019 at 9:16 PM Martin Navne <martin.navne@telavox.se> wrote: > Hi Nanang! > > Thanks for the update; I did a pen-and-paper test of the code and it seems > to do what it should. :) > Good idea with the adaptive approach! > > BR, > Martin > > On Thu, 5 Sep 2019 at 13:07, Nanang Izzuddin <nanang@pjsip.org> wrote: > >> Hi Martin, >> >> You're right, wow, the bug has been around for quite a long time! >> >> It seems that the encoder part is buggy too, so tried to update your >> patch for that. And in the middle of it, realizing that bit shifting >> operations seem to complicate things (were meant optimization perhaps, but >> now we are about to add absolute and more checks anyway), so tried to >> replace them with multiplication/division operations. Also, instead of >> immediately stop the shifting, perhaps reducing it step by step will be >> better (more adaptive perhaps, as you mentioned about setting pcm shift to >> 1). Attached is the updated patch, please check and let us know if you have >> any feedback. >> >> Thank you for the report, the analysis, and the patch. >> >> BR, >> nanang >> >> >> On Tue, Sep 3, 2019 at 8:18 PM Martin Navne <martin.navne@telavox.se> >> wrote: >> >>> Hi! >>> >>> We've been using PJMEDIA as part of a media server for voip calls. When >>> support for G722 was added between endpoints we noticed that the sound >>> level was much lower compared to G711a. >>> >>> After some investigation we found that the clip detector in the G722 >>> decoder kicked in immediately when the decoding started, and that it >>> seemed to be due to negative signal levels being fed into the clip >>> detector. The current implementation of the g722_dec_decoder return >>> values in the range [-16384, 16383]. The way the current clip detector >>> is constructed it will always detect clipping on negative values because >>> of >>> the bit representation of negative values. >>> >>> With a small modification in how the signal is compared with the PCM >>> clip mask we avoid clip detection by negative values. The result is >>> that >>> the sound level of the decoded G722 signal is on par with G711a. >>> >>> The attached patch (git format-patch) show the fix we use. >>> >>> P.S. >>> A PCM shift of 2 has sometimes in our environment resulted in >>> distortion, probably due to clipping, and because of this we use a PCM >>> shift of 1. >>> >>> BR, >>> Martin Navne >>> Telavox >>> _______________________________________________ >>> Visit our blog: http://blog.pjsip.org >>> >>> pjsip mailing list >>> pjsip@lists.pjsip.org >>> http://lists.pjsip.org/mailman/listinfo/pjsip_lists.pjsip.org >>> >> _______________________________________________ >> Visit our blog: http://blog.pjsip.org >> >> pjsip mailing list >> pjsip@lists.pjsip.org >> http://lists.pjsip.org/mailman/listinfo/pjsip_lists.pjsip.org >> > _______________________________________________ > Visit our blog: http://blog.pjsip.org > > pjsip mailing list > pjsip@lists.pjsip.org > http://lists.pjsip.org/mailman/listinfo/pjsip_lists.pjsip.org >