mbox series

[FFmpeg-devel,v5,0/5] enhancement for libopenh264

Message ID 1588065363-7104-1-git-send-email-linjie.fu@intel.com
Headers show
Series enhancement for libopenh264 | expand

Message

Fu, Linjie April 28, 2020, 9:15 a.m. UTC 
Separate the set, and updated a v5 version as suggested.

Linjie Fu (5):
  lavc/libopenh264enc: Add qmin/qmax support
  lavc/libopenh264enc: add default gop size and bit rate
  lavc/libopenh264enc: add bit rate control select support
  lavc/libopenh264enc: prompt slice number changing inside libopenh264
  lavc/libopenh264enc: set slice_mode option to deprecated

 libavcodec/libopenh264enc.c | 45 +++++++++++++++++++++++++++++++++++++++++----
 libavcodec/version.h        |  3 +++
 2 files changed, 44 insertions(+), 4 deletions(-)

Comments

Martin Storsjö April 28, 2020, 10:29 a.m. UTC | #1 
On Tue, 28 Apr 2020, Linjie Fu wrote:

> It would be 200kbps bitrate with gop size = 12 by default
> which generated too many IDR frames in rather low bit rate.
> The quality would be poor.
>
> Set these default values to -1 to check whether it's specified
> by user explicitly.
>
> Use 2Mbps bitrate as nvenc sugguested, and leave gop size
> untouched in libopenh264.
>
> Signed-off-by: Linjie Fu <linjie.fu@intel.com>
> ---
> libavcodec/libopenh264enc.c | 9 +++++++--
> 1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/libavcodec/libopenh264enc.c b/libavcodec/libopenh264enc.c
> index 1b6b53a..0951412 100644
> --- a/libavcodec/libopenh264enc.c
> +++ b/libavcodec/libopenh264enc.c
> @@ -37,6 +37,8 @@
> #define SM_SIZELIMITED_SLICE SM_DYN_SLICE
> #endif
> 
> +#define TARGET_BITRATE_DEFAULT 2*1000*1000
> +
> typedef struct SVCContext {
>     const AVClass *av_class;
>     ISVCEncoder *encoder;
> @@ -132,7 +134,7 @@ FF_ENABLE_DEPRECATION_WARNINGS
>     param.fMaxFrameRate              = 1/av_q2d(avctx->time_base);
>     param.iPicWidth                  = avctx->width;
>     param.iPicHeight                 = avctx->height;
> -    param.iTargetBitrate             = avctx->bit_rate;
> +    param.iTargetBitrate             = avctx->bit_rate > 0 ? avctx->bit_rate : TARGET_BITRATE_DEFAULT;
>     param.iMaxBitrate                = FFMAX(avctx->rc_max_rate, avctx->bit_rate);
>     param.iRCMode                    = RC_QUALITY_MODE;
>     // QP = 0 is not well supported, so clip to (1, 51)
> @@ -148,7 +150,8 @@ FF_ENABLE_DEPRECATION_WARNINGS
>     param.bEnableFrameSkip           = s->skip_frames;
>     param.bEnableLongTermReference   = 0;
>     param.iLtrMarkPeriod             = 30;
> -    param.uiIntraPeriod              = avctx->gop_size;
> +    if (avctx->gop_size)
> +        param.uiIntraPeriod          = avctx->gop_size;

This should be gop_size > 0, or maybe >= 0. Otherwise we'll end up setting 
the default -1.

// Martin
Fu, Linjie April 28, 2020, 4:21 p.m. UTC | #2 
> From: Martin Storsjö <martin@martin.st>
> Sent: Tuesday, April 28, 2020 18:29
> To: FFmpeg development discussions and patches <ffmpeg-
> devel@ffmpeg.org>
> Cc: Fu, Linjie <linjie.fu@intel.com>
> Subject: Re: [FFmpeg-devel] [PATCH v5 2/5] lavc/libopenh264enc: add
> default gop size and bit rate
> 
> On Tue, 28 Apr 2020, Linjie Fu wrote:
> 
> > It would be 200kbps bitrate with gop size = 12 by default
> > which generated too many IDR frames in rather low bit rate.
> > The quality would be poor.
> >
> > Set these default values to -1 to check whether it's specified
> > by user explicitly.
> >
> > Use 2Mbps bitrate as nvenc sugguested, and leave gop size
> > untouched in libopenh264.
> >
> > Signed-off-by: Linjie Fu <linjie.fu@intel.com>
> > ---
> > libavcodec/libopenh264enc.c | 9 +++++++--
> > 1 file changed, 7 insertions(+), 2 deletions(-)
> >
> > diff --git a/libavcodec/libopenh264enc.c b/libavcodec/libopenh264enc.c
> > index 1b6b53a..0951412 100644
> > --- a/libavcodec/libopenh264enc.c
> > +++ b/libavcodec/libopenh264enc.c
> > @@ -37,6 +37,8 @@
> > #define SM_SIZELIMITED_SLICE SM_DYN_SLICE
> > #endif
> >
> > +#define TARGET_BITRATE_DEFAULT 2*1000*1000
> > +
> > typedef struct SVCContext {
> >     const AVClass *av_class;
> >     ISVCEncoder *encoder;
> > @@ -132,7 +134,7 @@ FF_ENABLE_DEPRECATION_WARNINGS
> >     param.fMaxFrameRate              = 1/av_q2d(avctx->time_base);
> >     param.iPicWidth                  = avctx->width;
> >     param.iPicHeight                 = avctx->height;
> > -    param.iTargetBitrate             = avctx->bit_rate;
> > +    param.iTargetBitrate             = avctx->bit_rate > 0 ? avctx->bit_rate :
> TARGET_BITRATE_DEFAULT;
> >     param.iMaxBitrate                = FFMAX(avctx->rc_max_rate, avctx-
> >bit_rate);
> >     param.iRCMode                    = RC_QUALITY_MODE;
> >     // QP = 0 is not well supported, so clip to (1, 51)
> > @@ -148,7 +150,8 @@ FF_ENABLE_DEPRECATION_WARNINGS
> >     param.bEnableFrameSkip           = s->skip_frames;
> >     param.bEnableLongTermReference   = 0;
> >     param.iLtrMarkPeriod             = 30;
> > -    param.uiIntraPeriod              = avctx->gop_size;
> > +    if (avctx->gop_size)
> > +        param.uiIntraPeriod          = avctx->gop_size;
> 
> This should be gop_size > 0, or maybe >= 0. Otherwise we'll end up setting
> the default -1.
> 
Ops, fixed and thanks for pointing this out.