search

boatbod / op25

Fork of osmocom OP25 by boatbod
319 stars 100 forks source link

Adaptive Smoothing Optimization #162

Closed tadscottsmith closed 1 year ago

tadscottsmith commented 2 years ago

I've been trying to go through your code against the standard and optimize audio quality as much as possible. I believe that in the line below, it is not an error in the standard, and that it should be E4 instead of ET. https://github.com/boatbod/op25/blob/a8dd3419b36b0d4aba3e05889c753d02ba93293b/op25/gr-op25_repeater/lib/software_imbe_decoder.cc#L797 The value E4 above would be the number of errors calculated when decoding U4 in this line. https://github.com/boatbod/op25/blob/60d8532e5be5e7129099074e3a1a3d111ae302e6/op25/gr-op25/lib/op25_imbe_frame.h#L326 Let me know if you would like a PR with these changes. Thank you!

boatbod commented 1 year ago

Sorry for the delay in looking at this - been really crazy at work for the past several months. Let me dig in to it a little further, thanks. ETA: I explored a little more and it seems the adaptive_smoothing() routine really only does anything meaningful for IMBE full rate decode. When adaptive_smoothing() is invoked by decode_tap() the ET parameter is always = 0.

ETA2: I've re-read the spec and gone through the code and apart from that one single reference to E4 in the VM formula, nothing else suggests there are any inputs to the smoothing algorithm other than ER or ET. Further, for ET to be 0, E4 necessarily would have to be 0 as well.

Curious to know whether your modification results in subjectively better IMBE (phase 1) audio? As noted, this code has no bearing on AMBE (phase 2) since ET=0 for decode_tap() entry point and frame repeats are handled outside of the software decoder.

tadscottsmith commented 1 year ago

The error estimation section explains that E0 through E6 make up ET.

If ET = 0, then certainly E4 = 0. However, if E4 = 0, that has no direct bearing on ET. Essentially, for Algorithm 112, if ET > 4 and E4 = 0, the wrong adaptive threshold is being applied. I'll see if I can come up with a way to see if it sounds objectively better with the changes.

boatbod commented 1 year ago

I understand what you're saying, and to the way I read it there's no doubt that Section 9 of the TIA-102.BABA-A spec has an inconsistency between Algorithm 112 and Figure 25. I'm still leaning towards the mistake being in Algo 112.

tadscottsmith commented 1 year ago

I haven't had a chance to put the effort into comparing the results side by side, and I don't want to leave this open forever, so I'll just close it for now. I can reopen if I have a chance to dig into it more later.