Message ID | 1592577334-19049-3-git-send-email-lance.lmwang@gmail.com |
---|---|
State | New |
Headers | show |
Series | [FFmpeg-devel,1/4] avfilter/vf_drawtext: add option to draw timecode in S12M side data | expand |
Context | Check | Description |
---|---|---|
andriy/default | pending | |
andriy/make | success | Make finished |
andriy/make_fate | success | Make fate finished |
lance.lmwang@gmail.com (12020-06-19): > From: Limin Wang <lance.lmwang@gmail.com> > > Signed-off-by: Limin Wang <lance.lmwang@gmail.com> > --- > libavutil/timecode.c | 26 ++++++++++++++++++++++++++ > libavutil/timecode.h | 9 +++++++++ > 2 files changed, 35 insertions(+) > > diff --git a/libavutil/timecode.c b/libavutil/timecode.c > index 60077ba..7a7ed1d 100644 > --- a/libavutil/timecode.c > +++ b/libavutil/timecode.c > @@ -81,6 +81,32 @@ uint32_t av_timecode_get_smpte_from_framenum(const AVTimecode *tc, int framenum) > (hh % 10); // units of hours > } > > +uint32_t av_timecode_get_smpte_by_tc_string(const char *buf) > +{ > + char c; > + int hh, mm, ss, ff, drop; > + > + if (sscanf(buf, "%d:%d:%d%c%d", &hh, &mm, &ss, &c, &ff) != 5) { This accepts garbage at the end. Is it on purpose? > + return AVERROR_INVALIDDATA; You cannot return an error code with that function signature. > + } > + drop = c != ':' ? AV_TIMECODE_FLAG_DROPFRAME : 0; // drop if ';', '.', ... > + > + return 0 << 31 | // color frame flag (0: unsync mode, 1: sync mode) > + drop << 30 | // drop frame flag (0: non drop, 1: drop) > + (ff / 10) << 28 | // tens of frames > + (ff % 10) << 24 | // units of frames > + 0 << 23 | // PC (NTSC) or BGF0 (PAL) > + (ss / 10) << 20 | // tens of seconds > + (ss % 10) << 16 | // units of seconds > + 0 << 15 | // BGF0 (NTSC) or BGF2 (PAL) > + (mm / 10) << 12 | // tens of minutes > + (mm % 10) << 8 | // units of minutes > + 0 << 7 | // BGF2 (NTSC) or PC (PAL) > + 0 << 6 | // BGF1 > + (hh / 10) << 4 | // tens of hours > + (hh % 10); // units of hours > +} > + > char *av_timecode_make_string(const AVTimecode *tc, char *buf, int framenum) > { > int fps = tc->fps; > diff --git a/libavutil/timecode.h b/libavutil/timecode.h > index 37c1361..7bb4b78 100644 > --- a/libavutil/timecode.h > +++ b/libavutil/timecode.h > @@ -71,6 +71,15 @@ int av_timecode_adjust_ntsc_framenum2(int framenum, int fps); > uint32_t av_timecode_get_smpte_from_framenum(const AVTimecode *tc, int framenum); > > /** > + * Convert TC string to SMPTE 12M binary representation. > + * > + * @param buf TC string > + * @return the SMPTE binary representation, or AVERROR otherwise > + * > + */ > +uint32_t av_timecode_get_smpte_by_tc_string(const char *buf); > + > +/** > * Load timecode string in buf. > * > * @param buf destination buffer, must be at least AV_TIMECODE_STR_SIZE long Regards,
On Fri, Jun 19, 2020 at 04:48:46PM +0200, Nicolas George wrote: > lance.lmwang@gmail.com (12020-06-19): > > From: Limin Wang <lance.lmwang@gmail.com> > > > > Signed-off-by: Limin Wang <lance.lmwang@gmail.com> > > --- > > libavutil/timecode.c | 26 ++++++++++++++++++++++++++ > > libavutil/timecode.h | 9 +++++++++ > > 2 files changed, 35 insertions(+) > > > > diff --git a/libavutil/timecode.c b/libavutil/timecode.c > > index 60077ba..7a7ed1d 100644 > > --- a/libavutil/timecode.c > > +++ b/libavutil/timecode.c > > @@ -81,6 +81,32 @@ uint32_t av_timecode_get_smpte_from_framenum(const AVTimecode *tc, int framenum) > > (hh % 10); // units of hours > > } > > > > +uint32_t av_timecode_get_smpte_by_tc_string(const char *buf) > > +{ > > + char c; > > + int hh, mm, ss, ff, drop; > > + > > > + if (sscanf(buf, "%d:%d:%d%c%d", &hh, &mm, &ss, &c, &ff) != 5) { > > This accepts garbage at the end. Is it on purpose? No, in fact I reuse the code from av_timecode_init_from_string(), any suggestion with safety? > > > + return AVERROR_INVALIDDATA; > > You cannot return an error code with that function signature. My fault, I'll add a parameter to get the binary TC. > > > + } > > + drop = c != ':' ? AV_TIMECODE_FLAG_DROPFRAME : 0; // drop if ';', '.', ... > > + > > + return 0 << 31 | // color frame flag (0: unsync mode, 1: sync mode) > > + drop << 30 | // drop frame flag (0: non drop, 1: drop) > > + (ff / 10) << 28 | // tens of frames > > + (ff % 10) << 24 | // units of frames > > + 0 << 23 | // PC (NTSC) or BGF0 (PAL) > > + (ss / 10) << 20 | // tens of seconds > > + (ss % 10) << 16 | // units of seconds > > + 0 << 15 | // BGF0 (NTSC) or BGF2 (PAL) > > + (mm / 10) << 12 | // tens of minutes > > + (mm % 10) << 8 | // units of minutes > > + 0 << 7 | // BGF2 (NTSC) or PC (PAL) > > + 0 << 6 | // BGF1 > > + (hh / 10) << 4 | // tens of hours > > + (hh % 10); // units of hours > > +} > > + > > char *av_timecode_make_string(const AVTimecode *tc, char *buf, int framenum) > > { > > int fps = tc->fps; > > diff --git a/libavutil/timecode.h b/libavutil/timecode.h > > index 37c1361..7bb4b78 100644 > > --- a/libavutil/timecode.h > > +++ b/libavutil/timecode.h > > @@ -71,6 +71,15 @@ int av_timecode_adjust_ntsc_framenum2(int framenum, int fps); > > uint32_t av_timecode_get_smpte_from_framenum(const AVTimecode *tc, int framenum); > > > > /** > > + * Convert TC string to SMPTE 12M binary representation. > > + * > > + * @param buf TC string > > + * @return the SMPTE binary representation, or AVERROR otherwise > > + * > > + */ > > +uint32_t av_timecode_get_smpte_by_tc_string(const char *buf); > > + > > +/** > > * Load timecode string in buf. > > * > > * @param buf destination buffer, must be at least AV_TIMECODE_STR_SIZE long > > Regards, > > -- > Nicolas George
lance.lmwang@gmail.com (12020-06-19):
> No, in fact I reuse the code from av_timecode_init_from_string(), any suggestion with safety?
For starters: DO NOT REUSE CODE.
Use it. Call av_timecode_init_from_string().
To test the garbage at the end of the string: ask scanf() where scanning
stopped, check it is the end of the string.
Regards,
On Fri, Jun 19, 2020 at 05:15:10PM +0200, Nicolas George wrote: > lance.lmwang@gmail.com (12020-06-19): > > No, in fact I reuse the code from av_timecode_init_from_string(), any suggestion with safety? > > For starters: DO NOT REUSE CODE. > > Use it. Call av_timecode_init_from_string(). No, different function, if I can use it, then I'll use it. > > To test the garbage at the end of the string: ask scanf() where scanning > stopped, check it is the end of the string. do you have a broken string for my condition, I'm glad to test and fix it? > > Regards, > > -- > Nicolas George > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
lance.lmwang@gmail.com (12020-06-20): > No, different function, if I can use it, then I'll use it. Duplicating parsing code is a big no. > do you have a broken string for my condition, I'm glad to test and fix it? Any valid string with anything appended at the end. Regards,
On Sat, Jun 20, 2020 at 02:28:39PM +0200, Nicolas George wrote: > lance.lmwang@gmail.com (12020-06-20): > > No, different function, if I can use it, then I'll use it. > > Duplicating parsing code is a big no. av_timecode_init_from_string() will initialize the AVTimecode structure, it'll do more functions, if don't duplicate the code. I can do like below: 1. use av_timecode_init_from_string() to init the tc 2. use av_timecode_get_smpte_from_framenum() to get the smpte binary do you think it's OK? > > > do you have a broken string for my condition, I'm glad to test and fix it? > > Any valid string with anything appended at the end. OK, I'll try to test it. > > Regards, > > -- > Nicolas George > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
lance.lmwang@gmail.com (12020-06-20): > av_timecode_init_from_string() will initialize the AVTimecode structure, > it'll do more functions, if don't duplicate the code. I can do like below: > 1. use av_timecode_init_from_string() to init the tc > 2. use av_timecode_get_smpte_from_framenum() to get the smpte binary > > do you think it's OK? Yes, I think it is the correct way of doing it: using existing functions instead of copying their code. Regards,
On Sat, Jun 20, 2020 at 02:51:59PM +0200, Nicolas George wrote: > lance.lmwang@gmail.com (12020-06-20): > > av_timecode_init_from_string() will initialize the AVTimecode structure, > > it'll do more functions, if don't duplicate the code. I can do like below: > > 1. use av_timecode_init_from_string() to init the tc > > 2. use av_timecode_get_smpte_from_framenum() to get the smpte binary > > > > do you think it's OK? > > Yes, I think it is the correct way of doing it: using existing functions > instead of copying their code. Thanks, have updated with 2 patch only. > > Regards, > > -- > Nicolas George > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
diff --git a/libavutil/timecode.c b/libavutil/timecode.c index 60077ba..7a7ed1d 100644 --- a/libavutil/timecode.c +++ b/libavutil/timecode.c @@ -81,6 +81,32 @@ uint32_t av_timecode_get_smpte_from_framenum(const AVTimecode *tc, int framenum) (hh % 10); // units of hours } +uint32_t av_timecode_get_smpte_by_tc_string(const char *buf) +{ + char c; + int hh, mm, ss, ff, drop; + + if (sscanf(buf, "%d:%d:%d%c%d", &hh, &mm, &ss, &c, &ff) != 5) { + return AVERROR_INVALIDDATA; + } + drop = c != ':' ? AV_TIMECODE_FLAG_DROPFRAME : 0; // drop if ';', '.', ... + + return 0 << 31 | // color frame flag (0: unsync mode, 1: sync mode) + drop << 30 | // drop frame flag (0: non drop, 1: drop) + (ff / 10) << 28 | // tens of frames + (ff % 10) << 24 | // units of frames + 0 << 23 | // PC (NTSC) or BGF0 (PAL) + (ss / 10) << 20 | // tens of seconds + (ss % 10) << 16 | // units of seconds + 0 << 15 | // BGF0 (NTSC) or BGF2 (PAL) + (mm / 10) << 12 | // tens of minutes + (mm % 10) << 8 | // units of minutes + 0 << 7 | // BGF2 (NTSC) or PC (PAL) + 0 << 6 | // BGF1 + (hh / 10) << 4 | // tens of hours + (hh % 10); // units of hours +} + char *av_timecode_make_string(const AVTimecode *tc, char *buf, int framenum) { int fps = tc->fps; diff --git a/libavutil/timecode.h b/libavutil/timecode.h index 37c1361..7bb4b78 100644 --- a/libavutil/timecode.h +++ b/libavutil/timecode.h @@ -71,6 +71,15 @@ int av_timecode_adjust_ntsc_framenum2(int framenum, int fps); uint32_t av_timecode_get_smpte_from_framenum(const AVTimecode *tc, int framenum); /** + * Convert TC string to SMPTE 12M binary representation. + * + * @param buf TC string + * @return the SMPTE binary representation, or AVERROR otherwise + * + */ +uint32_t av_timecode_get_smpte_by_tc_string(const char *buf); + +/** * Load timecode string in buf. * * @param buf destination buffer, must be at least AV_TIMECODE_STR_SIZE long