Re: [libav-devel] [PATCH 3/7] x265: Use the framerate information instead of the timebase

[Luca Barbato]

On 16/02/2018 18:28, Diego Biurrun wrote:

On Fri, Feb 16, 2018 at 06:02:05PM +0100, Luca Barbato wrote:

Unbreaks the rate-control behaviour.


How does this unbreak what?


Locally amended adding:

The API expects an average framerate, the timebase is often
1/1000 while the reported average framerate is 30.

Before this patch the resulting bitrate would be surprising if
the timebase provided isn't the inverse of the average framerate.

lu
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Re: [libav-devel] [PATCH 3/7] x265: Use the framerate information instead of the timebase

[Vittorio Giovara]

On Fri, Feb 16, 2018 at 9:15 PM, Luca Barbato  wrote:

> On 17/02/2018 00:57, wm4 wrote:
>
>> On Fri, 16 Feb 2018 18:02:05 +0100
>> Luca Barbato  wrote:
>>
>> Unbreaks the rate-control behaviour.
>>>
>>
Would be nice that the commit message is expanded, explaining what is
currently broken and how this fixes it.


> ---
>>>   libavcodec/libx265.c | 4 ++--
>>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/libavcodec/libx265.c b/libavcodec/libx265.c
>>> index fd5452193b..73aff2caef 100644
>>> --- a/libavcodec/libx265.c
>>> +++ b/libavcodec/libx265.c
>>> @@ -116,8 +116,8 @@ static av_cold int libx265_encode_init(AVCodecContext
>>> *avctx)
>>>   }
>>> ctx->params->frameNumThreads = avctx->thread_count;
>>> -ctx->params->fpsNum  = avctx->time_base.den;
>>> -ctx->params->fpsDenom= avctx->time_base.num *
>>> avctx->ticks_per_frame;
>>> +ctx->params->fpsNum  = avctx->framerate.num;
>>> +ctx->params->fpsDenom= avctx->framerate.den *
>>> avctx->ticks_per_frame;
>>>   ctx->params->sourceWidth = avctx->width;
>>>   ctx->params->sourceHeight= avctx->height;
>>>   ctx->params->bEnablePsnr = !!(avctx->flags &
>>> AV_CODEC_FLAG_PSNR);
>>>
>>
>> Doesn't this use a potentially broken demuxer reported framerate?
>> (Not uncommon with mkv files at least.)
>>
> --
Vittorio
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Re: [libav-devel] [PATCH 3/7] x265: Use the framerate information instead of the timebase

[Luca Barbato]

On 17/02/2018 00:57, wm4 wrote:

On Fri, 16 Feb 2018 18:02:05 +0100
Luca Barbato  wrote:


Unbreaks the rate-control behaviour.
---
  libavcodec/libx265.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/libavcodec/libx265.c b/libavcodec/libx265.c
index fd5452193b..73aff2caef 100644
--- a/libavcodec/libx265.c
+++ b/libavcodec/libx265.c
@@ -116,8 +116,8 @@ static av_cold int libx265_encode_init(AVCodecContext 
*avctx)
  }
  
  ctx->params->frameNumThreads = avctx->thread_count;

-ctx->params->fpsNum  = avctx->time_base.den;
-ctx->params->fpsDenom= avctx->time_base.num * 
avctx->ticks_per_frame;
+ctx->params->fpsNum  = avctx->framerate.num;
+ctx->params->fpsDenom= avctx->framerate.den * 
avctx->ticks_per_frame;
  ctx->params->sourceWidth = avctx->width;
  ctx->params->sourceHeight= avctx->height;
  ctx->params->bEnablePsnr = !!(avctx->flags & AV_CODEC_FLAG_PSNR);


Doesn't this use a potentially broken demuxer reported framerate?
(Not uncommon with mkv files at least.)


It is better than a definitely broken timebase.

Before you usually feed 1000 fps or worse, now it has at least a chance 
to have a correct framerate by default.


lu

___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Re: [libav-devel] [PATCH 3/7] x265: Use the framerate information instead of the timebase

[wm4]

On Fri, 16 Feb 2018 18:02:05 +0100
Luca Barbato  wrote:

> Unbreaks the rate-control behaviour.
> ---
>  libavcodec/libx265.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/libavcodec/libx265.c b/libavcodec/libx265.c
> index fd5452193b..73aff2caef 100644
> --- a/libavcodec/libx265.c
> +++ b/libavcodec/libx265.c
> @@ -116,8 +116,8 @@ static av_cold int libx265_encode_init(AVCodecContext 
> *avctx)
>  }
>  
>  ctx->params->frameNumThreads = avctx->thread_count;
> -ctx->params->fpsNum  = avctx->time_base.den;
> -ctx->params->fpsDenom= avctx->time_base.num * 
> avctx->ticks_per_frame;
> +ctx->params->fpsNum  = avctx->framerate.num;
> +ctx->params->fpsDenom= avctx->framerate.den * 
> avctx->ticks_per_frame;
>  ctx->params->sourceWidth = avctx->width;
>  ctx->params->sourceHeight= avctx->height;
>  ctx->params->bEnablePsnr = !!(avctx->flags & AV_CODEC_FLAG_PSNR);

Doesn't this use a potentially broken demuxer reported framerate?
(Not uncommon with mkv files at least.)
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Re: [libav-devel] [PATCH 3/7] x265: Use the framerate information instead of the timebase

[Luca Barbato]

On 16/02/2018 18:28, Diego Biurrun wrote:

On Fri, Feb 16, 2018 at 06:02:05PM +0100, Luca Barbato wrote:

Unbreaks the rate-control behaviour.


How does this unbreak what?



If your timebase is 1/1000 and your actual frame rate is 30, it gets 
extra creative in deciding how many bits allocate per frame.


lu
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Re: [libav-devel] [PATCH 3/7] x265: Use the framerate information instead of the timebase

[Diego Biurrun]

On Fri, Feb 16, 2018 at 06:02:05PM +0100, Luca Barbato wrote:
> Unbreaks the rate-control behaviour.

How does this unbreak what?

Diego
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

[libav-devel] [PATCH 3/7] x265: Use the framerate information instead of the timebase

[Luca Barbato]

Unbreaks the rate-control behaviour.
---
 libavcodec/libx265.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/libavcodec/libx265.c b/libavcodec/libx265.c
index fd5452193b..73aff2caef 100644
--- a/libavcodec/libx265.c
+++ b/libavcodec/libx265.c
@@ -116,8 +116,8 @@ static av_cold int libx265_encode_init(AVCodecContext 
*avctx)
 }
 
 ctx->params->frameNumThreads = avctx->thread_count;
-ctx->params->fpsNum  = avctx->time_base.den;
-ctx->params->fpsDenom= avctx->time_base.num * 
avctx->ticks_per_frame;
+ctx->params->fpsNum  = avctx->framerate.num;
+ctx->params->fpsDenom= avctx->framerate.den * 
avctx->ticks_per_frame;
 ctx->params->sourceWidth = avctx->width;
 ctx->params->sourceHeight= avctx->height;
 ctx->params->bEnablePsnr = !!(avctx->flags & AV_CODEC_FLAG_PSNR);
-- 
2.12.2

___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel