Hi Clément,
That is a good point! I am attaching an additional patch to remove those
cases even before entering the mod test loop.
Now the logic should look like this:
static int check_fps(int fps)
{
if (fps <= 0) return -1;
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;
}
I am still really curious to know if switching to this division (modulo)
test breaks the "spirit" of check_fps. I could not find anywhere that
benefited from the explicit list the method currently used, but that
doesn't mean it isn't out there.
Thanks,
Jon
On 1/24/15 2:27 AM, Clément Bœsch wrote:
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
[...]
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
From 0a72d78992bbeb6c2536285397149cceb64b05d8 Mon Sep 17 00:00:00 2001
From: Jon Morley <j...@tweaksoftware.com>
Date: Sat, 24 Jan 2015 07:28:40 -0800
Subject: [PATCH 2/2] libavutil/timecode.c: check_fps must reject rates at or
below zero
An earlier change to check_fps's logic which now confirms that the
incoming evaluation fps is evenly divisable by a list of supported
rates leaves open the possibility of accepting zero and negative frame
rates. This change removes that posibility.
---
libavutil/timecode.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/libavutil/timecode.c b/libavutil/timecode.c
index c10895c..446e2d5 100644
--- a/libavutil/timecode.c
+++ b/libavutil/timecode.c
@@ -140,6 +140,8 @@ char *av_timecode_make_mpeg_tc_string(char *buf, uint32_t
tc25bit)
static int check_fps(int fps)
{
+ if (fps <= 0) return -1;
+
int i;
static const int supported_fps_bases[] = {24, 25, 30};
--
1.8.5.2 (Apple Git-48)
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
