lavc/qsvenc: set HRD buffer size

Message ID 1512158651-25007-1-git-send-email-zhong.li@intel.com
State New
Headers show

Commit Message

Li, Zhong Dec. 1, 2017, 8:04 p.m.
Hypothetical Reference Decoding (HRD) model assumes that data flows into a
buffer of the fixed size BufferSizeInKB with a constant bitrate.
BufferSizeInKB represents the maximum possible size of any compressed frames.

Signed-off-by: Zhong Li <zhong.li@intel.com>
---
 libavcodec/qsvenc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Luca Barbato Nov. 28, 2017, 12:40 p.m. | #1
On 01/12/2017 21:04, Zhong Li wrote:
> Hypothetical Reference Decoding (HRD) model assumes that data flows into a
> buffer of the fixed size BufferSizeInKB with a constant bitrate.
> BufferSizeInKB represents the maximum possible size of any compressed frames.
> 
> Signed-off-by: Zhong Li <zhong.li@intel.com>
> ---
>   libavcodec/qsvenc.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/libavcodec/qsvenc.c b/libavcodec/qsvenc.c
> index 9db9eb3..cdddfd9 100644
> --- a/libavcodec/qsvenc.c
> +++ b/libavcodec/qsvenc.c
> @@ -437,7 +437,7 @@ static int init_video_param(AVCodecContext *avctx, QSVEncContext *q)
>       q->param.mfx.NumSlice           = avctx->slices;
>       q->param.mfx.NumRefFrame        = FFMAX(0, avctx->refs);
>       q->param.mfx.EncodedOrder       = 0;
> -    q->param.mfx.BufferSizeInKB     = 0;
> +    q->param.mfx.BufferSizeInKB     = (avctx->rc_buffer_size > 0) ? avctx->rc_buffer_size / 8000 : 0;
>   
>       desc = av_pix_fmt_desc_get(sw_format);
>       if (!desc)
> 

Sounds good to me.
Mark Thompson Nov. 28, 2017, 10:41 p.m. | #2
On 01/12/17 20:04, Zhong Li wrote:
> Hypothetical Reference Decoding (HRD) model assumes that data flows into a
> buffer of the fixed size BufferSizeInKB with a constant bitrate.
> BufferSizeInKB represents the maximum possible size of any compressed frames.
> 
> Signed-off-by: Zhong Li <zhong.li@intel.com>
> ---
>  libavcodec/qsvenc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/libavcodec/qsvenc.c b/libavcodec/qsvenc.c
> index 9db9eb3..cdddfd9 100644
> --- a/libavcodec/qsvenc.c
> +++ b/libavcodec/qsvenc.c
> @@ -437,7 +437,7 @@ static int init_video_param(AVCodecContext *avctx, QSVEncContext *q)
>      q->param.mfx.NumSlice           = avctx->slices;
>      q->param.mfx.NumRefFrame        = FFMAX(0, avctx->refs);
>      q->param.mfx.EncodedOrder       = 0;
> -    q->param.mfx.BufferSizeInKB     = 0;
> +    q->param.mfx.BufferSizeInKB     = (avctx->rc_buffer_size > 0) ? avctx->rc_buffer_size / 8000 : 0;
>  
>      desc = av_pix_fmt_desc_get(sw_format);
>      if (!desc)
> 

The Intel documentation says "BufferSizeInKB represents the maximum possible size of any compressed frames.".  Is that just confused?  Some of the surrounding text does suggest that it's the right thing.

Probably fine, but are you sure it should be set when in non-bitrate-targetted modes?  (The related HRD parameter InitialDelayInKB is set further down in CBR/VBR modes only.)

- Mark


Aside:  Please try to avoid sending messages from the future, it's confusing when things are sorted by time.  (You probably have the clock set wrong on a machine used for git send-email.)
Maxym Dmytrychenko Nov. 29, 2017, 10:13 a.m. | #3
good point about CBR... and this would require to get BufferSizeInKB
adjusted at the proper place, with InitialDelayInKB

On Tue, Nov 28, 2017 at 11:41 PM, Mark Thompson <sw@jkqxz.net> wrote:

