On Sat, 24 Jan 2015 17:21:40 +0100
Clément Bœsch <u...@pkh.me> wrote:
> On Sat, Jan 24, 2015 at 07:40:38AM -0800, jon morley wrote:
> > Hi Clément,
> >
>
> Hi,
>
> > 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};
>
> You can't put statements before declarations, some compilers will choke on
> it.
Which ones? We even expect C99 support from the compiler.
> Also, please squash it with the previous patch since it wasn't applied
> yet.
>
> >
> > 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.
>
> I'm more concerned about how the rest of the code will behave. Typically,
> av_timecode_adjust_ntsc_framenum2() could benefit from some improvements
> (checking if fps % 30, and deducing drop_frames and frames_per_10mins
> accordingly) if you allow such thing. Then you might need to adjust
> check_timecode() as well to allow the drop frame for the other % 30.
>
> >
> > Thanks,
> > Jon
> >
>
> [...]
>
> Note: please do not top post on this mailing list, it is considered rude.
AFAIK only 1 person does.
> Regards,
>
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
