[opus] API for checking whether the encoder is in DTX (PR #107)

[Jean-Marc Valin]

Can you post the updated patch here?

	Jean-Marc

On 04/08/2019 07:55 AM, Gustaf Ullberg wrote:
> Thank you Mark.
> 
> I agree and have now updated the pull request with a new commit,
> addressing your comments.
> Please take a look.
> 
> /Gustaf
> 
> On Fri, 5 Apr 2019 at 11:41, Mark Harris <mark.hsj at gmail.com
> <mailto:mark.hsj at gmail.com>> wrote:
> 
>     On 2019-04-01 3:37, Gustaf Ullberg wrote:
>     > Hi everyone,
>     >
>     > Some time ago, I sent a pull request
>     > <https://github.com/xiph/opus/pull/107> to the Opus github page.
>     > Jean-Marc asked me to post it to the mailing list so everyone can
>     have a
>     > look at it.
>     >
>     > You can find the description and code changes below. Please let me
>     know
>     > if you have any questions or concerns.
>     >
>     > Best regards
>     > Gustaf Ullberg
>     >
>     > In WebRTC, we would like to be able to differentiate between "normal"
>     > frames and the comfort noise update frames that are encoded before and
>     > in-between DTX frames. These CNG update frames could then be
>     flagged for
>     > routing or statistics reasons. Such flagging could also make it easier
>     > for a jitterbuffer to differ DTX from packet loss.
>     >
>     > This PR implements an encoder CTL named OPUS_GET_IN_DTX.
>     OPUS_GET_IN_DTX
>     > returns 1 if the last encoded frame was either a DTX frame, or a CNG
>     > update frame that is sent every 420 ms between the DTX frames.
>     >
>     > This is achieved by checking if the number of consecutive inactive
>     > frames (nb_no_activity_frames) is greater or equal to
>     > NB_SPEECH_FRAMES_BEFORE_DTX.
>     >
>     >
>     > diff --git a/include/opus_defines.h b/include/opus_defines.h
>     > index fbf5d0eb..e35114e4 100644
>     > --- a/include/opus_defines.h
>     > +++ b/include/opus_defines.h
>     > @@ -168,6 +168,7 @@ extern "C" {
>     >  /* Don't use 4045, it's already taken by OPUS_GET_GAIN_REQUEST */
>     >  #define OPUS_SET_PHASE_INVERSION_DISABLED_REQUEST 4046
>     >  #define OPUS_GET_PHASE_INVERSION_DISABLED_REQUEST 4047
>     > +#define OPUS_GET_IN_DTX_REQUEST              4049
>     >  
>     >  /** Defines for the presence of extended APIs. */
>     >  #define OPUS_HAVE_OPUS_PROJECTION_H
>     > @@ -715,6 +716,16 @@ extern "C" {
>     >    * </dl>
>     >    * @hideinitializer */
>     >  #define OPUS_GET_PHASE_INVERSION_DISABLED(x)
>     > OPUS_GET_PHASE_INVERSION_DISABLED_REQUEST, __opus_check_int_ptr(x)
>     > +/** Gets the DTX state of the encoder.
>     > +  * Returns wheter the last encoded frame was either a comfort
>     noise update
> 
> 
>     s/wheter/whether/
> 
> 
>     > +  * during DTX or not encoded because of DTX.
>     > +  * @param[out] x <tt>opus_int32 *</tt>: Returns one of the following
>     > values:
>     > +  * <dl>
>     > +  * <dt>0</dt><dd>The encoder is not in DTX.</dd>
>     > +  * <dt>1</dt><dd>The encoder is in DTX.</dd>
>     > +  * </dl>
>     > +  * @hideinitializer */
>     > +#define OPUS_GET_IN_DTX(x) OPUS_GET_IN_DTX_REQUEST,
>     __opus_check_int_ptr(x)
>     >  
>     >  /**@}*/
>     >  
>     > diff --git a/src/opus_encoder.c b/src/opus_encoder.c
>     > index cbeb40ae..0d84737a 100644
>     > --- a/src/opus_encoder.c
>     > +++ b/src/opus_encoder.c
>     > @@ -2725,6 +2725,27 @@ int opus_encoder_ctl(OpusEncoder *st, int
>     > request, ...)
>     >              ret = celt_encoder_ctl(celt_enc,
>     OPUS_SET_ENERGY_MASK(value));
>     >          }
>     >          break;
>     > +        case OPUS_GET_IN_DTX_REQUEST:
>     > +        {
>     > +            opus_int32 *value = va_arg(ap, opus_int32*);
>     > +            if (!value)
>     > +            {
>     > +                goto bad_arg;
>     > +            }
>     > +            *value = 0;
>     > +            if (st->silk_mode.useDTX) {
>     > +                /* DTX determined by Silk. */
>     > +                void *silk_enc = (char*)st+st->silk_enc_offset;
>     > +                *value =
>     > ((silk_encoder*)silk_enc)->state_Fxx[0].sCmn.noSpeechCounter >=
>     > NB_SPEECH_FRAMES_BEFORE_DTX;
>     > +            }
> 
> 
>     It looks like this does not have the same behavior as the code that
>     decides whether to produce a DTX frame.  For example, in the case of
>     stereo the SILK DTX code considers both channels, but this code looks
>     only at state_Fxx[0].  Shouldn't the behavior match?
> 
>      - Mark
> 
> 
> 
>     > +#ifndef DISABLE_FLOAT_API
>     > +            else if (st->use_dtx) {
>     > +                /* DTX determined by Opus. */
>     > +                *value = st->nb_no_activity_frames >=
>     > NB_SPEECH_FRAMES_BEFORE_DTX;
>     > +            }
>     > +#endif
>     > +        }
>     > +        break;
>     >  
>     >          case CELT_GET_MODE_REQUEST:
>     >          {
>     >
>     >
> 
> 
> 
> _______________________________________________
> opus mailing list
> opus at xiph.org
> http://lists.xiph.org/mailman/listinfo/opus
>