On Sat, Jan 24, 2015 at 12:26:49PM -0800, jon morley wrote: > Hi Clément, > > I am sorry I was rude. That was not my intention. I was attempting > to follow these directions from the ffmpeg.org page: > > "You can use the FFmpeg libraries in your commercial program, but > you are encouraged to publish any patch you make. In this case the > best way to proceed is to send your patches to the ffmpeg-devel > mailing list following the guidelines illustrated in the remainder > of this document." > > I will stick to mailing patches exclusively in the future. > > Patches reflecting your suggestions attached. > > Sincerely, > Jon > > On 1/24/15 8:21 AM, Clément Bœsch 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. > > > >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. > > > >Regards, > > > > > > > >_______________________________________________ > >ffmpeg-devel mailing list > >ffmpeg-devel@ffmpeg.org > >http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > >
> timecode.c | 33 ++++++++++++++++----------------- > 1 file changed, 16 insertions(+), 17 deletions(-) > af5b5d09fd78bb929dc38e2cc11e562a281d5544 > 0001-libavutil-timecode.c-Add-support-for-frame-rates-bey.patch > From 95f1fb3695f086de1baa301015985742d688a159 Mon Sep 17 00:00:00 2001 > From: Jon Morley <j...@tweaksoftware.com> > Date: Sat, 24 Jan 2015 12:18:50 -0800 > Subject: [PATCH] libavutil/timecode.c: Add support for frame rates beyond 60 > fps > > Instead of looking for specifically supported frame rates this > collection of changes checks to see if the given rates are evenly > divisible by supported common factors. > --- > libavutil/timecode.c | 33 ++++++++++++++++----------------- > 1 file changed, 16 insertions(+), 17 deletions(-) > > diff --git a/libavutil/timecode.c b/libavutil/timecode.c > index 1dfd040..c2469c0 100644 > --- a/libavutil/timecode.c > +++ b/libavutil/timecode.c > @@ -33,18 +33,15 @@ > > int av_timecode_adjust_ntsc_framenum2(int framenum, int fps) > { > - /* only works for NTSC 29.97 and 59.94 */ > + int factor = 1; > int drop_frames = 0; > int d, m, frames_per_10mins; > > - if (fps == 30) { > - drop_frames = 2; > - frames_per_10mins = 17982; > - } else if (fps == 60) { > - drop_frames = 4; > - frames_per_10mins = 35964; > - } else > - return framenum; > + if (fps < 30 || fps % 30 != 0) return framenum; > + > + factor = fps / 30; > + drop_frames = factor * 2; > + frames_per_10mins = factor * 17982; > > d = framenum / frames_per_10mins; > m = framenum % frames_per_10mins; > @@ -141,10 +138,12 @@ 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_multiples[] = {24, 25, 30}; > + > + if (fps <= 0) return -1; > > - for (i = 0; i < FF_ARRAY_ELEMS(supported_fps); i++) > - if (fps == supported_fps[i]) > + for (i = 0; i < FF_ARRAY_ELEMS(supported_fps_multiples); i++) > + if (fps % supported_fps_multiples[i] == 0) > return 0; > return -1; > } > @@ -155,8 +154,8 @@ static int check_timecode(void *log_ctx, AVTimecode *tc) > av_log(log_ctx, AV_LOG_ERROR, "Timecode frame rate must be > specified\n"); > return AVERROR(EINVAL); > } > - if ((tc->flags & AV_TIMECODE_FLAG_DROPFRAME) && tc->fps != 30 && tc->fps > != 60) { > - av_log(log_ctx, AV_LOG_ERROR, "Drop frame is only allowed with > 30000/1001 or 60000/1001 FPS\n"); > + if ((tc->flags & AV_TIMECODE_FLAG_DROPFRAME) && tc->fps % 30 != 0) { > + av_log(log_ctx, AV_LOG_ERROR, "Drop frame is only allowed in frame > rates evenly divisible by 30 FPS\n"); > return AVERROR(EINVAL); > } > if (check_fps(tc->fps) < 0) { > @@ -201,9 +200,9 @@ int av_timecode_init_from_string(AVTimecode *tc, > AVRational rate, const char *st > } > > memset(tc, 0, sizeof(*tc)); > - tc->flags = c != ':' ? AV_TIMECODE_FLAG_DROPFRAME : 0; // drop if ';', > '.', ... > - tc->rate = rate; > - tc->fps = fps_from_frame_rate(rate); > + tc->flags = c != ':' ? AV_TIMECODE_FLAG_DROPFRAME : 0; // drop if ';', > '.', ... > + tc->rate = rate; > + tc->fps = fps_from_frame_rate(rate); unrelated cosmetic change also some of this code is used in the mov muxer while mov is limited to 8bits for one of the fields, see mov_write_tmcd_tag() i dont think an unlimited range will work there [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB I have often repented speaking, but never of holding my tongue. -- Xenocrates
signature.asc
Description: Digital signature
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel