srt icon indicating copy to clipboard operation
srt copied to clipboard

[core] Added SRTO_PACINGMODE pacing mode

Open maxsharabayko opened this issue 5 years ago • 4 comments

These changes are extracted from #1383. Closes #1383. Documentation is in PR #1533.

  • Added option SRTO_PACINGMODE
  • New pacing mode: SRT_PACING_INBW_MINESTIMATE

SRTO_PACINGMODE option can be used to select the desired output pacing only in live mode.

See the description in APISocketOptions.md.

Available modes are:

SRT_PACINGMODE bw bw_overhead Formula for MAXBW Calculation
SRT_PACING_UNSET - - The mode is deduced, see the table below
SRT_PACING_MAXBW_DEFAULT - - MAXBW is set to the default value of 1 Gbps
SRT_PACING_MAXBW_SET MAXBW - MAXBW is set explicitly, in bytes per second, must be positive
SRT_PACING_INBW_SET INPUTBW OHEADBW MAXBW = INPUTBW * (1 + OHEADBW /100)
SRT_PACING_INBW_ESTIMATE - OHEADBW MAXBW = EST_INPUTBW * (1 + OHEADBW /100)
SRT_PACING_INBW_MINESTIMANTE INPUTBW OHEADBW MAXBW = max(INPUTBW, EST_INPUTBW) * (1 + OHEADBW /100)

TODO

  • [ ] URI option pacing=mode,bw,ohead.
  • [ ] Don't allow to set SRTO_MAXBW, SRTO_INPUTBW, SRTO_OHEADBW if the currently set SRTO_PACINGMODE does not expect it.
  • [ ] Consider if SRTO_MININPUTBW option should be added.

Usage Examples

All required options are set at once together with the pacing mode. See test_socket_options.cpp for more examples.

struct SRT_PACEMODECFG
{
    SRT_PACINGMODE mode;
    int64_t bw;
    int32_t bw_ohead;
};
SRT_PACEMODECFG cfg = { SRT_PACING_INBW_SET, 80000000, 25 };
srt_setsockflag(sockid, SRTO_PACINGMODE, &cfg, sizeof cfg); // OK
struct SRT_PACEMODECFG
{
    SRT_PACINGMODE mode;
    int64_t bw;
    int32_t bw_ohead;
};
SRT_PACEMODECFG cfg = { SRT_PACING_INBW_SET, 0, 25 };
// The following function returns error: Invalid Param
// Because input BW value must be positive if INBW_SET mode is used
srt_setsockflag(sockid, SRTO_PACINGMODE, &cfg, sizeof cfg);

maxsharabayko avatar Sep 11 '20 11:09 maxsharabayko

Example Workflow Reporting Error

Example Workflow to consider. Should we report an error to instruct a user to properly configure the pacing mode?

Correct

srt_setsockflag(sockid, SRTO_INPUTBW, 80 MB, sizeof int);
srt_setsockflag(sockid, SRTO_PACINGMODE, SRT_PACING_INPUTBW, sizeof int);

Reporting Error

// Here we set the pacing mode, but the default SRTO_INPUTBW value is 0.
// Return -1 (error). Option is not applied.
// The user is protected from doing a mistake and not providing the input bitrate value.
srt_setsockflag(sockid, SRTO_PACINGMODE, SRT_PACING_INPUTBW, sizeof int);

Example Workflow Silencing Error

Example Workflow currently implemented. Do not report an error if the pacing mode is not configured properly. Set default values.

Correct

srt_setsockflag(sockid, SRTO_INPUTBW, 80 MB, sizeof int);
srt_setsockflag(sockid, SRTO_PACINGMODE, SRT_PACING_INPUTBW, sizeof int);

Silencing Error

// Here we set the pacing mode, but the default SRTO_INPUTBW value is 0.
// Return 0 (No Error). Option is applied. SRTO_INPUTBW is set to the default 1 Gbps.
// The user is not protected from doing a mistake and not providing the input bitrate value.
srt_setsockflag(sockid, SRTO_PACINGMODE, SRT_PACING_INPUTBW, sizeof int);