> On 01/12/17 20:04, Zhong Li wrote:
> > Hypothetical Reference Decoding (HRD) model assumes that data flows into
> a
> > buffer of the fixed size BufferSizeInKB with a constant bitrate.
> > BufferSizeInKB represents the maximum possible size of any compressed
> frames.
> >
> > Signed-off-by: Zhong Li <zhong.li@intel.com>
> > ---
> >  libavcodec/qsvenc.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/libavcodec/qsvenc.c b/libavcodec/qsvenc.c
> > index 9db9eb3..cdddfd9 100644
> > --- a/libavcodec/qsvenc.c
> > +++ b/libavcodec/qsvenc.c
> > @@ -437,7 +437,7 @@ static int init_video_param(AVCodecContext *avctx,
> QSVEncContext *q)
> >      q->param.mfx.NumSlice           = avctx->slices;
> >      q->param.mfx.NumRefFrame        = FFMAX(0, avctx->refs);
> >      q->param.mfx.EncodedOrder       = 0;
> > -    q->param.mfx.BufferSizeInKB     = 0;
> > +    q->param.mfx.BufferSizeInKB     = (avctx->rc_buffer_size > 0) ?
> avctx->rc_buffer_size / 8000 : 0;
> >
> >      desc = av_pix_fmt_desc_get(sw_format);
> >      if (!desc)
> >
>
> The Intel documentation says "BufferSizeInKB represents the maximum
> possible size of any compressed frames.".  Is that just confused?  Some of
> the surrounding text does suggest that it's the right thing.
>
> Probably fine, but are you sure it should be set when in
> non-bitrate-targetted modes?  (The related HRD parameter InitialDelayInKB
> is set further down in CBR/VBR modes only.)
>
> - Mark
>
>
> Aside:  Please try to avoid sending messages from the future, it's
> confusing when things are sorted by time.  (You probably have the clock set
> wrong on a machine used for git send-email.)
> _______________________________________________
> libav-devel mailing list
> libav-devel@libav.org
> https://lists.libav.org/mailman/listinfo/libav-devel
Li, Zhong Nov. 29, 2017, 1:19 p.m. | #4
> The Intel documentation says "BufferSizeInKB represents the maximum

> possible size of any compressed frames.".  Is that just confused?  Some of

> the surrounding text does suggest that it's the right thing.

> 

> Probably fine, but are you sure it should be set when in non-bitrate-targetted

> modes?  (The related HRD parameter InitialDelayInKB is set further down in

> CBR/VBR modes only.)

> 

Yes, it is useless if HRD/VBV compliance is not required. 
I note AVBR is also HRD/VBV compliance required.
So BufferSizeInKB/ InitialDelayInKB should also be set for AVBR.

> - Mark

> 

> 

> Aside:  Please try to avoid sending messages from the future, it's confusing

> when things are sorted by time.  (You probably have the clock set wrong on

> a machine used for git send-email.)


Sure, thanks for remind.
Li, Zhong Nov. 29, 2017, 1:21 p.m. | #5
> -----Original Message-----

> From: libav-devel [mailto:libav-devel-bounces@libav.org] On Behalf Of

> Maxym Dmytrychenko

> Sent: Wednesday, November 29, 2017 6:14 PM

> To: libav development <libav-devel@libav.org>

> Subject: Re: [libav-devel] [PATCH] lavc/qsvenc: set HRD buffer size

> 

> good point about CBR... and this would require to get BufferSizeInKB

> adjusted at the proper place, with InitialDelayInKB


Yeah, patch will be updated. Thanks for review.

Patch

diff --git a/libavcodec/qsvenc.c b/libavcodec/qsvenc.c
index 9db9eb3..cdddfd9 100644
--- a/libavcodec/qsvenc.c
+++ b/libavcodec/qsvenc.c
@@ -437,7 +437,7 @@  static int init_video_param(AVCodecContext *avctx, QSVEncContext *q)
     q->param.mfx.NumSlice           = avctx->slices;
     q->param.mfx.NumRefFrame        = FFMAX(0, avctx->refs);
     q->param.mfx.EncodedOrder       = 0;
-    q->param.mfx.BufferSizeInKB     = 0;
+    q->param.mfx.BufferSizeInKB     = (avctx->rc_buffer_size > 0) ? avctx->rc_buffer_size / 8000 : 0;
 
     desc = av_pix_fmt_desc_get(sw_format);
     if (!desc)