[FFmpeg-devel] Question about supported_fps in libavutil/timecode.c::check_fps

[Clément Bœsch]

On Fri, Jan 23, 2015 at 08:48:37AM -0800, jon morley wrote:
> Patch attached for consideration.
> 
> On 1/23/15 8:03 AM, jon morley wrote:
> >Currently check_fps has the following logic:
> >
> >static int check_fps(int fps)
> >{
> >     int i;
> >     static const int supported_fps[] = {24, 25, 30, 48, 50, 60};
> >
> >     for (i = 0; i < FF_ARRAY_ELEMS(supported_fps); i++)
> >         if (fps == supported_fps[i])
> >             return 0;
> >     return -1;
> >}
> >
> >I am starting to see more and more movies with fps rates in excess of
> >this list from modified GoPro files and other raw camera sources.
> >
> >I was originally adding more entries as the sources came rolling in
> >because I could not see any issue in how this was getting called later
> >with that approach.
> >
> >I still don't see the drawback of adding more, but I am tired of adding
> >a new rate every time I encounter one in the wild. I was curious if it
> >wouldn't make more sense to change the logic to the following:
> >
> >static int check_fps(int fps)
> >{
> >     int i;
> >     static const int supported_fps_bases[] = {24, 25, 30};
> >
> >     for (i = 0; i < FF_ARRAY_ELEMS(supported_fps_bases); i++)
> >         if (fps % supported_fps_bases[i] == 0)
> >             return 0;
> >     return -1;
> >}
> >
> >If that makes sense to you, then I will submit a patch. Please let me
> >know if I have overlooked some other usage/meaning of check_fps that I
> >am overlooking.
> >
> >Thanks,
> >Jon
> >_______________________________________________
> >ffmpeg-devel mailing list
> >ffmpeg-devel at ffmpeg.org
> >http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

> From 73e5339ec76305d34214b5e84dc5a38673f784b7 Mon Sep 17 00:00:00 2001
> From: Jon Morley <jon at tweaksoftware.com>
> Date: Fri, 23 Jan 2015 08:43:33 -0800
> Subject: [PATCH] libavutil/timecode.c: Extend check_fps logic to handle high
>  frame rates
> 
> QuickTime sources continue to push higher and higher frame rates. This
> change moves away from explictly testing incoming fps values towards
> ensuring the incoming value is evenly divisable by 24, 25, or 30.
> ---
>  libavutil/timecode.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/libavutil/timecode.c b/libavutil/timecode.c
> index 1dfd040..c10895c 100644
> --- a/libavutil/timecode.c
> +++ b/libavutil/timecode.c
> @@ -141,10 +141,10 @@ char *av_timecode_make_mpeg_tc_string(char *buf, uint32_t tc25bit)
>  static int check_fps(int fps)
>  {
>      int i;
> -    static const int supported_fps[] = {24, 25, 30, 48, 50, 60};
> +    static const int supported_fps_bases[] = {24, 25, 30};
>  
> -    for (i = 0; i < FF_ARRAY_ELEMS(supported_fps); i++)
> -        if (fps == supported_fps[i])
> +    for (i = 0; i < FF_ARRAY_ELEMS(supported_fps_bases); i++)
> +        if (fps % supported_fps_bases[i] == 0)
>              return 0;
>      return -1;

I don't think you want to accept fps ≤ 0

[...]

-- 
Clément B.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 473 bytes
Desc: not available
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20150124/652aea85/attachment.asc>