| Message ID | 20180704183514.71654-7-baptiste.coudurier@gmail.com |
|---|---|
| State | New |
| Headers | show |
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) ? [...]
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
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. [...]
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 [...]
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;