| Message ID | 1588065363-7104-4-git-send-email-linjie.fu@intel.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | enhancement for libopenh264 | expand |
| Context | Check | Description |
|---|---|---|
| andriy/default | pending | |
| andriy/make | success | Make finished |
| andriy/make_fate | success | Make fate finished |
On Tue, 28 Apr 2020, Linjie Fu wrote: > RC_BITRATE_MODE: > set BITS_EXCEEDED to iCurrentBitsLevel and allows QP adjust > in RcCalculatePictureQp(). > > RC_BUFFERBASED_MODE: > use buffer status to adjust the video quality, introduced in > release 1.2. > > RC_TIMESTAMP_MODE: > bit rate control based on timestamp, introduced in release 1.4. > > Default to use RC_QUALITY_MODE. > > Signed-off-by: Linjie Fu <linjie.fu@intel.com> > --- > libavcodec/libopenh264enc.c | 15 +++++++++++++++ > 1 file changed, 15 insertions(+) > > diff --git a/libavcodec/libopenh264enc.c b/libavcodec/libopenh264enc.c > index 0951412..93d3de2 100644 > --- a/libavcodec/libopenh264enc.c > +++ b/libavcodec/libopenh264enc.c > @@ -49,6 +49,9 @@ typedef struct SVCContext { > int skip_frames; > int skipped; > int cabac; > + > + // rate control mode > + int rc_mode; > } SVCContext; > > #define OFFSET(x) offsetof(SVCContext, x) > @@ -73,6 +76,18 @@ static const AVOption options[] = { > { "max_nal_size", "set maximum NAL size in bytes", OFFSET(max_nal_size), AV_OPT_TYPE_INT, { .i64 = 0 }, 0, INT_MAX, VE }, > { "allow_skip_frames", "allow skipping frames to hit the target bitrate", OFFSET(skip_frames), AV_OPT_TYPE_BOOL, { .i64 = 0 }, 0, 1, VE }, > { "cabac", "Enable cabac", OFFSET(cabac), AV_OPT_TYPE_INT, { .i64 = 0 }, 0, 1, VE }, > + > + { "rc_mode", "Select rate control mode", OFFSET(rc_mode), AV_OPT_TYPE_INT, { .i64 = RC_QUALITY_MODE }, RC_OFF_MODE, RC_TIMESTAMP_MODE, VE, "rc_mode" }, > + { "off", "bit rate control off", 0, AV_OPT_TYPE_CONST, { .i64 = RC_OFF_MODE }, 0, 0, VE, "rc_mode" }, > + { "quality", "quality mode", 0, AV_OPT_TYPE_CONST, { .i64 = RC_QUALITY_MODE }, 0, 0, VE, "rc_mode" }, > + { "bitrate", "bitrate mode", 0, AV_OPT_TYPE_CONST, { .i64 = RC_BITRATE_MODE }, 0, 0, VE, "rc_mode" }, > +#if OPENH264_VER_AT_LEAST(1, 2) > + { "buffer", "using buffer status to adjust the video quality (no bitrate control)", 0, AV_OPT_TYPE_CONST, { .i64 = RC_BUFFERBASED_MODE }, 0, 0, VE, "rc_mode" }, > +#endif We don't need checks for 1.2 - the initial version of openh264 that we support is 1.3, so we only need checks for 1.4 and newer. > +#if OPENH264_VER_AT_LEAST(1, 4) > + { "timestamp", "bit rate control based on timestamp", 0, AV_OPT_TYPE_CONST, { .i64 = RC_TIMESTAMP_MODE }, 0, 0, VE, "rc_mode" }, > +#endif > + > { NULL } > }; This commit seems to lack something that actually sets the rc_mode value in the parameters struct though - wasn't that part of the previous version of the patch? // Martin
> From: Martin Storsjö <martin@martin.st> > Sent: Tuesday, April 28, 2020 18:31 > To: FFmpeg development discussions and patches <ffmpeg- > devel@ffmpeg.org> > Cc: Fu, Linjie <linjie.fu@intel.com> > Subject: Re: [FFmpeg-devel] [PATCH v5 3/5] lavc/libopenh264enc: add bit > rate control select support > > On Tue, 28 Apr 2020, Linjie Fu wrote: > > > RC_BITRATE_MODE: > > set BITS_EXCEEDED to iCurrentBitsLevel and allows QP adjust > > in RcCalculatePictureQp(). > > > > RC_BUFFERBASED_MODE: > > use buffer status to adjust the video quality, introduced in > > release 1.2. > > > > RC_TIMESTAMP_MODE: > > bit rate control based on timestamp, introduced in release 1.4. > > > > Default to use RC_QUALITY_MODE. > > > > Signed-off-by: Linjie Fu <linjie.fu@intel.com> > > --- > > libavcodec/libopenh264enc.c | 15 +++++++++++++++ > > 1 file changed, 15 insertions(+) > > > > diff --git a/libavcodec/libopenh264enc.c b/libavcodec/libopenh264enc.c > > index 0951412..93d3de2 100644 > > --- a/libavcodec/libopenh264enc.c > > +++ b/libavcodec/libopenh264enc.c > > @@ -49,6 +49,9 @@ typedef struct SVCContext { > > int skip_frames; > > int skipped; > > int cabac; > > + > > + // rate control mode > > + int rc_mode; > > } SVCContext; > > > > #define OFFSET(x) offsetof(SVCContext, x) > > @@ -73,6 +76,18 @@ static const AVOption options[] = { > > { "max_nal_size", "set maximum NAL size in bytes", > OFFSET(max_nal_size), AV_OPT_TYPE_INT, { .i64 = 0 }, 0, INT_MAX, VE }, > > { "allow_skip_frames", "allow skipping frames to hit the target bitrate", > OFFSET(skip_frames), AV_OPT_TYPE_BOOL, { .i64 = 0 }, 0, 1, VE }, > > { "cabac", "Enable cabac", OFFSET(cabac), AV_OPT_TYPE_INT, { .i64 = 0 }, > 0, 1, VE }, > > + > > + { "rc_mode", "Select rate control mode", OFFSET(rc_mode), > AV_OPT_TYPE_INT, { .i64 = RC_QUALITY_MODE }, RC_OFF_MODE, > RC_TIMESTAMP_MODE, VE, "rc_mode" }, > > + { "off", "bit rate control off", 0, > AV_OPT_TYPE_CONST, { .i64 = RC_OFF_MODE }, 0, 0, VE, "rc_mode" }, > > + { "quality", "quality mode", 0, > AV_OPT_TYPE_CONST, { .i64 = RC_QUALITY_MODE }, 0, 0, VE, "rc_mode" }, > > + { "bitrate", "bitrate mode", 0, > AV_OPT_TYPE_CONST, { .i64 = RC_BITRATE_MODE }, 0, 0, VE, "rc_mode" }, > > +#if OPENH264_VER_AT_LEAST(1, 2) > > + { "buffer", "using buffer status to adjust the video quality (no bitrate > control)", 0, AV_OPT_TYPE_CONST, { .i64 = RC_BUFFERBASED_MODE }, 0, 0, > VE, "rc_mode" }, > > +#endif > > We don't need checks for 1.2 - the initial version of openh264 that we > support is 1.3, so we only need checks for 1.4 and newer. Ahh, didn't noticed this until checked the commit message of 8a3d9ca603f4d15ecaa9ca379cbaab4ecaec8ce4. IMHO it seems to be better if we add this version check in the configure as well. (Did a quick verification with version modified in pkgconfig to 1.1.0, configure is still good for libopenh264 ) Will send another fix for this if no against. > > +#if OPENH264_VER_AT_LEAST(1, 4) > > + { "timestamp", "bit rate control based on timestamp", > 0, AV_OPT_TYPE_CONST, { .i64 = RC_TIMESTAMP_MODE }, 0, 0, VE, > "rc_mode" }, > > +#endif > > + > > { NULL } > > }; > > This commit seems to lack something that actually sets the rc_mode value > in the parameters struct though - wasn't that part of the previous version > of the patch? > Did you mean setting rc_mode through parameters like bit_rate/qp? This patch enables explicitly setting the rc_mode as part of that logic. As to full enhancement, it's WIP and I'm still considering some parameters like -qp to specify exact QP. (libopenh264 seems to only accept iMax/MinQp in RC_QUALITY_MODE) For specific QP settings like -qp 26, we have to select RC_OFF_MODE and specify QP by iDLayerQp in sSpatialLayers, which seems a little bit confusing. (setting -qp 26 which leads to a non-quality mode) - Linjie
> From: Martin Storsjö <martin@martin.st> > Sent: Wednesday, April 29, 2020 03:18 > To: Fu, Linjie <linjie.fu@intel.com> > Cc: FFmpeg development discussions and patches <ffmpeg- > devel@ffmpeg.org> > Subject: RE: [FFmpeg-devel] [PATCH v5 3/5] lavc/libopenh264enc: add bit > rate control select support > > On Tue, 28 Apr 2020, Fu, Linjie wrote: > > >> From: Martin Storsjö <martin@martin.st> > >> Sent: Tuesday, April 28, 2020 18:31 > >> To: FFmpeg development discussions and patches <ffmpeg- > >> devel@ffmpeg.org> > >> Cc: Fu, Linjie <linjie.fu@intel.com> > >> Subject: Re: [FFmpeg-devel] [PATCH v5 3/5] lavc/libopenh264enc: add bit > >> rate control select support > >> > >> On Tue, 28 Apr 2020, Linjie Fu wrote: > >> > >> We don't need checks for 1.2 - the initial version of openh264 that we > >> support is 1.3, so we only need checks for 1.4 and newer. > > > > Ahh, didn't noticed this until checked the commit message of > > 8a3d9ca603f4d15ecaa9ca379cbaab4ecaec8ce4. > > > > IMHO it seems to be better if we add this version check in the configure as > well. > > (Did a quick verification with version modified in pkgconfig to 1.1.0, > configure is > > still good for libopenh264 ) > > We don't need an explicit version check for 1.3 in configure - we require > that WelsGetCodecVersion can be found in the configure check, and that > function was added in 1.3 (explicitly for the purpose so that we can check > that the version that we have linked is the same as we compiled against). > Checked and confirmed that you are right, WelsGetCodecVersion is enough for 1.3.0 version check, thanks for elaborations. > > > >>> +#if OPENH264_VER_AT_LEAST(1, 4) > >>> + { "timestamp", "bit rate control based on timestamp", > >> 0, AV_OPT_TYPE_CONST, { .i64 = RC_TIMESTAMP_MODE }, 0, 0, VE, > >> "rc_mode" }, > >>> +#endif > >>> + > >>> { NULL } > >>> }; > >> > >> This commit seems to lack something that actually sets the rc_mode value > >> in the parameters struct though - wasn't that part of the previous version > >> of the patch? > >> > > Did you mean setting rc_mode through parameters like bit_rate/qp? > > This patch enables explicitly setting the rc_mode as part of that logic. > > No, I didn't mean that. > > The previous version of this patch had the following change: > > - param.iRCMode = RC_QUALITY_MODE; > + param.iRCMode = s->rc_mode; > > Now I don't find that part any longer in this patch - and without that > change, the rest of the commit is pretty pointless, no? it's somehow missed while rebasing the set, thanks. - Linjie
diff --git a/libavcodec/libopenh264enc.c b/libavcodec/libopenh264enc.c index 0951412..93d3de2 100644 --- a/libavcodec/libopenh264enc.c +++ b/libavcodec/libopenh264enc.c @@ -49,6 +49,9 @@ typedef struct SVCContext { int skip_frames; int skipped; int cabac; + + // rate control mode + int rc_mode; } SVCContext; #define OFFSET(x) offsetof(SVCContext, x) @@ -73,6 +76,18 @@ static const AVOption options[] = { { "max_nal_size", "set maximum NAL size in bytes", OFFSET(max_nal_size), AV_OPT_TYPE_INT, { .i64 = 0 }, 0, INT_MAX, VE }, { "allow_skip_frames", "allow skipping frames to hit the target bitrate", OFFSET(skip_frames), AV_OPT_TYPE_BOOL, { .i64 = 0 }, 0, 1, VE }, { "cabac", "Enable cabac", OFFSET(cabac), AV_OPT_TYPE_INT, { .i64 = 0 }, 0, 1, VE }, + + { "rc_mode", "Select rate control mode", OFFSET(rc_mode), AV_OPT_TYPE_INT, { .i64 = RC_QUALITY_MODE }, RC_OFF_MODE, RC_TIMESTAMP_MODE, VE, "rc_mode" }, + { "off", "bit rate control off", 0, AV_OPT_TYPE_CONST, { .i64 = RC_OFF_MODE }, 0, 0, VE, "rc_mode" }, + { "quality", "quality mode", 0, AV_OPT_TYPE_CONST, { .i64 = RC_QUALITY_MODE }, 0, 0, VE, "rc_mode" }, + { "bitrate", "bitrate mode", 0, AV_OPT_TYPE_CONST, { .i64 = RC_BITRATE_MODE }, 0, 0, VE, "rc_mode" }, +#if OPENH264_VER_AT_LEAST(1, 2) + { "buffer", "using buffer status to adjust the video quality (no bitrate control)", 0, AV_OPT_TYPE_CONST, { .i64 = RC_BUFFERBASED_MODE }, 0, 0, VE, "rc_mode" }, +#endif +#if OPENH264_VER_AT_LEAST(1, 4) + { "timestamp", "bit rate control based on timestamp", 0, AV_OPT_TYPE_CONST, { .i64 = RC_TIMESTAMP_MODE }, 0, 0, VE, "rc_mode" }, +#endif + { NULL } };
RC_BITRATE_MODE: set BITS_EXCEEDED to iCurrentBitsLevel and allows QP adjust in RcCalculatePictureQp(). RC_BUFFERBASED_MODE: use buffer status to adjust the video quality, introduced in release 1.2. RC_TIMESTAMP_MODE: bit rate control based on timestamp, introduced in release 1.4. Default to use RC_QUALITY_MODE. Signed-off-by: Linjie Fu <linjie.fu@intel.com> --- libavcodec/libopenh264enc.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+)