[FFmpeg-devel] [PATCH 01/11] lavu: add public timecode API.
Stefano Sabatini
stefasab at gmail.com
Thu Jan 19 01:15:26 CET 2012
On date Monday 2012-01-16 17:30:04 +0100, Clément Bœsch encoded:
> From: Clément Bœsch <clement.boesch at smartjog.com>
>
> ---
> doc/APIchanges | 3 +
> libavutil/Makefile | 2 +
> libavutil/avutil.h | 2 +-
> libavutil/timecode.c | 171 ++++++++++++++++++++++++++++++++++++++++++++++++++
> libavutil/timecode.h | 125 ++++++++++++++++++++++++++++++++++++
> 5 files changed, 302 insertions(+), 1 deletions(-)
> create mode 100644 libavutil/timecode.c
> create mode 100644 libavutil/timecode.h
>
> diff --git a/doc/APIchanges b/doc/APIchanges
> index 3b3da66..dde07a4 100644
> --- a/doc/APIchanges
> +++ b/doc/APIchanges
> @@ -13,6 +13,9 @@ libavutil: 2011-04-18
>
> API changes, most recent first:
>
> +2012-01-xx - xxxxxxx - lavu 51.35.100
> + Add public timecode helpers.
> +
> 2012-01-xx - xxxxxxx - lavfi 2.15.0
> Add a new installed header -- libavfilter/version.h -- with version macros.
>
> diff --git a/libavutil/Makefile b/libavutil/Makefile
> index 7ed590d..a20a1ed 100644
> --- a/libavutil/Makefile
> +++ b/libavutil/Makefile
> @@ -36,6 +36,7 @@ HEADERS = adler32.h \
> rational.h \
> samplefmt.h \
> sha.h \
> + timecode.h \
>
> BUILT_HEADERS = avconfig.h
>
> @@ -70,6 +71,7 @@ OBJS = adler32.o \
> rc4.o \
> samplefmt.o \
> sha.o \
> + timecode.o \
> tree.o \
> utils.o \
>
> diff --git a/libavutil/avutil.h b/libavutil/avutil.h
> index d6855a4..99af12d 100644
> --- a/libavutil/avutil.h
> +++ b/libavutil/avutil.h
> @@ -154,7 +154,7 @@
> */
>
> #define LIBAVUTIL_VERSION_MAJOR 51
> -#define LIBAVUTIL_VERSION_MINOR 34
> +#define LIBAVUTIL_VERSION_MINOR 35
> #define LIBAVUTIL_VERSION_MICRO 100
>
> #define LIBAVUTIL_VERSION_INT AV_VERSION_INT(LIBAVUTIL_VERSION_MAJOR, \
> diff --git a/libavutil/timecode.c b/libavutil/timecode.c
> new file mode 100644
> index 0000000..5556db8
> --- /dev/null
> +++ b/libavutil/timecode.c
> @@ -0,0 +1,171 @@
> +/*
> + * Copyright (c) 2006 Smartjog S.A.S, Baptiste Coudurier <baptiste.coudurier at gmail.com>
> + * Copyright (c) 2011-2012 Smartjog S.A.S, Clément Bœsch <clement.boesch at smartjog.com>
> + *
> + * This file is part of FFmpeg.
> + *
> + * FFmpeg is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * FFmpeg is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with FFmpeg; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
> + */
> +
> +/**
> + * @file
> + * Timecode helpers
> + */
Please add a link or a reference to the spec, it's always nice to find
them when you're exploring for the first time a format implementation.
> +
> +#include <stdio.h>
> +#include "timecode.h"
> +#include "log.h"
> +#include "error.h"
nit+++: sort them
> +
> +char *av_mpegtc_to_timecode_string(char *buf, uint32_t tc25bit)
> +{
> + snprintf(buf, AVTIMECODE_STR_LEN, "%02d:%02d:%02d%c%02d",
> + tc25bit>>19 & 0x1f, // hours
> + tc25bit>>13 & 0x3f, // minutes
> + tc25bit>>6 & 0x3f, // seconds
> + tc25bit & 1<<24 ? ';' : ':', // drop
> + tc25bit & 0x3f); // frames
> + return buf;
> +}
> +
> +int av_adjust_ntsc_framenum(int framenum)
> +{
> + /* only works for NTSC 29.97 */
> + int d = framenum / 17982;
> + int m = framenum % 17982;
> + //if (m < 2) m += 2; /* not needed since -2,-1 / 1798 in C returns 0 */
> + return framenum + 18 * d + 2 * ((m - 2) / 1798);
> +}
> +
> +uint32_t av_framenum_to_smpte_timecode(const AVTimecode *tc, int framenum)
> +{
> + unsigned fps = tc->fps;
> + int drop = !!(tc->flags & AVTIMECODE_FLAG_DROPFRAME);
> + int hh, mm, ss, ff;
> +
> + framenum += tc->start;
> + if (drop)
> + framenum = av_adjust_ntsc_framenum(framenum);
> + ff = framenum % fps;
> + ss = framenum / fps % 60;
> + mm = framenum / (fps*60) % 60;
> + hh = framenum / (fps*3600) % 24;
> + return 0 << 31 | // color frame flag
> + drop << 30 | // drop frame flag
> + (ff / 10) << 28 | // tens of frames
> + (ff % 10) << 24 | // units of frames
> + 0 << 23 | // field phase (NTSC), b0 (PAL)
> + (ss / 10) << 20 | // tens of seconds
> + (ss % 10) << 16 | // units of seconds
> + 0 << 15 | // b0 (NTSC), b2 (PAL)
> + (mm / 10) << 12 | // tens of minutes
> + (mm % 10) << 8 | // units of minutes
> + 0 << 7 | // b1
> + 0 << 6 | // b2 (NTSC), field phase (PAL)
> + (hh / 10) << 4 | // tens of hours
> + (hh % 10); // units of hours
> +}
can't comment without the spec
> +char *av_framenum_to_timecode_string(char *buf, const AVTimecode *tc, int framenum)
> +{
> + int fps = tc->fps;
> + int drop = tc->flags & AVTIMECODE_FLAG_DROPFRAME;
> + int hh, mm, ss, ff, neg = 0;
> +
> + framenum += tc->start;
> + if (drop)
> + framenum = av_adjust_ntsc_framenum(framenum);
> + if (framenum < 0) {
> + framenum = -framenum;
> + neg = tc->flags & AVTIMECODE_FLAG_NEGATIVEOK;
> + }
I assume it is acceptable to show the opposite of the number in case
it is negative and the flag is not enabled.
> + ff = framenum % fps;
> + ss = framenum / fps % 60;
> + mm = framenum / (fps*60) % 60;
> + hh = framenum / (fps*3600);
> + if (tc->flags & AVTIMECODE_FLAG_24HOURSMAX)
> + hh = hh % 24;
> + snprintf(buf, AVTIMECODE_STR_LEN, "%s%02d:%02d:%02d%c%02d",
> + neg ? "-" : "",
> + hh, mm, ss, drop ? ';' : ':', ff);
> + return buf;
> +}
> +
> +static int fps_from_frame_rate(AVRational rate)
> +{
> + if (!rate.den || !rate.num)
> + return -1;
> + return (rate.num + rate.den/2) / rate.den;
> +}
> +
Move this down, just before the function where it is used.
> +static int check_timecode(void *avcl, AVTimecode *tc)
avcl -> log_ctx?
> +{
> + if (tc->fps <= 0) {
> + av_log(avcl, AV_LOG_ERROR, "Timecode frame rate must be specified\n");
> + return AVERROR(EINVAL);
> + }
> + if ((tc->flags & AVTIMECODE_FLAG_DROPFRAME) && tc->fps != 30) {
> + av_log(avcl, AV_LOG_ERROR, "Drop frame is only allowed with 30000/1001 FPS\n");
> + return AVERROR(EINVAL);
> + }
> + switch (tc->fps) {
> + case 24:
> + case 25:
> + case 30: return 0;
> +
> + default:
> + av_log(avcl, AV_LOG_ERROR, "Timecode frame rate not supported\n");
> + return AVERROR_PATCHWELCOME;
> + }
> +}
> +
> +int av_init_timecode(void *avcl, AVTimecode *tc, AVRational rate, int flags, int frame_start)
> +{
> + memset(tc, 0, sizeof(*tc));
> + tc->start = frame_start;
> + tc->flags = flags;
> + tc->rate = rate;
> + tc->fps = fps_from_frame_rate(rate);
> + return check_timecode(avcl, tc);
> +}
> +
> +int av_init_timecode_from_string(void *avcl, AVTimecode *tc, AVRational rate, const char *str)
> +{
> + char c;
> + int hh, mm, ss, ff, ret;
> +
> + if (sscanf(str, "%d:%d:%d%c%d", &hh, &mm, &ss, &c, &ff) != 5) {
> + av_log(avcl, AV_LOG_ERROR, "Unable to parse timecode, "
> + "syntax: hh:mm:ss[:;.]ff\n");
> + return AVERROR_INVALIDDATA;
> + }
> +
> + memset(tc, 0, sizeof(*tc));
> + tc->flags = c != ':' ? AVTIMECODE_FLAG_DROPFRAME : 0; // drop if ';', '.', ...
> + tc->rate = rate;
> + tc->fps = fps_from_frame_rate(rate);
> +
> + ret = check_timecode(avcl, tc);
> + if (ret < 0)
> + return ret;
> +
> + tc->start = (hh*3600 + mm*60 + ss) * tc->fps + ff;
> + if (tc->flags & AVTIMECODE_FLAG_DROPFRAME) { /* adjust frame number */
> + int tmins = 60*hh + mm;
> + tc->start -= 2 * (tmins - tmins/10);
> + }
> + return 0;
> +}
> +
> diff --git a/libavutil/timecode.h b/libavutil/timecode.h
> new file mode 100644
> index 0000000..dd8a0dc
> --- /dev/null
> +++ b/libavutil/timecode.h
> @@ -0,0 +1,125 @@
> +/*
> + * Copyright (c) 2006 Smartjog S.A.S, Baptiste Coudurier <baptiste.coudurier at gmail.com>
> + * Copyright (c) 2011-2012 Smartjog S.A.S, Clément Bœsch <clement.boesch at smartjog.com>
> + *
> + * This file is part of FFmpeg.
> + *
> + * FFmpeg is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * FFmpeg is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with FFmpeg; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
> + */
> +
> +/**
> + * @file
> + * Timecode helpers header
> + */
> +
> +#ifndef AVUTIL_TIMECODE_H
> +#define AVUTIL_TIMECODE_H
> +
> +#include <stdint.h>
is this required?
> +#include "rational.h"
> +
> +#define AVTIMECODE_STR_LEN 16
General, AV_TIMECODE_*
> +
> +#define AVTIMECODE_OPTION(ctx, string_field, flags) \
> + "timecode", "set timecode value following hh:mm:ss[:;.]ff format, " \
> + "use ';' or '.' before frame number for drop frame", \
> + offsetof(ctx, string_field), \
> + AV_OPT_TYPE_STRING, {.str=NULL}, CHAR_MIN, CHAR_MAX, flags
> +
> +enum AVTimecodeFlag {
> + AVTIMECODE_FLAG_DROPFRAME = 1<<0,
> + AVTIMECODE_FLAG_24HOURSMAX = 1<<1,
> + AVTIMECODE_FLAG_NEGATIVEOK = 1<<2,
NEGATIVEOK is *very* confusing, please use a less confusing name, in
general "OK" should be banned from API.
> +};
please shortly comment the meaning of each flag
> +
> +typedef struct {
> + int start; ///< timecode frame start (first base frame number)
> + uint32_t flags; ///< flags such as drop frame, +24 hours support, ...
> + AVRational rate; ///< frame rate in rational form
> + unsigned fps; ///< frame per second; must be consistent with the rate field
> +} AVTimecode;
> +
> +/**
> + * Get the timecode string from the 25-bit timecode format (MPEG GOP format).
> + *
> + * @param buf destination buffer, must be at least AVTIMECODE_STR_LEN long.
nit, no need for final point
> + * @param tc25bit the 25-bits timecode
> + * @return the buf parameter
> + */
> +char *av_mpegtc_to_timecode_string(char *buf, uint32_t tc25bit);
For keeping consistent hierarchical naming scheme:
av_timecode_mpegtc_to_string()
alternatively:
av_timecode_get_mpegtc_string()
more consistent with the av_*_get_*_string()
(e.g. av_get_pix_fmt_string(), get_sample_fmt_name(), also consistent
with the convenction of keeping a verb in a function name.
> +
> +/**
> + * Adjust frame number for NTSC drop frame time code.
> + *
> + * @param framenum frame number to adjust
> + * @return adjusted frame number
> + * @warning adjustment is only valid in NTSC 29.97
> + */
> +int av_adjust_ntsc_framenum(int framenum);
_timecode_ ? (ignore me if you think this function is not necessarily
timecode-related).
> +
> +/**
> + * Convert frame number to SMPTE 12M binary representation.
> + *
> + * @param tc timecode data correctly initialized
> + * @param framenum frame number
> + * @return the SMPTE binary representation
> + *
> + * @note frame number adjustment is automatically done in case of drop timecode,
> + * you do NOT have to call av_adjust_ntsc_framenum().
> + * @note the frame number is relative to tc->start.
> + */
> +uint32_t av_framenum_to_smpte_timecode(const AVTimecode *tc, int framenum);
I suggest:
av_timecode_convert_framenum_to_smpte()
av_timecode_get_smpte_from_framenum() => (shorter and more mnemonic)
> +
> +/**
> + * Load timecode string in buf.
> + *
> + * @param buf destination buffer, must be at least AVTIMECODE_STR_LEN long.
nit++, incomplete main sentence so no need of the final point
> + * @param tc timecode data correctly initialized
> + * @param framenum frame number
> + * @return the buf parameter
> + *
> + * @note timecode representation can be a negative timecode and have more than
> + * 24 hours, but will only be honored if the flags are correctly set.
> + * @note the frame number is relative to tc->start.
nit++: complete sentences, Capitalize
> + */
> +char *av_framenum_to_timecode_string(char *buf, const AVTimecode *tc, int framenum);
uhm a bit confusing, as the timecode is obtained not only from the
framenum.
I'd say:
av_timecode_get_string()
> +/**
> + * Init AVTimecode.
> + *
"Init AVTimecode" is a bit weird, it is like to say "Groom AVHorse",
rather than "Groom an allocated multimedia horse".
> + * @param avcl a pointer to an arbitrary struct of which the first field
> + * is a pointer to an AVClass struct (used for av_log)
I suggest log_ctx as it's used in many other places, also avcl is
misleading as I'd tend to read it as "avclass"
BTW usual dilemma for logging, we have the options:
* use no log_ctx, messages are logged to the NULL context (BAD) or not
at all (bad in case an error message may provide useful specific information)
* use log_ctx for logging, but logging can't be inhibited
* use log_ctx + log_level_offset (check av_expr_* API), you have
control but the API gets more complex from the user POV
Anyway I'm OK with log_ctx alone.
> + * @param tc pointer to an allocated AVTimecode
> + * @param rate frame rate in rational form
> + * @param flags miscellaneous flags such as drop frame, +24 hours, ...
> + * (see AVTimecodeFlag)
> + * @param frame_start the first frame number
> + * @return 0 on success, AVERROR otherwise
> + */
> +int av_init_timecode(void *avcl, AVTimecode *tc, AVRational rate, int flags, int frame_start);
av_timecode_init()
> +
> +/**
> + * Parse timecode representation (hh:mm:ss[:;.]ff).
> + *
> + * @param avcl a pointer to an arbitrary struct of which the first field is a
> + * pointer to an AVClass struct (used for av_log).
ditto
> + * @param tc pointer to an allocated AVTimecode
> + * @param rate frame rate in rational form
> + * @param str timecode string which will determine the frame start
> + * @return 0 on success, AVERROR otherwise
> + */
> +int av_init_timecode_from_string(void *avcl, AVTimecode *tc, AVRational rate, const char *str);
av_timecode_init_from_string() should be fine()
> +
> +#endif /* AVUTIL_TIMECODE_H */
> --
> 1.7.7.3
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
--
FFmpeg = Fabulous Furious MultiPurpose Exxagerate Gadget
More information about the ffmpeg-devel
mailing list