[FFmpeg-devel] lavf: consider codec timebase for framerate detection

Submitted by wm4 on May 23, 2017, 11:36 a.m.

Details

Message ID 20170523113651.26514-1-nfxjfg@googlemail.com
State New
Headers show

Commit Message

wm4 May 23, 2017, 11:36 a.m.
Fixes detection of some TV sample as 24.5 FPS. With the patch applied,
it's detected as 25 FPS.
---
 libavformat/utils.c | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

Comments

wm4 May 23, 2017, 11:48 a.m.
On Tue, 23 May 2017 13:36:51 +0200
wm4 <nfxjfg@googlemail.com> wrote:

> Fixes detection of some TV sample as 24.5 FPS. With the patch applied,
> it's detected as 25 FPS.
> ---
>  libavformat/utils.c | 22 ++++++++++++++++++++++
>  1 file changed, 22 insertions(+)
> 
> diff --git a/libavformat/utils.c b/libavformat/utils.c
> index fbd8b58ac2..778a82aeee 100644
> --- a/libavformat/utils.c
> +++ b/libavformat/utils.c
> @@ -3903,6 +3903,7 @@ FF_ENABLE_DEPRECATION_WARNINGS
>                  st->info->codec_info_duration) {
>                  int best_fps      = 0;
>                  double best_error = 0.01;
> +                AVRational codec_frame_rate = av_inv_q(avctx->time_base);
>  
>                  if (st->info->codec_info_duration        >= INT64_MAX / st->time_base.num / 2||
>                      st->info->codec_info_duration_fields >= INT64_MAX / st->time_base.den ||
> @@ -3924,6 +3925,27 @@ FF_ENABLE_DEPRECATION_WARNINGS
>                          best_fps   = std_fps.num;
>                      }
>                  }
> +                if (avctx->ticks_per_frame > 0)
> +                    codec_frame_rate.num /= avctx->ticks_per_frame;
> +                for (j = 0; j < MAX_STD_TIMEBASES; j++) {
> +                    AVRational std_fps = { get_std_framerate(j), 12 * 1001 };
> +                    double error       = fabs(av_q2d(st->avg_frame_rate) /
> +                                              av_q2d(std_fps) - 1);
> +
> +                    if (error < best_error) {
> +                        best_error = error;
> +                        best_fps   = std_fps.num;
> +                    }
> +
> +                    if (codec_frame_rate.num > 0 && codec_frame_rate.den > 0) {
> +                        error       = fabs(av_q2d(codec_frame_rate) /
> +                                           av_q2d(std_fps) - 1);
> +                        if (error < best_error) {
> +                            best_error = error;
> +                            best_fps   = std_fps.num;
> +                        }
> +                    }
> +                }
>                  if (best_fps)
>                      av_reduce(&st->avg_frame_rate.num, &st->avg_frame_rate.den,
>                                best_fps, 12 * 1001, INT_MAX);

Oops, the patch got mangled. The loop above the additions should have
been replaced. This should compute the same result anyway, and I'll
remove it before pushing.
wm4 May 30, 2017, 12:07 p.m.
On Tue, 23 May 2017 13:36:51 +0200
wm4 <nfxjfg@googlemail.com> wrote:

> Fixes detection of some TV sample as 24.5 FPS. With the patch applied,
> it's detected as 25 FPS.
> ---
>  libavformat/utils.c | 22 ++++++++++++++++++++++
>  1 file changed, 22 insertions(+)
> 
> diff --git a/libavformat/utils.c b/libavformat/utils.c
> index fbd8b58ac2..778a82aeee 100644
> --- a/libavformat/utils.c
> +++ b/libavformat/utils.c
> @@ -3903,6 +3903,7 @@ FF_ENABLE_DEPRECATION_WARNINGS
>                  st->info->codec_info_duration) {
>                  int best_fps      = 0;
>                  double best_error = 0.01;
> +                AVRational codec_frame_rate = av_inv_q(avctx->time_base);
>  
>                  if (st->info->codec_info_duration        >= INT64_MAX / st->time_base.num / 2||
>                      st->info->codec_info_duration_fields >= INT64_MAX / st->time_base.den ||
> @@ -3924,6 +3925,27 @@ FF_ENABLE_DEPRECATION_WARNINGS
>                          best_fps   = std_fps.num;
>                      }
>                  }
> +                if (avctx->ticks_per_frame > 0)
> +                    codec_frame_rate.num /= avctx->ticks_per_frame;
> +                for (j = 0; j < MAX_STD_TIMEBASES; j++) {
> +                    AVRational std_fps = { get_std_framerate(j), 12 * 1001 };
> +                    double error       = fabs(av_q2d(st->avg_frame_rate) /
> +                                              av_q2d(std_fps) - 1);
> +
> +                    if (error < best_error) {
> +                        best_error = error;
> +                        best_fps   = std_fps.num;
> +                    }
> +
> +                    if (codec_frame_rate.num > 0 && codec_frame_rate.den > 0) {
> +                        error       = fabs(av_q2d(codec_frame_rate) /
> +                                           av_q2d(std_fps) - 1);
> +                        if (error < best_error) {
> +                            best_error = error;
> +                            best_fps   = std_fps.num;
> +                        }
> +                    }
> +                }
>                  if (best_fps)
>                      av_reduce(&st->avg_frame_rate.num, &st->avg_frame_rate.den,
>                                best_fps, 12 * 1001, INT_MAX);

