[FFmpeg-devel,07/12] ratecontrol: fix stuffing bits for 30000/1001 fps one frame vbv

Submitted by Baptiste Coudurier on July 4, 2018, 6:35 p.m.

Details

Message ID 20180704183514.71654-7-baptiste.coudurier@gmail.com
State New
Headers show

Commit Message

Baptiste Coudurier July 4, 2018, 6:35 p.m.
---
 libavcodec/ratecontrol.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

Comments

Michael Niedermayer July 5, 2018, 10:43 a.m.
On Wed, Jul 04, 2018 at 11:35:09AM -0700, Baptiste Coudurier wrote:
> ---
>  libavcodec/ratecontrol.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/libavcodec/ratecontrol.c b/libavcodec/ratecontrol.c
> index 49d169ba25..28bdddbad1 100644
> --- a/libavcodec/ratecontrol.c
> +++ b/libavcodec/ratecontrol.c
> @@ -705,16 +705,16 @@ int ff_vbv_update(MpegEncContext *s, int frame_size)
>          rcc->buffer_index += av_clip(left, min_rate, max_rate);
>  
>          if (rcc->buffer_index > buffer_size) {
> -            int stuffing = ceil((rcc->buffer_index - buffer_size) / 8);
> +            int stuffing = rcc->buffer_index - buffer_size;
>  
> -            if (stuffing < 4 && s->codec_id == AV_CODEC_ID_MPEG4)
> -                stuffing = 4;
> -            rcc->buffer_index -= 8 * stuffing;
> +            if (stuffing < 32 && s->codec_id == AV_CODEC_ID_MPEG4)
> +                stuffing = 32;
> +            rcc->buffer_index -= stuffing;
>  
>              if (s->avctx->debug & FF_DEBUG_RC)
> -                av_log(s->avctx, AV_LOG_DEBUG, "stuffing %d bytes\n", stuffing);
> +                av_log(s->avctx, AV_LOG_DEBUG, "stuffing %d bytes\n", stuffing>>3);
>  
> -            return stuffing;
> +            return stuffing>>3;
>          }

This would break VBV for most codecs
buffer_index is a double, the if() condition is triggered if the current frame
is too small. All the rounding has to be up as in the ceil() that was there.
The stuffing that is added, is minimal, so a decrease would be expected to
break cases occasionally

Can you explain how this code fails, in what exact case 
(max/min rate/ buffer_size/fps) ?

[...]
Baptiste Coudurier July 5, 2018, 3:23 p.m.
Hi Michael

On Thu, Jul 5, 2018 at 3:43 AM, Michael Niedermayer <michael@niedermayer.cc>
wrote:

> On Wed, Jul 04, 2018 at 11:35:09AM -0700, Baptiste Coudurier wrote:
> > ---
> >  libavcodec/ratecontrol.c | 12 ++++++------
> >  1 file changed, 6 insertions(+), 6 deletions(-)
> >
> > diff --git a/libavcodec/ratecontrol.c b/libavcodec/ratecontrol.c
> > index 49d169ba25..28bdddbad1 100644
> > --- a/libavcodec/ratecontrol.c
> > +++ b/libavcodec/ratecontrol.c
> > @@ -705,16 +705,16 @@ int ff_vbv_update(MpegEncContext *s, int
> frame_size)
> >          rcc->buffer_index += av_clip(left, min_rate, max_rate);
> >
> >          if (rcc->buffer_index > buffer_size) {
> > -            int stuffing = ceil((rcc->buffer_index - buffer_size) / 8);
> > +            int stuffing = rcc->buffer_index - buffer_size;
> >
> > -            if (stuffing < 4 && s->codec_id == AV_CODEC_ID_MPEG4)
> > -                stuffing = 4;
> > -            rcc->buffer_index -= 8 * stuffing;
> > +            if (stuffing < 32 && s->codec_id == AV_CODEC_ID_MPEG4)
> > +                stuffing = 32;
> > +            rcc->buffer_index -= stuffing;
> >
> >              if (s->avctx->debug & FF_DEBUG_RC)
> > -                av_log(s->avctx, AV_LOG_DEBUG, "stuffing %d bytes\n",
> stuffing);
> > +                av_log(s->avctx, AV_LOG_DEBUG, "stuffing %d bytes\n",
> stuffing>>3);
> >
> > -            return stuffing;
> > +            return stuffing>>3;
> >          }
>
> This would break VBV for most codecs
> buffer_index is a double, the if() condition is triggered if the current
> frame
> is too small. All the rounding has to be up as in the ceil() that was
> there.
> The stuffing that is added, is minimal, so a decrease would be expected to
> break cases occasionally
>
> Can you explain how this code fails, in what exact case
> (max/min rate/ buffer_size/fps) ?
>

Basically It doesn't work for IMX50, requiring exact frame size of 208541
bytes at 30000/1001 fps
and vbv delay of 3003
Michael Niedermayer July 5, 2018, 9:49 p.m.
On Thu, Jul 05, 2018 at 08:23:24AM -0700, Baptiste Coudurier wrote:
> Hi Michael
> 
> On Thu, Jul 5, 2018 at 3:43 AM, Michael Niedermayer <michael@niedermayer.cc>
> wrote:
> 
> > On Wed, Jul 04, 2018 at 11:35:09AM -0700, Baptiste Coudurier wrote:
> > > ---
> > >  libavcodec/ratecontrol.c | 12 ++++++------
> > >  1 file changed, 6 insertions(+), 6 deletions(-)
> > >
> > > diff --git a/libavcodec/ratecontrol.c b/libavcodec/ratecontrol.c
> > > index 49d169ba25..28bdddbad1 100644
> > > --- a/libavcodec/ratecontrol.c
> > > +++ b/libavcodec/ratecontrol.c
> > > @@ -705,16 +705,16 @@ int ff_vbv_update(MpegEncContext *s, int
> > frame_size)
> > >          rcc->buffer_index += av_clip(left, min_rate, max_rate);
> > >
> > >          if (rcc->buffer_index > buffer_size) {
> > > -            int stuffing = ceil((rcc->buffer_index - buffer_size) / 8);
> > > +            int stuffing = rcc->buffer_index - buffer_size;
> > >
> > > -            if (stuffing < 4 && s->codec_id == AV_CODEC_ID_MPEG4)
> > > -                stuffing = 4;
> > > -            rcc->buffer_index -= 8 * stuffing;
> > > +            if (stuffing < 32 && s->codec_id == AV_CODEC_ID_MPEG4)
> > > +                stuffing = 32;
> > > +            rcc->buffer_index -= stuffing;
> > >
> > >              if (s->avctx->debug & FF_DEBUG_RC)
> > > -                av_log(s->avctx, AV_LOG_DEBUG, "stuffing %d bytes\n",
> > stuffing);
> > > +                av_log(s->avctx, AV_LOG_DEBUG, "stuffing %d bytes\n",
> > stuffing>>3);
> > >
> > > -            return stuffing;
> > > +            return stuffing>>3;
> > >          }
> >
> > This would break VBV for most codecs
> > buffer_index is a double, the if() condition is triggered if the current
> > frame
> > is too small. All the rounding has to be up as in the ceil() that was
> > there.
> > The stuffing that is added, is minimal, so a decrease would be expected to
> > break cases occasionally
> >
> > Can you explain how this code fails, in what exact case
> > (max/min rate/ buffer_size/fps) ?
> >
> 
> Basically It doesn't work for IMX50, requiring exact frame size of 208541
> bytes at 30000/1001 fps
> and vbv delay of 3003

Is exactly 208541 required or is it "up to 208541"? Tim said in
https://ffmpeg.org/pipermail/ffmpeg-user/2015-June/026872.html
that its "up to"

