Re: [FFmpeg-devel] [PATCH v4 1/2] avutil/timecode: add description for SMPTE binary format
lance.lmw...@gmail.com (12020-06-30): > Below is my update by your suggestion, please help to review. Fine by me. But it was only about form. I do not know timecodes at all, so the semantic of the doc is beyond my skills. Regards, -- Nicolas George signature.asc Description: PGP signature ___ 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".
Re: [FFmpeg-devel] [PATCH v4 1/2] avutil/timecode: add description for SMPTE binary format
On Tue, Jun 30, 2020 at 12:28:34PM +0200, Nicolas George wrote:
> lance.lmw...@gmail.com (12020-06-30):
> > From: Limin Wang
> >
> > 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
> > ---
> > 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".
Re: [FFmpeg-devel] [PATCH v4 1/2] avutil/timecode: add description for SMPTE binary format
lance.lmw...@gmail.com (12020-06-30):
> From: Limin Wang
>
> 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
> ---
> 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.
> + * 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.
> * @note Frame number adjustment is automatically done in case of drop
> timecode,
> * you do NOT have to call av_timecode_adjust_ntsc_framenum2().
> * @note The frame number is relative to tc->start.
Regards,
--
Nicolas George
signature.asc
Description: PGP signature
___
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".
[FFmpeg-devel] [PATCH v4 1/2] avutil/timecode: add description for SMPTE binary format
From: Limin Wang
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
---
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:
+ * 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
* @note Frame number adjustment is automatically done in case of drop
timecode,
* you do NOT have to call av_timecode_adjust_ntsc_framenum2().
* @note The frame number is relative to tc->start.
--
1.8.3.1
___
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".