Nobody wants to review this? It's pretty intrusive. I guess I'll push
tomorrow or so.
James Almer May 30, 2017, 9:46 p.m.
On 5/30/2017 9:07 AM, wm4 wrote:
> On Tue, 23 May 2017 13:36:51 +0200
> wm4 <nfxjfg@googlemail.com> wrote:
> 
>> Fixes detection of some TV sample as 24.5 FPS. With the patch applied,
>> it's detected as 25 FPS.
>> ---
>>  libavformat/utils.c | 22 ++++++++++++++++++++++
>>  1 file changed, 22 insertions(+)
>>
>> diff --git a/libavformat/utils.c b/libavformat/utils.c
>> index fbd8b58ac2..778a82aeee 100644
>> --- a/libavformat/utils.c
>> +++ b/libavformat/utils.c
>> @@ -3903,6 +3903,7 @@ FF_ENABLE_DEPRECATION_WARNINGS
>>                  st->info->codec_info_duration) {
>>                  int best_fps      = 0;
>>                  double best_error = 0.01;
>> +                AVRational codec_frame_rate = av_inv_q(avctx->time_base);
>>  
>>                  if (st->info->codec_info_duration        >= INT64_MAX / st->time_base.num / 2||
>>                      st->info->codec_info_duration_fields >= INT64_MAX / st->time_base.den ||
>> @@ -3924,6 +3925,27 @@ FF_ENABLE_DEPRECATION_WARNINGS
>>                          best_fps   = std_fps.num;
>>                      }
>>                  }
>> +                if (avctx->ticks_per_frame > 0)
>> +                    codec_frame_rate.num /= avctx->ticks_per_frame;
>> +                for (j = 0; j < MAX_STD_TIMEBASES; j++) {
>> +                    AVRational std_fps = { get_std_framerate(j), 12 * 1001 };
>> +                    double error       = fabs(av_q2d(st->avg_frame_rate) /
>> +                                              av_q2d(std_fps) - 1);
>> +
>> +                    if (error < best_error) {
>> +                        best_error = error;
>> +                        best_fps   = std_fps.num;
>> +                    }
>> +
>> +                    if (codec_frame_rate.num > 0 && codec_frame_rate.den > 0) {
>> +                        error       = fabs(av_q2d(codec_frame_rate) /
>> +                                           av_q2d(std_fps) - 1);
>> +                        if (error < best_error) {
>> +                            best_error = error;
>> +                            best_fps   = std_fps.num;
>> +                        }
>> +                    }
>> +                }
>>                  if (best_fps)
>>                      av_reduce(&st->avg_frame_rate.num, &st->avg_frame_rate.den,
>>                                best_fps, 12 * 1001, INT_MAX);
> 
> Nobody wants to review this? It's pretty intrusive. I guess I'll push
> tomorrow or so.

According to the AVCodecContext->time_base doxy

     * - encoding: MUST be set by user.
     * - decoding: the use of this field for decoding is deprecated.
     *             Use framerate instead.

And it's scheduled to be dropped in the next bump (libav already did it).
It's still used for decoding elsewhere in avformat_find_stream_info(),
though, and even in other places, so this is probably going in the wrong
direction and the other cases should be fixed instead.
ffmpeg.c is apparently not using it for decoding, so that's good.

You may be able to use avctx->framerate for this. Check how the
deprecated code in in lavc/utils.c and lavc/decode.c uses it to set
avctx->time_base.
wm4 May 30, 2017, 10:21 p.m.
On Tue, 30 May 2017 18:46:36 -0300
James Almer <jamrial@gmail.com> wrote:

