On Tue, Jun 30, 2020 at 12:28:34PM +0200, Nicolas George wrote: > lance.lmw...@gmail.com (12020-06-30): > > From: Limin Wang <lance.lmw...@gmail.com> > > > > AV_FRAME_DATA_S12M_TIMECODE is public API, so user need to know its format > > by the API header instead of reading the code. > > > > Signed-off-by: Limin Wang <lance.lmw...@gmail.com> > > --- > > libavutil/frame.h | 4 ++-- > > libavutil/timecode.h | 16 +++++++++++++++- > > 2 files changed, 17 insertions(+), 3 deletions(-) > > > > diff --git a/libavutil/frame.h b/libavutil/frame.h > > index 3fb8c56..c694b12 100644 > > --- a/libavutil/frame.h > > +++ b/libavutil/frame.h > > @@ -162,8 +162,8 @@ enum AVFrameSideDataType { > > /** > > * Timecode which conforms to SMPTE ST 12-1. The data is an array of 4 > > uint32_t > > * where the first uint32_t describes how many (1-3) of the other > > timecodes are used. > > - * The timecode format is described in the > > av_timecode_get_smpte_from_framenum() > > - * function in libavutil/timecode.c. > > + * The timecode format is described in the comments of > > av_timecode_get_smpte_from_framenum() > > + * function in libavutil/timecode.h. > > */ > > AV_FRAME_DATA_S12M_TIMECODE, > > > > diff --git a/libavutil/timecode.h b/libavutil/timecode.h > > index ab38e66..b1c1887 100644 > > --- a/libavutil/timecode.h > > +++ b/libavutil/timecode.h > > @@ -61,7 +61,21 @@ int av_timecode_adjust_ntsc_framenum2(int framenum, int > > fps); > > * @param tc timecode data correctly initialized > > * @param framenum frame number > > * @return the SMPTE binary representation > > - * > > > + * the format description as follows, note it's in system byte order: > > Better avoid contractions in documentation. > > The result is uint32_t, not char[4]: there is no byte order.
Sure, will remove the note. > > > + * 31b: color frame flag (0: unsync mode, 1: sync mode) > > + * 30b: drop frame flag (0: non drop, 1: drop) > > + * 28b,29b: tens of frames > > + * 24-27b: units of frames > > + * 23b: PC (NTSC) or BGF0 (PAL) > > + * 20b,22b: tens of seconds > > + * 16-19b: units of seconds > > + * 15b: BGF0 (NTSC) or BGF2 (PAL) > > + * 12b,13b: tens of minutes > > + * 8-11b: units of minutes > > + * 7b: BGF2 (NTSC) or PC (PAL) > > + * 6b: BGF1 > > + * 4b,5b: tens of hours > > + * 0b-3b: units of hours > > It seems more logical in the opposite order. > > I do not think this b suffix is very standard. Better be explicit: > > * bits 24-27: units of frames > > Also, no reason to make a special case when there are exactly two. > > And since several numbers are stored the same way, I would suggest to > regroup: > > * bits 8-13: minutes, in BCD > * ... > * BCD numbers (6 bits): 4 lower bits for units, 2 higher bits for tens. Below is my update by your suggestion, please help to review. MPTE binary representation + * the format description as follows: + * bits 0-5: hours, in BCD + * bits 6: BGF1 + * bits 7: BGF2 (NTSC) or PC (PAL) + * bits 8-13: minutes, in BCD + * bits 15: BGF0 (NTSC) or BGF2 (PAL) + * bits 16-21: seconds, in BCD + * bits 23: PC (NTSC) or BGF0 (PAL) + * bits 24-29: frames, in BCD + * bits 30: drop frame flag (0: non drop, 1: drop) + * bits 31: color frame flag (0: unsync mode, 1: sync mode) + * @note BCD numbers (6 bits): 4 lower bits for units, 2 higher bits for tens. > > * you do NOT have to call av_timecode_adjust_ntsc_framenum2(). > > * @note The frame number is relative to tc->start. > > Regards, > > -- > Nicolas George -- Thanks, Limin Wang _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".