thr3ads.net - opus - [opus] API for checking whether the encoder is in DTX (PR #107) [Apr 2019]

If this information is useful, please help other people find it:
Share via:

Gustaf Ullberg

2019-Apr-08 11:55 UTC

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

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> 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:
> >          {
> >
> >
>-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.xiph.org/pipermail/opus/attachments/20190408/10c405b5/attachment.html>

Jean-Marc Valin

2019-Apr-08 15:34 UTC

head link

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

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
>

Mark Harris

2019-Apr-09 10:42 UTC

head link

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

On 2019-04-08 4:55, 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
I think you will also need to check the mode of the previous frame
(st->prev_mode) before using internal SILK encoder state.  It could have
been in SILK DTX some time ago, but then switched to CELT.  Normally
there would be at least one non-DTX SILK/Hybrid frame before it switches
to CELT only, but it is possible to switch directly from SILK DTX to
CELT only mode (for example, if the frame size is reduced to less than
10 ms then it will have no choice but to switch to CELT immediately).
If it ever switches back to SILK or Hybrid it will reset the SILK
encoder state at that time, but until then the SILK encoder state may
contain stale information.  Checking that st->prev_mode is
MODE_SILK_ONLY or MODE_HYBRID will verify that the SILK encoder state
corresponds to the previous frame.

 - Mark

Gustaf Ullberg

2019-Apr-10 10:27 UTC

head link

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

Yes, good point. I added the checking of prev_mode for Silk DTX to avoid
using stale data from the Silk state.
The PR is updated, and I'm attaching an updated patch.

/Gustaf

On Tue, 9 Apr 2019 at 12:42, Mark Harris <mark.hsj at gmail.com> wrote:
> On 2019-04-08 4:55, 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
>
> I think you will also need to check the mode of the previous frame
> (st->prev_mode) before using internal SILK encoder state.  It could have
> been in SILK DTX some time ago, but then switched to CELT.  Normally
> there would be at least one non-DTX SILK/Hybrid frame before it switches
> to CELT only, but it is possible to switch directly from SILK DTX to
> CELT only mode (for example, if the frame size is reduced to less than
> 10 ms then it will have no choice but to switch to CELT immediately).
> If it ever switches back to SILK or Hybrid it will reset the SILK
> encoder state at that time, but until then the SILK encoder state may
> contain stale information.  Checking that st->prev_mode is
> MODE_SILK_ONLY or MODE_HYBRID will verify that the SILK encoder state
> corresponds to the previous frame.
>
>  - Mark
>-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.xiph.org/pipermail/opus/attachments/20190410/cb01eda1/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-API-for-checking-whether-the-encoder-is-in-DTX.patch
Type: text/x-patch
Size: 2938 bytes
Desc: not available
URL:
<http://lists.xiph.org/pipermail/opus/attachments/20190410/cb01eda1/attachment.bin>

Seemingly Similar Threads

Search for more reasonably related threads

opus - Apr 2019 - API for checking whether the encoder is in DTX (PR #107)

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

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

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

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

Seemingly Similar Threads