up to should be easy achievable by adjusting the bitrates and vbv buffer
size to values consistent and below 208541. This would then result in a
mix of 208541 and 208540 bytes. (always 208541 does not result in a integer
bitrate so its tricky to specify and also due to float use could then still
be somewhat unreliable
(208541 bytes and 30000/1001 fps result in a bitrate of 49999840.159840159840159840159840...)

iam not sure if theres a combination of parameters to achieve always 208541
currently.
If not it may be an option to add a parameter to force a specific frame size
directly to the ratecontrol code.

[...]
Michael Niedermayer July 5, 2018, 10:08 p.m.
On Thu, Jul 05, 2018 at 11:49:19PM +0200, Michael Niedermayer wrote:
> On Thu, Jul 05, 2018 at 08:23:24AM -0700, Baptiste Coudurier wrote:
> > Hi Michael
> > 
> > On Thu, Jul 5, 2018 at 3:43 AM, Michael Niedermayer <michael@niedermayer.cc>
> > wrote:
> > 
> > > On Wed, Jul 04, 2018 at 11:35:09AM -0700, Baptiste Coudurier wrote:
> > > > ---
> > > >  libavcodec/ratecontrol.c | 12 ++++++------
> > > >  1 file changed, 6 insertions(+), 6 deletions(-)
> > > >
> > > > diff --git a/libavcodec/ratecontrol.c b/libavcodec/ratecontrol.c
> > > > index 49d169ba25..28bdddbad1 100644
> > > > --- a/libavcodec/ratecontrol.c
> > > > +++ b/libavcodec/ratecontrol.c
> > > > @@ -705,16 +705,16 @@ int ff_vbv_update(MpegEncContext *s, int
> > > frame_size)
> > > >          rcc->buffer_index += av_clip(left, min_rate, max_rate);
> > > >
> > > >          if (rcc->buffer_index > buffer_size) {
> > > > -            int stuffing = ceil((rcc->buffer_index - buffer_size) / 8);
> > > > +            int stuffing = rcc->buffer_index - buffer_size;
> > > >
> > > > -            if (stuffing < 4 && s->codec_id == AV_CODEC_ID_MPEG4)
> > > > -                stuffing = 4;
> > > > -            rcc->buffer_index -= 8 * stuffing;
> > > > +            if (stuffing < 32 && s->codec_id == AV_CODEC_ID_MPEG4)
> > > > +                stuffing = 32;
> > > > +            rcc->buffer_index -= stuffing;
> > > >
> > > >              if (s->avctx->debug & FF_DEBUG_RC)
> > > > -                av_log(s->avctx, AV_LOG_DEBUG, "stuffing %d bytes\n",
> > > stuffing);
> > > > +                av_log(s->avctx, AV_LOG_DEBUG, "stuffing %d bytes\n",
> > > stuffing>>3);
> > > >
> > > > -            return stuffing;
> > > > +            return stuffing>>3;
> > > >          }
> > >
> > > This would break VBV for most codecs
> > > buffer_index is a double, the if() condition is triggered if the current
> > > frame
> > > is too small. All the rounding has to be up as in the ceil() that was
> > > there.
> > > The stuffing that is added, is minimal, so a decrease would be expected to
> > > break cases occasionally
> > >
> > > Can you explain how this code fails, in what exact case
> > > (max/min rate/ buffer_size/fps) ?
> > >
> > 
> > Basically It doesn't work for IMX50, requiring exact frame size of 208541
> > bytes at 30000/1001 fps
> > and vbv delay of 3003
> 
> Is exactly 208541 required or is it "up to 208541"? Tim said in
> https://ffmpeg.org/pipermail/ffmpeg-user/2015-June/026872.html
> that its "up to"
> 
> up to should be easy achievable by adjusting the bitrates and vbv buffer
> size to values consistent and below 208541. This would then result in a
> mix of 208541 and 208540 bytes. (always 208541 does not result in a integer
> bitrate so its tricky to specify and also due to float use could then still
> be somewhat unreliable
> (208541 bytes and 30000/1001 fps result in a bitrate of 49999840.159840159840159840159840...)
> 
> iam not sure if theres a combination of parameters to achieve always 208541
> currently.
> If not it may be an option to add a parameter to force a specific frame size
> directly to the ratecontrol code.

After looking a bit, you can achive 208541 with
-vcodec mpeg2video   -minrate 49999841 -maxrate 49999841 -b:v 49999841 -bufsize 1668329  -rc_init_occupancy 1668329    -f image2 -an  delthisimg%d.mpeg2

not sure if this has issues but it seems to consistently produce the intended
frame size

[...]

Patch hide | download patch | download mbox

diff --git a/libavcodec/ratecontrol.c b/libavcodec/ratecontrol.c
index 49d169ba25..28bdddbad1 100644
--- a/libavcodec/ratecontrol.c
+++ b/libavcodec/ratecontrol.c
@@ -705,16 +705,16 @@  int ff_vbv_update(MpegEncContext *s, int frame_size)
         rcc->buffer_index += av_clip(left, min_rate, max_rate);
 
         if (rcc->buffer_index > buffer_size) {
-            int stuffing = ceil((rcc->buffer_index - buffer_size) / 8);
+            int stuffing = rcc->buffer_index - buffer_size;
 
-            if (stuffing < 4 && s->codec_id == AV_CODEC_ID_MPEG4)
-                stuffing = 4;
-            rcc->buffer_index -= 8 * stuffing;
+            if (stuffing < 32 && s->codec_id == AV_CODEC_ID_MPEG4)
+                stuffing = 32;
+            rcc->buffer_index -= stuffing;
 
             if (s->avctx->debug & FF_DEBUG_RC)
-                av_log(s->avctx, AV_LOG_DEBUG, "stuffing %d bytes\n", stuffing);
+                av_log(s->avctx, AV_LOG_DEBUG, "stuffing %d bytes\n", stuffing>>3);
 
-            return stuffing;
+            return stuffing>>3;
         }
     }
     return 0;