[FFmpeg-devel,03/10] lavc/libopenh264enc: add default gop size and bit rate

Signed-off-by: Linjie Fu <linjie.fu@intel.com>
---
 libavcodec/libopenh264enc.c | 2 ++
 1 file changed, 2 insertions(+)

Quoting Linjie Fu (2020-04-06 13:14:46)
> Signed-off-by: Linjie Fu <linjie.fu@intel.com>
> ---
>  libavcodec/libopenh264enc.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/libavcodec/libopenh264enc.c b/libavcodec/libopenh264enc.c
> index c7ae5b1..3ff5be7 100644
> --- a/libavcodec/libopenh264enc.c
> +++ b/libavcodec/libopenh264enc.c
> @@ -335,6 +335,8 @@ static int svc_encode_frame(AVCodecContext *avctx, AVPacket *avpkt,
>  }
>  
>  static const AVCodecDefault svc_enc_defaults[] = {
> +    { "b",         "2M"    },
> +    { "g",         "120"   },

Why these values specifically? What happens if we leave them unset?

> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> Anton Khirnov
> Sent: Friday, April 10, 2020 18:14
> To: FFmpeg development discussions and patches <ffmpeg-
> devel@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [PATCH 03/10] lavc/libopenh264enc: add
> default gop size and bit rate
> 
> Quoting Linjie Fu (2020-04-06 13:14:46)
> > Signed-off-by: Linjie Fu <linjie.fu@intel.com>
> > ---
> >  libavcodec/libopenh264enc.c | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/libavcodec/libopenh264enc.c b/libavcodec/libopenh264enc.c
> > index c7ae5b1..3ff5be7 100644
> > --- a/libavcodec/libopenh264enc.c
> > +++ b/libavcodec/libopenh264enc.c
> > @@ -335,6 +335,8 @@ static int svc_encode_frame(AVCodecContext
> *avctx, AVPacket *avpkt,
> >  }
> >
> >  static const AVCodecDefault svc_enc_defaults[] = {
> > +    { "b",         "2M"    },
> > +    { "g",         "120"   },
> 
> Why these values specifically? 

According to the default settings in nvenc (b = 2M) and vaapi encoder for h264 (g = 120).

>What happens if we leave them unset?

It would be 200kbps bitrate with gop size = 12.

There would be too much IDR frames with in small bitrate, which leads to
a poor quality. 

 (I should have added such statements in the commit message)

- Linjie

Quoting Fu, Linjie (2020-04-10 15:33:04)
> > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> > Anton Khirnov
> > Sent: Friday, April 10, 2020 18:14
> > To: FFmpeg development discussions and patches <ffmpeg-
> > devel@ffmpeg.org>
> > Subject: Re: [FFmpeg-devel] [PATCH 03/10] lavc/libopenh264enc: add
> > default gop size and bit rate
> > 
> > Quoting Linjie Fu (2020-04-06 13:14:46)
> > > Signed-off-by: Linjie Fu <linjie.fu@intel.com>
> > > ---
> > >  libavcodec/libopenh264enc.c | 2 ++
> > >  1 file changed, 2 insertions(+)
> > >
> > > diff --git a/libavcodec/libopenh264enc.c b/libavcodec/libopenh264enc.c
> > > index c7ae5b1..3ff5be7 100644
> > > --- a/libavcodec/libopenh264enc.c
> > > +++ b/libavcodec/libopenh264enc.c
> > > @@ -335,6 +335,8 @@ static int svc_encode_frame(AVCodecContext
> > *avctx, AVPacket *avpkt,
> > >  }
> > >
> > >  static const AVCodecDefault svc_enc_defaults[] = {
> > > +    { "b",         "2M"    },
> > > +    { "g",         "120"   },
> > 
> > Why these values specifically? 
> 
> According to the default settings in nvenc (b = 2M) and vaapi encoder for h264 (g = 120).
> 
> >What happens if we leave them unset?
> 
> It would be 200kbps bitrate with gop size = 12.
> 
> There would be too much IDR frames with in small bitrate, which leads to
> a poor quality. 
> 
>  (I should have added such statements in the commit message)

I mean we could override the defaults to "-1" like is done for e.g.
x264, then we can distinguish between "the user specified the bitrate to
be <x>" vs. "the user did not specify anything about the target
bitrate".
And if the user did not specify the bitrate we could leave libopenh264
to choose bitrate on its own or use some other rate limiting mechanism
like constant QP or something CRF-like (don't know if it has that).

> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> Anton Khirnov
> Sent: Saturday, April 11, 2020 16:43
> To: FFmpeg development discussions and patches <ffmpeg-
> devel@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [PATCH 03/10] lavc/libopenh264enc: add
> default gop size and bit rate
> 
> Quoting Fu, Linjie (2020-04-10 15:33:04)
> > > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> > > Anton Khirnov
> > > Sent: Friday, April 10, 2020 18:14
> > > To: FFmpeg development discussions and patches <ffmpeg-
> > > devel@ffmpeg.org>
> > > Subject: Re: [FFmpeg-devel] [PATCH 03/10] lavc/libopenh264enc: add
> > > default gop size and bit rate
> > >
> > > Quoting Linjie Fu (2020-04-06 13:14:46)
> > > > Signed-off-by: Linjie Fu <linjie.fu@intel.com>
> > > > ---
> > > >  libavcodec/libopenh264enc.c | 2 ++
> > > >  1 file changed, 2 insertions(+)
> > > >
> > > > diff --git a/libavcodec/libopenh264enc.c b/libavcodec/libopenh264enc.c
> > > > index c7ae5b1..3ff5be7 100644
> > > > --- a/libavcodec/libopenh264enc.c
> > > > +++ b/libavcodec/libopenh264enc.c
> > > > @@ -335,6 +335,8 @@ static int svc_encode_frame(AVCodecContext
> > > *avctx, AVPacket *avpkt,
> > > >  }
> > > >
> > > >  static const AVCodecDefault svc_enc_defaults[] = {
> > > > +    { "b",         "2M"    },
> > > > +    { "g",         "120"   },
> > >
> > > Why these values specifically?
> >
> > According to the default settings in nvenc (b = 2M) and vaapi encoder for
> h264 (g = 120).
> >
> > >What happens if we leave them unset?
> >
> > It would be 200kbps bitrate with gop size = 12.
> >
> > There would be too much IDR frames with in small bitrate, which leads to
> > a poor quality.
> >
> >  (I should have added such statements in the commit message)
> 
> I mean we could override the defaults to "-1" like is done for e.g.
> x264, then we can distinguish between "the user specified the bitrate to
> be <x>" vs. "the user did not specify anything about the target
> bitrate".

Indeed, I noticed this while attempting to select rc_mode by avctx->bit_rate.
It makes sense to distinguish them, and I've changed it.

> And if the user did not specify the bitrate we could leave libopenh264
> to choose bitrate on its own or use some other rate limiting mechanism
> like constant QP or something CRF-like (don't know if it has that).

I'm afraid it doesn't have one if we leave the bitrate unset:
[OpenH264] this = 0x0x55a61f8dddd0, Error: Invalid bitrate settings in total configure, bitrate= 0

so I'll assign a default target bitrate to 2M.

- Linjie