Example Workflow: Set All at Once

struct SRT_PACEMODECFG
{
    SRT_PACINGMODE mode;
    int64_t bw;
    int32_t bw_ohead;
};
SRT_PACEMODECFG cfg = { SRT_PACING_INPUTBW, 80000000, 25 };
srt_setsockflag(sockid, SRTO_PACINGMODE, &cfg, sizeof cfg); // OK
struct SRT_PACEMODECFG
{
    SRT_PACINGMODE mode;
    int64_t bw;
    int32_t bw_ohead;
};
SRT_PACEMODECFG cfg = { SRT_PACING_INPUTBW, 0, 25 };
srt_setsockflag(sockid, SRTO_PACINGMODE, &cfg, sizeof cfg); // Error: Invalid Param

maxsharabayko avatar Sep 11 '20 11:09 maxsharabayko

Additionally, I'd propose the following list:

  • SRT_PACING_UNLIM (default, args = {-1, -1}) - use default SRTO_MAXBW = -1 (1 Gbps);
  • SRT_PACING_MAXBW (args = {<maxbw>, -1}) - use user-provided SRTO_MAXBW value;
  • SRT_PACING_INPUTBW (args = {<inputbw>, <oheadbw=-1>}) - use user-provided SRTO_INPUTBW and SRTO_OHEADBW to calculate Max BW.
  • SRT_PACING_ESTINPUTBW (args = {-1, <oheadbw=-1>}) - use 0 for SRTO_INPUTBW so that ESTINPUTBW (internally estimated input bitrate at runtime) is used together with SRTO_OHEADBW to set Max BW limitation;
  • SRT_PACING_HYBRIDBW (args = {<inputbw>, <oheadbw=1>}) - Same as SRT_PACING_INPUTBW, but additionally internal "hybrid" flag is set to true, which means that INPUTBW or ESTINPUTBW is used as input bitrate, whichever is greater.

In all modes where <oheadbw> matters, the -1 value can be specified to enforce the current default value 25. The positive value for bw is mandatory for SRT_PACING_MAXBW, SRT_PACING_INPUTBW and SRT_PACING_HYBRIDBW, in all other modes it's ignored.

I'm thinking if the 0 value for <oheadbw> wouldn't be also a good enough default, as compilers usually allow less values for the structure to make them default-initialized with 0. So SRT_PACEMODECFG pm = {SRT_PACING_MAXBW, 2000000}; would be possibly allowed.

ethouris avatar Sep 14 '20 15:09 ethouris

From the docs, SRTO_MAXBW binding: pre (must be set prior to calling srt_connect() or srt_bind() and never changed thereafter).

TEV_INIT_RESET initialization event comes from two places:

  • a socket is created;
  • SRTO_MAXBW was updated while connected.

From the code of CUDT::updateCC, if init event type is not TEV_INIT_RESET and SRTO_MAXBW is nonzero, nothing is updated. It means that MAXBW can be changed in runtime.

if (event_source != TEV_INIT_RESET && m_llMaxBW)
{
    HLOGC(rslog.Debug, log << CONID() << "updateCC/TEV_INIT: non-RESET stage and m_llMaxBW already set...");
    // Don't change
}

maxsharabayko avatar Sep 15 '20 10:09 maxsharabayko

Consider adjusting pacing mode names to be more distinguishable:

  • SRT_PACING_DEFMAXBW -> SRT_PACING_MAXBWDFLT
  • SRT_PACING_MAXBW -> SRT_PACING_MAXBWSET
  • SRT_PACING_INPUTBW -> SRT_PACING_INPUTBWSET
  • SRT_PACING_ESTINPUTBW -> SRT_PACING_INPUTBWEST
  • SRT_PACING_HYBRIDBW -> SRT_PACING_INPUTBWADJ ???

maxsharabayko avatar Sep 15 '20 13:09 maxsharabayko