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@ffmpeg.org
> >http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> From 73e5339ec76305d34214b5e84dc5a38673f784b7 Mon Sep 17 00:00:00 2001
> From: Jon Morley <j...@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.
pgpgZoLG__S2C.pgp
Description: PGP signature
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
