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
> >[email protected]
> >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 <[email protected]> > 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 [email protected] http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