> On 5/30/2017 9:07 AM, wm4 wrote:
> > On Tue, 23 May 2017 13:36:51 +0200
> > wm4 <nfxjfg@googlemail.com> wrote:
> >   
> >> Fixes detection of some TV sample as 24.5 FPS. With the patch applied,
> >> it's detected as 25 FPS.
> >> ---
> >>  libavformat/utils.c | 22 ++++++++++++++++++++++
> >>  1 file changed, 22 insertions(+)
> >>
> >> diff --git a/libavformat/utils.c b/libavformat/utils.c
> >> index fbd8b58ac2..778a82aeee 100644
> >> --- a/libavformat/utils.c
> >> +++ b/libavformat/utils.c
> >> @@ -3903,6 +3903,7 @@ FF_ENABLE_DEPRECATION_WARNINGS
> >>                  st->info->codec_info_duration) {
> >>                  int best_fps      = 0;
> >>                  double best_error = 0.01;
> >> +                AVRational codec_frame_rate = av_inv_q(avctx->time_base);
> >>  
> >>                  if (st->info->codec_info_duration        >= INT64_MAX / st->time_base.num / 2||
> >>                      st->info->codec_info_duration_fields >= INT64_MAX / st->time_base.den ||
> >> @@ -3924,6 +3925,27 @@ FF_ENABLE_DEPRECATION_WARNINGS
> >>                          best_fps   = std_fps.num;
> >>                      }
> >>                  }
> >> +                if (avctx->ticks_per_frame > 0)
> >> +                    codec_frame_rate.num /= avctx->ticks_per_frame;
> >> +                for (j = 0; j < MAX_STD_TIMEBASES; j++) {
> >> +                    AVRational std_fps = { get_std_framerate(j), 12 * 1001 };
> >> +                    double error       = fabs(av_q2d(st->avg_frame_rate) /
> >> +                                              av_q2d(std_fps) - 1);
> >> +
> >> +                    if (error < best_error) {
> >> +                        best_error = error;
> >> +                        best_fps   = std_fps.num;
> >> +                    }
> >> +
> >> +                    if (codec_frame_rate.num > 0 && codec_frame_rate.den > 0) {
> >> +                        error       = fabs(av_q2d(codec_frame_rate) /
> >> +                                           av_q2d(std_fps) - 1);
> >> +                        if (error < best_error) {
> >> +                            best_error = error;
> >> +                            best_fps   = std_fps.num;
> >> +                        }
> >> +                    }
> >> +                }
> >>                  if (best_fps)
> >>                      av_reduce(&st->avg_frame_rate.num, &st->avg_frame_rate.den,
> >>                                best_fps, 12 * 1001, INT_MAX);  
> > 
> > Nobody wants to review this? It's pretty intrusive. I guess I'll push
> > tomorrow or so.  
> 
> According to the AVCodecContext->time_base doxy
> 
>      * - encoding: MUST be set by user.
>      * - decoding: the use of this field for decoding is deprecated.
>      *             Use framerate instead.
> 
> And it's scheduled to be dropped in the next bump (libav already did it).
> It's still used for decoding elsewhere in avformat_find_stream_info(),
> though, and even in other places, so this is probably going in the wrong
> direction and the other cases should be fixed instead.
> ffmpeg.c is apparently not using it for decoding, so that's good.
> 
> You may be able to use avctx->framerate for this. Check how the
> deprecated code in in lavc/utils.c and lavc/decode.c uses it to set
> avctx->time_base.

Right, the h264 decoder even sets the framerate field directly. I'll
see whether using that works.

Btw. what I really want is setting this as the framerate for mpegts,
any tips how to achieve this?

Patch hide | download patch | download mbox

diff --git a/libavformat/utils.c b/libavformat/utils.c
index fbd8b58ac2..778a82aeee 100644
--- a/libavformat/utils.c
+++ b/libavformat/utils.c
@@ -3903,6 +3903,7 @@  FF_ENABLE_DEPRECATION_WARNINGS
                 st->info->codec_info_duration) {
                 int best_fps      = 0;
                 double best_error = 0.01;
+                AVRational codec_frame_rate = av_inv_q(avctx->time_base);
 
                 if (st->info->codec_info_duration        >= INT64_MAX / st->time_base.num / 2||
                     st->info->codec_info_duration_fields >= INT64_MAX / st->time_base.den ||
@@ -3924,6 +3925,27 @@  FF_ENABLE_DEPRECATION_WARNINGS
                         best_fps   = std_fps.num;
                     }
                 }
+                if (avctx->ticks_per_frame > 0)
+                    codec_frame_rate.num /= avctx->ticks_per_frame;
+                for (j = 0; j < MAX_STD_TIMEBASES; j++) {
+                    AVRational std_fps = { get_std_framerate(j), 12 * 1001 };
+                    double error       = fabs(av_q2d(st->avg_frame_rate) /
+                                              av_q2d(std_fps) - 1);
+
+                    if (error < best_error) {
+                        best_error = error;
+                        best_fps   = std_fps.num;
+                    }
+
+                    if (codec_frame_rate.num > 0 && codec_frame_rate.den > 0) {
+                        error       = fabs(av_q2d(codec_frame_rate) /
+                                           av_q2d(std_fps) - 1);
+                        if (error < best_error) {
+                            best_error = error;
+                            best_fps   = std_fps.num;
+                        }
+                    }
+                }
                 if (best_fps)
                     av_reduce(&st->avg_frame_rate.num, &st->avg_frame_rate.den,
                               best_fps, 12 * 1001, INT_MAX);