| Message ID | 1592577334-19049-2-git-send-email-lance.lmwang@gmail.com |
|---|---|
| State | Accepted |
| 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: > From: Limin Wang <lance.lmwang@gmail.com> > > Signed-off-by: Limin Wang <lance.lmwang@gmail.com> > --- > doc/APIchanges | 3 +++ > libavcodec/avpacket.c | 1 + > libavcodec/decode.c | 1 + > libavcodec/packet.h | 8 ++++++++ > libavcodec/version.h | 2 +- > libavformat/dump.c | 22 ++++++++++++++++++++++ > 6 files changed, 36 insertions(+), 1 deletion(-) > > diff --git a/doc/APIchanges b/doc/APIchanges > index 1d6cc36..7cad7fa 100644 > --- a/doc/APIchanges > +++ b/doc/APIchanges > @@ -15,6 +15,9 @@ libavutil: 2017-10-21 > > API changes, most recent first: > > +2020-06-xx - xxxxxxxxxx - lavc 58.94.100 - packet.h > + Add AV_PKT_DATA_S12M_TIMECODE. > + > 2020-06-12 - b09fb030c1 - lavu 56.55.100 - pixdesc.h > Add AV_PIX_FMT_X2RGB10. > > diff --git a/libavcodec/avpacket.c b/libavcodec/avpacket.c > index dce26cb..a4316a7 100644 > --- a/libavcodec/avpacket.c > +++ b/libavcodec/avpacket.c > @@ -400,6 +400,7 @@ const char *av_packet_side_data_name(enum AVPacketSideDataType type) > case AV_PKT_DATA_PRFT: return "Producer Reference Time"; > case AV_PKT_DATA_ICC_PROFILE: return "ICC Profile"; > case AV_PKT_DATA_DOVI_CONF: return "DOVI configuration record"; > + case AV_PKT_DATA_S12M_TIMECODE: return "SMPTE 12-1 timecode"; > } > return NULL; > } > diff --git a/libavcodec/decode.c b/libavcodec/decode.c > index a4e50c0..a2bf0cc 100644 > --- a/libavcodec/decode.c > +++ b/libavcodec/decode.c > @@ -1699,6 +1699,7 @@ int ff_decode_frame_props(AVCodecContext *avctx, AVFrame *frame) > { AV_PKT_DATA_CONTENT_LIGHT_LEVEL, AV_FRAME_DATA_CONTENT_LIGHT_LEVEL }, > { AV_PKT_DATA_A53_CC, AV_FRAME_DATA_A53_CC }, > { AV_PKT_DATA_ICC_PROFILE, AV_FRAME_DATA_ICC_PROFILE }, > + { AV_PKT_DATA_S12M_TIMECODE, AV_FRAME_DATA_S12M_TIMECODE }, > }; > > if (pkt) { > diff --git a/libavcodec/packet.h b/libavcodec/packet.h > index 41485f4..72cb4f7 100644 > --- a/libavcodec/packet.h > +++ b/libavcodec/packet.h > @@ -283,6 +283,14 @@ enum AVPacketSideDataType { > AV_PKT_DATA_DOVI_CONF, > > /** > + * 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 format is documented in a C source file, not a public header? Sure about that? > + */ > + AV_PKT_DATA_S12M_TIMECODE, > + > + /** > * The number of side data types. > * This is not part of the public API/ABI in the sense that it may > * change when new side data types are added. > diff --git a/libavcodec/version.h b/libavcodec/version.h > index 0359302..482cc6d 100644 > --- a/libavcodec/version.h > +++ b/libavcodec/version.h > @@ -28,7 +28,7 @@ > #include "libavutil/version.h" > > #define LIBAVCODEC_VERSION_MAJOR 58 > -#define LIBAVCODEC_VERSION_MINOR 93 > +#define LIBAVCODEC_VERSION_MINOR 94 > #define LIBAVCODEC_VERSION_MICRO 100 > > #define LIBAVCODEC_VERSION_INT AV_VERSION_INT(LIBAVCODEC_VERSION_MAJOR, \ > diff --git a/libavformat/dump.c b/libavformat/dump.c > index 117c681..c5f0bd6 100644 > --- a/libavformat/dump.c > +++ b/libavformat/dump.c > @@ -34,6 +34,7 @@ > #include "libavutil/replaygain.h" > #include "libavutil/spherical.h" > #include "libavutil/stereo3d.h" > +#include "libavutil/timecode.h" > > #include "avformat.h" > > @@ -402,6 +403,23 @@ static void dump_dovi_conf(void *ctx, AVPacketSideData* sd) > dovi->dv_bl_signal_compatibility_id); > } > > +static void dump_s12m_timecode(void *ctx, AVPacketSideData* sd) > +{ > + uint32_t *tc = (uint32_t*)sd->data; > + int m = tc[0] & 3; > + > + if (sd->size != sizeof(uint32_t) * 4) { > + av_log(ctx, AV_LOG_ERROR, "invalid data"); > + return; > + } > + > + for (int j = 1; j <= m; j++) { > + char tcbuf[AV_TIMECODE_STR_SIZE]; > + av_timecode_make_smpte_tc_string(tcbuf, tc[j], 0); > + av_log(ctx, AV_LOG_INFO, "timecode - %s%s", tcbuf, j != m ? ", " : ""); > + } > +} > + > static void dump_sidedata(void *ctx, AVStream *st, const char *indent) > { > int i; > @@ -468,6 +486,10 @@ static void dump_sidedata(void *ctx, AVStream *st, const char *indent) > av_log(ctx, AV_LOG_INFO, "DOVI configuration record: "); > dump_dovi_conf(ctx, &sd); > break; > + case AV_PKT_DATA_S12M_TIMECODE: > + av_log(ctx, AV_LOG_INFO, "S12M timecode: "); > + dump_s12m_timecode(ctx, &sd); > + break; > default: > av_log(ctx, AV_LOG_INFO, > "unknown side data type %d (%d bytes)", sd.type, sd.size); >
On Mon, Jun 29, 2020 at 04:53:21AM +0200, Andreas Rheinhardt wrote: > lance.lmwang@gmail.com: > > From: Limin Wang <lance.lmwang@gmail.com> > > > > Signed-off-by: Limin Wang <lance.lmwang@gmail.com> > > --- > > doc/APIchanges | 3 +++ > > libavcodec/avpacket.c | 1 + > > libavcodec/decode.c | 1 + > > libavcodec/packet.h | 8 ++++++++ > > libavcodec/version.h | 2 +- > > libavformat/dump.c | 22 ++++++++++++++++++++++ > > 6 files changed, 36 insertions(+), 1 deletion(-) > > > > diff --git a/doc/APIchanges b/doc/APIchanges > > index 1d6cc36..7cad7fa 100644 > > --- a/doc/APIchanges > > +++ b/doc/APIchanges > > @@ -15,6 +15,9 @@ libavutil: 2017-10-21 > > > > API changes, most recent first: > > > > +2020-06-xx - xxxxxxxxxx - lavc 58.94.100 - packet.h > > + Add AV_PKT_DATA_S12M_TIMECODE. > > + > > 2020-06-12 - b09fb030c1 - lavu 56.55.100 - pixdesc.h > > Add AV_PIX_FMT_X2RGB10. > > > > diff --git a/libavcodec/avpacket.c b/libavcodec/avpacket.c > > index dce26cb..a4316a7 100644 > > --- a/libavcodec/avpacket.c > > +++ b/libavcodec/avpacket.c > > @@ -400,6 +400,7 @@ const char *av_packet_side_data_name(enum AVPacketSideDataType type) > > case AV_PKT_DATA_PRFT: return "Producer Reference Time"; > > case AV_PKT_DATA_ICC_PROFILE: return "ICC Profile"; > > case AV_PKT_DATA_DOVI_CONF: return "DOVI configuration record"; > > + case AV_PKT_DATA_S12M_TIMECODE: return "SMPTE 12-1 timecode"; > > } > > return NULL; > > } > > diff --git a/libavcodec/decode.c b/libavcodec/decode.c > > index a4e50c0..a2bf0cc 100644 > > --- a/libavcodec/decode.c > > +++ b/libavcodec/decode.c > > @@ -1699,6 +1699,7 @@ int ff_decode_frame_props(AVCodecContext *avctx, AVFrame *frame) > > { AV_PKT_DATA_CONTENT_LIGHT_LEVEL, AV_FRAME_DATA_CONTENT_LIGHT_LEVEL }, > > { AV_PKT_DATA_A53_CC, AV_FRAME_DATA_A53_CC }, > > { AV_PKT_DATA_ICC_PROFILE, AV_FRAME_DATA_ICC_PROFILE }, > > + { AV_PKT_DATA_S12M_TIMECODE, AV_FRAME_DATA_S12M_TIMECODE }, > > }; > > > > if (pkt) { > > diff --git a/libavcodec/packet.h b/libavcodec/packet.h > > index 41485f4..72cb4f7 100644 > > --- a/libavcodec/packet.h > > +++ b/libavcodec/packet.h > > @@ -283,6 +283,14 @@ enum AVPacketSideDataType { > > AV_PKT_DATA_DOVI_CONF, > > > > /** > > + * 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 format is documented in a C source file, not a public header? Sure > about that? For AV_PKT_DATA_S12M_TIMECODE use the same format with AV_FRAME_DATA_S12M_TIMECODE, so I copy the comments for AV_FRAME_DATA_S12M_TIMECODE, it's not my comments for it. I think the format is described in "SMPTE ST 12-2:2014" standard document, but it's not free for download. What's your suggestion to change? > > > + */ > > + AV_PKT_DATA_S12M_TIMECODE, > > + > > + /** > > * The number of side data types. > > * This is not part of the public API/ABI in the sense that it may > > * change when new side data types are added. > > diff --git a/libavcodec/version.h b/libavcodec/version.h > > index 0359302..482cc6d 100644 > > --- a/libavcodec/version.h > > +++ b/libavcodec/version.h > > @@ -28,7 +28,7 @@ > > #include "libavutil/version.h" > > > > #define LIBAVCODEC_VERSION_MAJOR 58 > > -#define LIBAVCODEC_VERSION_MINOR 93 > > +#define LIBAVCODEC_VERSION_MINOR 94 > > #define LIBAVCODEC_VERSION_MICRO 100 > > > > #define LIBAVCODEC_VERSION_INT AV_VERSION_INT(LIBAVCODEC_VERSION_MAJOR, \ > > diff --git a/libavformat/dump.c b/libavformat/dump.c > > index 117c681..c5f0bd6 100644 > > --- a/libavformat/dump.c > > +++ b/libavformat/dump.c > > @@ -34,6 +34,7 @@ > > #include "libavutil/replaygain.h" > > #include "libavutil/spherical.h" > > #include "libavutil/stereo3d.h" > > +#include "libavutil/timecode.h" > > > > #include "avformat.h" > > > > @@ -402,6 +403,23 @@ static void dump_dovi_conf(void *ctx, AVPacketSideData* sd) > > dovi->dv_bl_signal_compatibility_id); > > } > > > > +static void dump_s12m_timecode(void *ctx, AVPacketSideData* sd) > > +{ > > + uint32_t *tc = (uint32_t*)sd->data; > > + int m = tc[0] & 3; > > + > > + if (sd->size != sizeof(uint32_t) * 4) { > > + av_log(ctx, AV_LOG_ERROR, "invalid data"); > > + return; > > + } > > + > > + for (int j = 1; j <= m; j++) { > > + char tcbuf[AV_TIMECODE_STR_SIZE]; > > + av_timecode_make_smpte_tc_string(tcbuf, tc[j], 0); > > + av_log(ctx, AV_LOG_INFO, "timecode - %s%s", tcbuf, j != m ? ", " : ""); > > + } > > +} > > + > > static void dump_sidedata(void *ctx, AVStream *st, const char *indent) > > { > > int i; > > @@ -468,6 +486,10 @@ static void dump_sidedata(void *ctx, AVStream *st, const char *indent) > > av_log(ctx, AV_LOG_INFO, "DOVI configuration record: "); > > dump_dovi_conf(ctx, &sd); > > break; > > + case AV_PKT_DATA_S12M_TIMECODE: > > + av_log(ctx, AV_LOG_INFO, "S12M timecode: "); > > + dump_s12m_timecode(ctx, &sd); > > + break; > > default: > > av_log(ctx, AV_LOG_INFO, > > "unknown side data type %d (%d bytes)", sd.type, sd.size); > > > > _______________________________________________ > 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: > On Mon, Jun 29, 2020 at 04:53:21AM +0200, Andreas Rheinhardt wrote: >> lance.lmwang@gmail.com: >>> From: Limin Wang <lance.lmwang@gmail.com> >>> >>> Signed-off-by: Limin Wang <lance.lmwang@gmail.com> >>> --- >>> doc/APIchanges | 3 +++ >>> libavcodec/avpacket.c | 1 + >>> libavcodec/decode.c | 1 + >>> libavcodec/packet.h | 8 ++++++++ >>> libavcodec/version.h | 2 +- >>> libavformat/dump.c | 22 ++++++++++++++++++++++ >>> 6 files changed, 36 insertions(+), 1 deletion(-) >>> >>> diff --git a/doc/APIchanges b/doc/APIchanges >>> index 1d6cc36..7cad7fa 100644 >>> --- a/doc/APIchanges >>> +++ b/doc/APIchanges >>> @@ -15,6 +15,9 @@ libavutil: 2017-10-21 >>> >>> API changes, most recent first: >>> >>> +2020-06-xx - xxxxxxxxxx - lavc 58.94.100 - packet.h >>> + Add AV_PKT_DATA_S12M_TIMECODE. >>> + >>> 2020-06-12 - b09fb030c1 - lavu 56.55.100 - pixdesc.h >>> Add AV_PIX_FMT_X2RGB10. >>> >>> diff --git a/libavcodec/avpacket.c b/libavcodec/avpacket.c >>> index dce26cb..a4316a7 100644 >>> --- a/libavcodec/avpacket.c >>> +++ b/libavcodec/avpacket.c >>> @@ -400,6 +400,7 @@ const char *av_packet_side_data_name(enum AVPacketSideDataType type) >>> case AV_PKT_DATA_PRFT: return "Producer Reference Time"; >>> case AV_PKT_DATA_ICC_PROFILE: return "ICC Profile"; >>> case AV_PKT_DATA_DOVI_CONF: return "DOVI configuration record"; >>> + case AV_PKT_DATA_S12M_TIMECODE: return "SMPTE 12-1 timecode"; >>> } >>> return NULL; >>> } >>> diff --git a/libavcodec/decode.c b/libavcodec/decode.c >>> index a4e50c0..a2bf0cc 100644 >>> --- a/libavcodec/decode.c >>> +++ b/libavcodec/decode.c >>> @@ -1699,6 +1699,7 @@ int ff_decode_frame_props(AVCodecContext *avctx, AVFrame *frame) >>> { AV_PKT_DATA_CONTENT_LIGHT_LEVEL, AV_FRAME_DATA_CONTENT_LIGHT_LEVEL }, >>> { AV_PKT_DATA_A53_CC, AV_FRAME_DATA_A53_CC }, >>> { AV_PKT_DATA_ICC_PROFILE, AV_FRAME_DATA_ICC_PROFILE }, >>> + { AV_PKT_DATA_S12M_TIMECODE, AV_FRAME_DATA_S12M_TIMECODE }, >>> }; >>> >>> if (pkt) { >>> diff --git a/libavcodec/packet.h b/libavcodec/packet.h >>> index 41485f4..72cb4f7 100644 >>> --- a/libavcodec/packet.h >>> +++ b/libavcodec/packet.h >>> @@ -283,6 +283,14 @@ enum AVPacketSideDataType { >>> AV_PKT_DATA_DOVI_CONF, >>> >>> /** >>> + * 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 format is documented in a C source file, not a public header? Sure >> about that? > > For AV_PKT_DATA_S12M_TIMECODE use the same format with AV_FRAME_DATA_S12M_TIMECODE, so I copy the > comments for AV_FRAME_DATA_S12M_TIMECODE, it's not my comments for it. I think the format is described > in "SMPTE ST 12-2:2014" standard document, but it's not free for download. > It's SMPTE ST 12-1:2014 and this standard is currently (until 21 July) available for free [1] due to Corona. > What's your suggestion to change? > The best is probably to incorporate the documentation of the format into the documentation of av_timecode_get_smpte_from_framenum. >> >>> + */ >>> + AV_PKT_DATA_S12M_TIMECODE, >>> + >>> + /** >>> * The number of side data types. >>> * This is not part of the public API/ABI in the sense that it may >>> * change when new side data types are added. >>> diff --git a/libavcodec/version.h b/libavcodec/version.h >>> index 0359302..482cc6d 100644 >>> --- a/libavcodec/version.h >>> +++ b/libavcodec/version.h >>> @@ -28,7 +28,7 @@ >>> #include "libavutil/version.h" >>> >>> #define LIBAVCODEC_VERSION_MAJOR 58 >>> -#define LIBAVCODEC_VERSION_MINOR 93 >>> +#define LIBAVCODEC_VERSION_MINOR 94 >>> #define LIBAVCODEC_VERSION_MICRO 100 >>> >>> #define LIBAVCODEC_VERSION_INT AV_VERSION_INT(LIBAVCODEC_VERSION_MAJOR, \ >>> diff --git a/libavformat/dump.c b/libavformat/dump.c >>> index 117c681..c5f0bd6 100644 >>> --- a/libavformat/dump.c >>> +++ b/libavformat/dump.c >>> @@ -34,6 +34,7 @@ >>> #include "libavutil/replaygain.h" >>> #include "libavutil/spherical.h" >>> #include "libavutil/stereo3d.h" >>> +#include "libavutil/timecode.h" >>> >>> #include "avformat.h" >>> >>> @@ -402,6 +403,23 @@ static void dump_dovi_conf(void *ctx, AVPacketSideData* sd) >>> dovi->dv_bl_signal_compatibility_id); >>> } >>> >>> +static void dump_s12m_timecode(void *ctx, AVPacketSideData* sd) >>> +{ >>> + uint32_t *tc = (uint32_t*)sd->data; >>> + int m = tc[0] & 3; >>> + >>> + if (sd->size != sizeof(uint32_t) * 4) { You already dereferenced tc before this check which might crash if the size is actually < sizeof(uint32_t). And why do you only look at the lowest two bits of m? Is this designed with future extensions of the timecode in mind? But even then one can't simply use the upper 30 bits to store something else, because that has not been documented. (Moreover, if one allowed extensions, one should allow sd->size to be >= sizeof(uint32_t) * 4.) >>> + av_log(ctx, AV_LOG_ERROR, "invalid data"); >>> + return; >>> + } >>> + >>> + for (int j = 1; j <= m; j++) { >>> + char tcbuf[AV_TIMECODE_STR_SIZE]; >>> + av_timecode_make_smpte_tc_string(tcbuf, tc[j], 0); >>> + av_log(ctx, AV_LOG_INFO, "timecode - %s%s", tcbuf, j != m ? ", " : ""); >>> + } >>> +} >>> + - Andreas [1]: https://www.smpte.org/free-standards-and-publications
On Mon, Jun 29, 2020 at 02:06:49PM +0200, Andreas Rheinhardt wrote: > lance.lmwang@gmail.com: > > On Mon, Jun 29, 2020 at 04:53:21AM +0200, Andreas Rheinhardt wrote: > >> lance.lmwang@gmail.com: > >>> From: Limin Wang <lance.lmwang@gmail.com> > >>> > >>> Signed-off-by: Limin Wang <lance.lmwang@gmail.com> > >>> --- > >>> doc/APIchanges | 3 +++ > >>> libavcodec/avpacket.c | 1 + > >>> libavcodec/decode.c | 1 + > >>> libavcodec/packet.h | 8 ++++++++ > >>> libavcodec/version.h | 2 +- > >>> libavformat/dump.c | 22 ++++++++++++++++++++++ > >>> 6 files changed, 36 insertions(+), 1 deletion(-) > >>> > >>> diff --git a/doc/APIchanges b/doc/APIchanges > >>> index 1d6cc36..7cad7fa 100644 > >>> --- a/doc/APIchanges > >>> +++ b/doc/APIchanges > >>> @@ -15,6 +15,9 @@ libavutil: 2017-10-21 > >>> > >>> API changes, most recent first: > >>> > >>> +2020-06-xx - xxxxxxxxxx - lavc 58.94.100 - packet.h > >>> + Add AV_PKT_DATA_S12M_TIMECODE. > >>> + > >>> 2020-06-12 - b09fb030c1 - lavu 56.55.100 - pixdesc.h > >>> Add AV_PIX_FMT_X2RGB10. > >>> > >>> diff --git a/libavcodec/avpacket.c b/libavcodec/avpacket.c > >>> index dce26cb..a4316a7 100644 > >>> --- a/libavcodec/avpacket.c > >>> +++ b/libavcodec/avpacket.c > >>> @@ -400,6 +400,7 @@ const char *av_packet_side_data_name(enum AVPacketSideDataType type) > >>> case AV_PKT_DATA_PRFT: return "Producer Reference Time"; > >>> case AV_PKT_DATA_ICC_PROFILE: return "ICC Profile"; > >>> case AV_PKT_DATA_DOVI_CONF: return "DOVI configuration record"; > >>> + case AV_PKT_DATA_S12M_TIMECODE: return "SMPTE 12-1 timecode"; > >>> } > >>> return NULL; > >>> } > >>> diff --git a/libavcodec/decode.c b/libavcodec/decode.c > >>> index a4e50c0..a2bf0cc 100644 > >>> --- a/libavcodec/decode.c > >>> +++ b/libavcodec/decode.c > >>> @@ -1699,6 +1699,7 @@ int ff_decode_frame_props(AVCodecContext *avctx, AVFrame *frame) > >>> { AV_PKT_DATA_CONTENT_LIGHT_LEVEL, AV_FRAME_DATA_CONTENT_LIGHT_LEVEL }, > >>> { AV_PKT_DATA_A53_CC, AV_FRAME_DATA_A53_CC }, > >>> { AV_PKT_DATA_ICC_PROFILE, AV_FRAME_DATA_ICC_PROFILE }, > >>> + { AV_PKT_DATA_S12M_TIMECODE, AV_FRAME_DATA_S12M_TIMECODE }, > >>> }; > >>> > >>> if (pkt) { > >>> diff --git a/libavcodec/packet.h b/libavcodec/packet.h > >>> index 41485f4..72cb4f7 100644 > >>> --- a/libavcodec/packet.h > >>> +++ b/libavcodec/packet.h > >>> @@ -283,6 +283,14 @@ enum AVPacketSideDataType { > >>> AV_PKT_DATA_DOVI_CONF, > >>> > >>> /** > >>> + * 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 format is documented in a C source file, not a public header? Sure > >> about that? > > > > For AV_PKT_DATA_S12M_TIMECODE use the same format with AV_FRAME_DATA_S12M_TIMECODE, so I copy the > > comments for AV_FRAME_DATA_S12M_TIMECODE, it's not my comments for it. I think the format is described > > in "SMPTE ST 12-2:2014" standard document, but it's not free for download. > > > > It's SMPTE ST 12-1:2014 and this standard is currently (until 21 July) > available for free [1] due to Corona. oh, it's good to hear it's free due to Corona. I haven't notice it's online. > > > What's your suggestion to change? > > > > The best is probably to incorporate the documentation of the format into > the documentation of av_timecode_get_smpte_from_framenum. OK, I'll remove the comments for the timecode format. > > >> > >>> + */ > >>> + AV_PKT_DATA_S12M_TIMECODE, > >>> + > >>> + /** > >>> * The number of side data types. > >>> * This is not part of the public API/ABI in the sense that it may > >>> * change when new side data types are added. > >>> diff --git a/libavcodec/version.h b/libavcodec/version.h > >>> index 0359302..482cc6d 100644 > >>> --- a/libavcodec/version.h > >>> +++ b/libavcodec/version.h > >>> @@ -28,7 +28,7 @@ > >>> #include "libavutil/version.h" > >>> > >>> #define LIBAVCODEC_VERSION_MAJOR 58 > >>> -#define LIBAVCODEC_VERSION_MINOR 93 > >>> +#define LIBAVCODEC_VERSION_MINOR 94 > >>> #define LIBAVCODEC_VERSION_MICRO 100 > >>> > >>> #define LIBAVCODEC_VERSION_INT AV_VERSION_INT(LIBAVCODEC_VERSION_MAJOR, \ > >>> diff --git a/libavformat/dump.c b/libavformat/dump.c > >>> index 117c681..c5f0bd6 100644 > >>> --- a/libavformat/dump.c > >>> +++ b/libavformat/dump.c > >>> @@ -34,6 +34,7 @@ > >>> #include "libavutil/replaygain.h" > >>> #include "libavutil/spherical.h" > >>> #include "libavutil/stereo3d.h" > >>> +#include "libavutil/timecode.h" > >>> > >>> #include "avformat.h" > >>> > >>> @@ -402,6 +403,23 @@ static void dump_dovi_conf(void *ctx, AVPacketSideData* sd) > >>> dovi->dv_bl_signal_compatibility_id); > >>> } > >>> > >>> +static void dump_s12m_timecode(void *ctx, AVPacketSideData* sd) > >>> +{ > >>> + uint32_t *tc = (uint32_t*)sd->data; > >>> + int m = tc[0] & 3; > >>> + > >>> + if (sd->size != sizeof(uint32_t) * 4) { > > You already dereferenced tc before this check which might crash if the > size is actually < sizeof(uint32_t). And why do you only look at the > lowest two bits of m? Is this designed with future extensions of the > timecode in mind? But even then one can't simply use the upper 30 bits > to store something else, because that has not been documented. > (Moreover, if one allowed extensions, one should allow sd->size to be >= > sizeof(uint32_t) * 4.) It's from the comments for the side data, it used to avoid index overflow, for the array is 4. I'll fix to get the m after the checking. comments: where the first uint32_t describes how many (1-3) of the other timecodes are used I think it's for the timecode SEI define 2bits for the count of used timecomes, it's not related to SMPTE ST 12-1. vf_showinfo.c have dump AV_FRAME_DATA_S12M_TIMECODE, I'm following it as format is same, but change m to & 3 for I think it's more simple. > > >>> + av_log(ctx, AV_LOG_ERROR, "invalid data"); > >>> + return; > >>> + } > >>> + > >>> + for (int j = 1; j <= m; j++) { > >>> + char tcbuf[AV_TIMECODE_STR_SIZE]; > >>> + av_timecode_make_smpte_tc_string(tcbuf, tc[j], 0); > >>> + av_log(ctx, AV_LOG_INFO, "timecode - %s%s", tcbuf, j != m ? ", " : ""); > >>> + } > >>> +} > >>> + > > - Andreas > > [1]: https://www.smpte.org/free-standards-and-publications > _______________________________________________ > 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: > On Mon, Jun 29, 2020 at 02:06:49PM +0200, Andreas Rheinhardt wrote: >> lance.lmwang@gmail.com: >>> On Mon, Jun 29, 2020 at 04:53:21AM +0200, Andreas Rheinhardt wrote: >>>> lance.lmwang@gmail.com: >>>>> From: Limin Wang <lance.lmwang@gmail.com> >>>>> >>>>> Signed-off-by: Limin Wang <lance.lmwang@gmail.com> >>>>> --- >>>>> doc/APIchanges | 3 +++ >>>>> libavcodec/avpacket.c | 1 + >>>>> libavcodec/decode.c | 1 + >>>>> libavcodec/packet.h | 8 ++++++++ >>>>> libavcodec/version.h | 2 +- >>>>> libavformat/dump.c | 22 ++++++++++++++++++++++ >>>>> 6 files changed, 36 insertions(+), 1 deletion(-) >>>>> >>>>> diff --git a/doc/APIchanges b/doc/APIchanges >>>>> index 1d6cc36..7cad7fa 100644 >>>>> --- a/doc/APIchanges >>>>> +++ b/doc/APIchanges >>>>> @@ -15,6 +15,9 @@ libavutil: 2017-10-21 >>>>> >>>>> API changes, most recent first: >>>>> >>>>> +2020-06-xx - xxxxxxxxxx - lavc 58.94.100 - packet.h >>>>> + Add AV_PKT_DATA_S12M_TIMECODE. >>>>> + >>>>> 2020-06-12 - b09fb030c1 - lavu 56.55.100 - pixdesc.h >>>>> Add AV_PIX_FMT_X2RGB10. >>>>> >>>>> diff --git a/libavcodec/avpacket.c b/libavcodec/avpacket.c >>>>> index dce26cb..a4316a7 100644 >>>>> --- a/libavcodec/avpacket.c >>>>> +++ b/libavcodec/avpacket.c >>>>> @@ -400,6 +400,7 @@ const char *av_packet_side_data_name(enum AVPacketSideDataType type) >>>>> case AV_PKT_DATA_PRFT: return "Producer Reference Time"; >>>>> case AV_PKT_DATA_ICC_PROFILE: return "ICC Profile"; >>>>> case AV_PKT_DATA_DOVI_CONF: return "DOVI configuration record"; >>>>> + case AV_PKT_DATA_S12M_TIMECODE: return "SMPTE 12-1 timecode"; >>>>> } >>>>> return NULL; >>>>> } >>>>> diff --git a/libavcodec/decode.c b/libavcodec/decode.c >>>>> index a4e50c0..a2bf0cc 100644 >>>>> --- a/libavcodec/decode.c >>>>> +++ b/libavcodec/decode.c >>>>> @@ -1699,6 +1699,7 @@ int ff_decode_frame_props(AVCodecContext *avctx, AVFrame *frame) >>>>> { AV_PKT_DATA_CONTENT_LIGHT_LEVEL, AV_FRAME_DATA_CONTENT_LIGHT_LEVEL }, >>>>> { AV_PKT_DATA_A53_CC, AV_FRAME_DATA_A53_CC }, >>>>> { AV_PKT_DATA_ICC_PROFILE, AV_FRAME_DATA_ICC_PROFILE }, >>>>> + { AV_PKT_DATA_S12M_TIMECODE, AV_FRAME_DATA_S12M_TIMECODE }, >>>>> }; >>>>> >>>>> if (pkt) { >>>>> diff --git a/libavcodec/packet.h b/libavcodec/packet.h >>>>> index 41485f4..72cb4f7 100644 >>>>> --- a/libavcodec/packet.h >>>>> +++ b/libavcodec/packet.h >>>>> @@ -283,6 +283,14 @@ enum AVPacketSideDataType { >>>>> AV_PKT_DATA_DOVI_CONF, >>>>> >>>>> /** >>>>> + * 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 format is documented in a C source file, not a public header? Sure >>>> about that? >>> >>> For AV_PKT_DATA_S12M_TIMECODE use the same format with AV_FRAME_DATA_S12M_TIMECODE, so I copy the >>> comments for AV_FRAME_DATA_S12M_TIMECODE, it's not my comments for it. I think the format is described >>> in "SMPTE ST 12-2:2014" standard document, but it's not free for download. >>> >> >> It's SMPTE ST 12-1:2014 and this standard is currently (until 21 July) >> available for free [1] due to Corona. > > oh, it's good to hear it's free due to Corona. I haven't notice it's online. > >> >>> What's your suggestion to change? >>> >> >> The best is probably to incorporate the documentation of the format into >> the documentation of av_timecode_get_smpte_from_framenum. > OK, I'll remove the comments for the timecode format. >> >>>> >>>>> + */ >>>>> + AV_PKT_DATA_S12M_TIMECODE, >>>>> + >>>>> + /** >>>>> * The number of side data types. >>>>> * This is not part of the public API/ABI in the sense that it may >>>>> * change when new side data types are added. >>>>> diff --git a/libavcodec/version.h b/libavcodec/version.h >>>>> index 0359302..482cc6d 100644 >>>>> --- a/libavcodec/version.h >>>>> +++ b/libavcodec/version.h >>>>> @@ -28,7 +28,7 @@ >>>>> #include "libavutil/version.h" >>>>> >>>>> #define LIBAVCODEC_VERSION_MAJOR 58 >>>>> -#define LIBAVCODEC_VERSION_MINOR 93 >>>>> +#define LIBAVCODEC_VERSION_MINOR 94 >>>>> #define LIBAVCODEC_VERSION_MICRO 100 >>>>> >>>>> #define LIBAVCODEC_VERSION_INT AV_VERSION_INT(LIBAVCODEC_VERSION_MAJOR, \ >>>>> diff --git a/libavformat/dump.c b/libavformat/dump.c >>>>> index 117c681..c5f0bd6 100644 >>>>> --- a/libavformat/dump.c >>>>> +++ b/libavformat/dump.c >>>>> @@ -34,6 +34,7 @@ >>>>> #include "libavutil/replaygain.h" >>>>> #include "libavutil/spherical.h" >>>>> #include "libavutil/stereo3d.h" >>>>> +#include "libavutil/timecode.h" >>>>> >>>>> #include "avformat.h" >>>>> >>>>> @@ -402,6 +403,23 @@ static void dump_dovi_conf(void *ctx, AVPacketSideData* sd) >>>>> dovi->dv_bl_signal_compatibility_id); >>>>> } >>>>> >>>>> +static void dump_s12m_timecode(void *ctx, AVPacketSideData* sd) >>>>> +{ >>>>> + uint32_t *tc = (uint32_t*)sd->data; >>>>> + int m = tc[0] & 3; >>>>> + >>>>> + if (sd->size != sizeof(uint32_t) * 4) { >> >> You already dereferenced tc before this check which might crash if the >> size is actually < sizeof(uint32_t). And why do you only look at the >> lowest two bits of m? Is this designed with future extensions of the >> timecode in mind? But even then one can't simply use the upper 30 bits >> to store something else, because that has not been documented. >> (Moreover, if one allowed extensions, one should allow sd->size to be >= >> sizeof(uint32_t) * 4.) > > It's from the comments for the side data, it used to avoid index overflow, > for the array is 4. I'll fix to get the m after the checking. I know it is to ensure that you don't overread. I'd simply consider it invalid data if the count were > 3. > > comments: > where the first uint32_t describes how many (1-3) of the other timecodes are used > > I think it's for the timecode SEI define 2bits for the count of used timecomes, > it's not related to SMPTE ST 12-1. vf_showinfo.c have dump AV_FRAME_DATA_S12M_TIMECODE, > I'm following it as format is same, but change m to & 3 for I think it's more simple. > > >> >>>>> + av_log(ctx, AV_LOG_ERROR, "invalid data"); >>>>> + return; >>>>> + } >>>>> + >>>>> + for (int j = 1; j <= m; j++) { >>>>> + char tcbuf[AV_TIMECODE_STR_SIZE]; >>>>> + av_timecode_make_smpte_tc_string(tcbuf, tc[j], 0); >>>>> + av_log(ctx, AV_LOG_INFO, "timecode - %s%s", tcbuf, j != m ? ", " : ""); >>>>> + } >>>>> +} >>>>> + >> >> - Andreas >> >> [1]: https://www.smpte.org/free-standards-and-publications >> _______________________________________________ >> 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". >
On Mon, Jun 29, 2020 at 03:52:00PM +0200, Andreas Rheinhardt wrote: > lance.lmwang@gmail.com: > > On Mon, Jun 29, 2020 at 02:06:49PM +0200, Andreas Rheinhardt wrote: > >> lance.lmwang@gmail.com: > >>> On Mon, Jun 29, 2020 at 04:53:21AM +0200, Andreas Rheinhardt wrote: > >>>> lance.lmwang@gmail.com: > >>>>> From: Limin Wang <lance.lmwang@gmail.com> > >>>>> > >>>>> Signed-off-by: Limin Wang <lance.lmwang@gmail.com> > >>>>> --- > >>>>> doc/APIchanges | 3 +++ > >>>>> libavcodec/avpacket.c | 1 + > >>>>> libavcodec/decode.c | 1 + > >>>>> libavcodec/packet.h | 8 ++++++++ > >>>>> libavcodec/version.h | 2 +- > >>>>> libavformat/dump.c | 22 ++++++++++++++++++++++ > >>>>> 6 files changed, 36 insertions(+), 1 deletion(-) > >>>>> > >>>>> diff --git a/doc/APIchanges b/doc/APIchanges > >>>>> index 1d6cc36..7cad7fa 100644 > >>>>> --- a/doc/APIchanges > >>>>> +++ b/doc/APIchanges > >>>>> @@ -15,6 +15,9 @@ libavutil: 2017-10-21 > >>>>> > >>>>> API changes, most recent first: > >>>>> > >>>>> +2020-06-xx - xxxxxxxxxx - lavc 58.94.100 - packet.h > >>>>> + Add AV_PKT_DATA_S12M_TIMECODE. > >>>>> + > >>>>> 2020-06-12 - b09fb030c1 - lavu 56.55.100 - pixdesc.h > >>>>> Add AV_PIX_FMT_X2RGB10. > >>>>> > >>>>> diff --git a/libavcodec/avpacket.c b/libavcodec/avpacket.c > >>>>> index dce26cb..a4316a7 100644 > >>>>> --- a/libavcodec/avpacket.c > >>>>> +++ b/libavcodec/avpacket.c > >>>>> @@ -400,6 +400,7 @@ const char *av_packet_side_data_name(enum AVPacketSideDataType type) > >>>>> case AV_PKT_DATA_PRFT: return "Producer Reference Time"; > >>>>> case AV_PKT_DATA_ICC_PROFILE: return "ICC Profile"; > >>>>> case AV_PKT_DATA_DOVI_CONF: return "DOVI configuration record"; > >>>>> + case AV_PKT_DATA_S12M_TIMECODE: return "SMPTE 12-1 timecode"; > >>>>> } > >>>>> return NULL; > >>>>> } > >>>>> diff --git a/libavcodec/decode.c b/libavcodec/decode.c > >>>>> index a4e50c0..a2bf0cc 100644 > >>>>> --- a/libavcodec/decode.c > >>>>> +++ b/libavcodec/decode.c > >>>>> @@ -1699,6 +1699,7 @@ int ff_decode_frame_props(AVCodecContext *avctx, AVFrame *frame) > >>>>> { AV_PKT_DATA_CONTENT_LIGHT_LEVEL, AV_FRAME_DATA_CONTENT_LIGHT_LEVEL }, > >>>>> { AV_PKT_DATA_A53_CC, AV_FRAME_DATA_A53_CC }, > >>>>> { AV_PKT_DATA_ICC_PROFILE, AV_FRAME_DATA_ICC_PROFILE }, > >>>>> + { AV_PKT_DATA_S12M_TIMECODE, AV_FRAME_DATA_S12M_TIMECODE }, > >>>>> }; > >>>>> > >>>>> if (pkt) { > >>>>> diff --git a/libavcodec/packet.h b/libavcodec/packet.h > >>>>> index 41485f4..72cb4f7 100644 > >>>>> --- a/libavcodec/packet.h > >>>>> +++ b/libavcodec/packet.h > >>>>> @@ -283,6 +283,14 @@ enum AVPacketSideDataType { > >>>>> AV_PKT_DATA_DOVI_CONF, > >>>>> > >>>>> /** > >>>>> + * 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 format is documented in a C source file, not a public header? Sure > >>>> about that? > >>> > >>> For AV_PKT_DATA_S12M_TIMECODE use the same format with AV_FRAME_DATA_S12M_TIMECODE, so I copy the > >>> comments for AV_FRAME_DATA_S12M_TIMECODE, it's not my comments for it. I think the format is described > >>> in "SMPTE ST 12-2:2014" standard document, but it's not free for download. > >>> > >> > >> It's SMPTE ST 12-1:2014 and this standard is currently (until 21 July) > >> available for free [1] due to Corona. > > > > oh, it's good to hear it's free due to Corona. I haven't notice it's online. > > > >> > >>> What's your suggestion to change? > >>> > >> > >> The best is probably to incorporate the documentation of the format into > >> the documentation of av_timecode_get_smpte_from_framenum. > > OK, I'll remove the comments for the timecode format. > >> > >>>> > >>>>> + */ > >>>>> + AV_PKT_DATA_S12M_TIMECODE, > >>>>> + > >>>>> + /** > >>>>> * The number of side data types. > >>>>> * This is not part of the public API/ABI in the sense that it may > >>>>> * change when new side data types are added. > >>>>> diff --git a/libavcodec/version.h b/libavcodec/version.h > >>>>> index 0359302..482cc6d 100644 > >>>>> --- a/libavcodec/version.h > >>>>> +++ b/libavcodec/version.h > >>>>> @@ -28,7 +28,7 @@ > >>>>> #include "libavutil/version.h" > >>>>> > >>>>> #define LIBAVCODEC_VERSION_MAJOR 58 > >>>>> -#define LIBAVCODEC_VERSION_MINOR 93 > >>>>> +#define LIBAVCODEC_VERSION_MINOR 94 > >>>>> #define LIBAVCODEC_VERSION_MICRO 100 > >>>>> > >>>>> #define LIBAVCODEC_VERSION_INT AV_VERSION_INT(LIBAVCODEC_VERSION_MAJOR, \ > >>>>> diff --git a/libavformat/dump.c b/libavformat/dump.c > >>>>> index 117c681..c5f0bd6 100644 > >>>>> --- a/libavformat/dump.c > >>>>> +++ b/libavformat/dump.c > >>>>> @@ -34,6 +34,7 @@ > >>>>> #include "libavutil/replaygain.h" > >>>>> #include "libavutil/spherical.h" > >>>>> #include "libavutil/stereo3d.h" > >>>>> +#include "libavutil/timecode.h" > >>>>> > >>>>> #include "avformat.h" > >>>>> > >>>>> @@ -402,6 +403,23 @@ static void dump_dovi_conf(void *ctx, AVPacketSideData* sd) > >>>>> dovi->dv_bl_signal_compatibility_id); > >>>>> } > >>>>> > >>>>> +static void dump_s12m_timecode(void *ctx, AVPacketSideData* sd) > >>>>> +{ > >>>>> + uint32_t *tc = (uint32_t*)sd->data; > >>>>> + int m = tc[0] & 3; > >>>>> + > >>>>> + if (sd->size != sizeof(uint32_t) * 4) { > >> > >> You already dereferenced tc before this check which might crash if the > >> size is actually < sizeof(uint32_t). And why do you only look at the > >> lowest two bits of m? Is this designed with future extensions of the > >> timecode in mind? But even then one can't simply use the upper 30 bits > >> to store something else, because that has not been documented. > >> (Moreover, if one allowed extensions, one should allow sd->size to be >= > >> sizeof(uint32_t) * 4.) > > > > It's from the comments for the side data, it used to avoid index overflow, > > for the array is 4. I'll fix to get the m after the checking. > > I know it is to ensure that you don't overread. I'd simply consider it > invalid data if the count were > 3. I agree, I'll update to report invalid data if count > 3. To keep consistent, I'll change the vf_showinfo.c dump to keep the code consistent later. > > > > > comments: > > where the first uint32_t describes how many (1-3) of the other timecodes are used > > > > I think it's for the timecode SEI define 2bits for the count of used timecomes, > > it's not related to SMPTE ST 12-1. vf_showinfo.c have dump AV_FRAME_DATA_S12M_TIMECODE, > > I'm following it as format is same, but change m to & 3 for I think it's more simple. > > > > > >> > >>>>> + av_log(ctx, AV_LOG_ERROR, "invalid data"); > >>>>> + return; > >>>>> + } > >>>>> + > >>>>> + for (int j = 1; j <= m; j++) { > >>>>> + char tcbuf[AV_TIMECODE_STR_SIZE]; > >>>>> + av_timecode_make_smpte_tc_string(tcbuf, tc[j], 0); > >>>>> + av_log(ctx, AV_LOG_INFO, "timecode - %s%s", tcbuf, j != m ? ", " : ""); > >>>>> + } > >>>>> +} > >>>>> + > >> > >> - Andreas > >> > >> [1]: https://www.smpte.org/free-standards-and-publications > >> _______________________________________________ > >> 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". > > > > _______________________________________________ > 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".
On Mon, Jun 29, 2020 at 03:52:00PM +0200, Andreas Rheinhardt wrote: > lance.lmwang@gmail.com: > > On Mon, Jun 29, 2020 at 02:06:49PM +0200, Andreas Rheinhardt wrote: > >> lance.lmwang@gmail.com: > >>> On Mon, Jun 29, 2020 at 04:53:21AM +0200, Andreas Rheinhardt wrote: > >>>> lance.lmwang@gmail.com: > >>>>> From: Limin Wang <lance.lmwang@gmail.com> > >>>>> > >>>>> Signed-off-by: Limin Wang <lance.lmwang@gmail.com> > >>>>> --- > >>>>> doc/APIchanges | 3 +++ > >>>>> libavcodec/avpacket.c | 1 + > >>>>> libavcodec/decode.c | 1 + > >>>>> libavcodec/packet.h | 8 ++++++++ > >>>>> libavcodec/version.h | 2 +- > >>>>> libavformat/dump.c | 22 ++++++++++++++++++++++ > >>>>> 6 files changed, 36 insertions(+), 1 deletion(-) > >>>>> > >>>>> diff --git a/doc/APIchanges b/doc/APIchanges > >>>>> index 1d6cc36..7cad7fa 100644 > >>>>> --- a/doc/APIchanges > >>>>> +++ b/doc/APIchanges > >>>>> @@ -15,6 +15,9 @@ libavutil: 2017-10-21 > >>>>> > >>>>> API changes, most recent first: > >>>>> > >>>>> +2020-06-xx - xxxxxxxxxx - lavc 58.94.100 - packet.h > >>>>> + Add AV_PKT_DATA_S12M_TIMECODE. > >>>>> + > >>>>> 2020-06-12 - b09fb030c1 - lavu 56.55.100 - pixdesc.h > >>>>> Add AV_PIX_FMT_X2RGB10. > >>>>> > >>>>> diff --git a/libavcodec/avpacket.c b/libavcodec/avpacket.c > >>>>> index dce26cb..a4316a7 100644 > >>>>> --- a/libavcodec/avpacket.c > >>>>> +++ b/libavcodec/avpacket.c > >>>>> @@ -400,6 +400,7 @@ const char *av_packet_side_data_name(enum AVPacketSideDataType type) > >>>>> case AV_PKT_DATA_PRFT: return "Producer Reference Time"; > >>>>> case AV_PKT_DATA_ICC_PROFILE: return "ICC Profile"; > >>>>> case AV_PKT_DATA_DOVI_CONF: return "DOVI configuration record"; > >>>>> + case AV_PKT_DATA_S12M_TIMECODE: return "SMPTE 12-1 timecode"; > >>>>> } > >>>>> return NULL; > >>>>> } > >>>>> diff --git a/libavcodec/decode.c b/libavcodec/decode.c > >>>>> index a4e50c0..a2bf0cc 100644 > >>>>> --- a/libavcodec/decode.c > >>>>> +++ b/libavcodec/decode.c > >>>>> @@ -1699,6 +1699,7 @@ int ff_decode_frame_props(AVCodecContext *avctx, AVFrame *frame) > >>>>> { AV_PKT_DATA_CONTENT_LIGHT_LEVEL, AV_FRAME_DATA_CONTENT_LIGHT_LEVEL }, > >>>>> { AV_PKT_DATA_A53_CC, AV_FRAME_DATA_A53_CC }, > >>>>> { AV_PKT_DATA_ICC_PROFILE, AV_FRAME_DATA_ICC_PROFILE }, > >>>>> + { AV_PKT_DATA_S12M_TIMECODE, AV_FRAME_DATA_S12M_TIMECODE }, > >>>>> }; > >>>>> > >>>>> if (pkt) { > >>>>> diff --git a/libavcodec/packet.h b/libavcodec/packet.h > >>>>> index 41485f4..72cb4f7 100644 > >>>>> --- a/libavcodec/packet.h > >>>>> +++ b/libavcodec/packet.h > >>>>> @@ -283,6 +283,14 @@ enum AVPacketSideDataType { > >>>>> AV_PKT_DATA_DOVI_CONF, > >>>>> > >>>>> /** > >>>>> + * 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 format is documented in a C source file, not a public header? Sure > >>>> about that? > >>> > >>> For AV_PKT_DATA_S12M_TIMECODE use the same format with AV_FRAME_DATA_S12M_TIMECODE, so I copy the > >>> comments for AV_FRAME_DATA_S12M_TIMECODE, it's not my comments for it. I think the format is described > >>> in "SMPTE ST 12-2:2014" standard document, but it's not free for download. > >>> > >> > >> It's SMPTE ST 12-1:2014 and this standard is currently (until 21 July) > >> available for free [1] due to Corona. > > > > oh, it's good to hear it's free due to Corona. I haven't notice it's online. > > > >> > >>> What's your suggestion to change? > >>> > >> > >> The best is probably to incorporate the documentation of the format into > >> the documentation of av_timecode_get_smpte_from_framenum. > > OK, I'll remove the comments for the timecode format. > >> > >>>> > >>>>> + */ > >>>>> + AV_PKT_DATA_S12M_TIMECODE, > >>>>> + > >>>>> + /** > >>>>> * The number of side data types. > >>>>> * This is not part of the public API/ABI in the sense that it may > >>>>> * change when new side data types are added. > >>>>> diff --git a/libavcodec/version.h b/libavcodec/version.h > >>>>> index 0359302..482cc6d 100644 > >>>>> --- a/libavcodec/version.h > >>>>> +++ b/libavcodec/version.h > >>>>> @@ -28,7 +28,7 @@ > >>>>> #include "libavutil/version.h" > >>>>> > >>>>> #define LIBAVCODEC_VERSION_MAJOR 58 > >>>>> -#define LIBAVCODEC_VERSION_MINOR 93 > >>>>> +#define LIBAVCODEC_VERSION_MINOR 94 > >>>>> #define LIBAVCODEC_VERSION_MICRO 100 > >>>>> > >>>>> #define LIBAVCODEC_VERSION_INT AV_VERSION_INT(LIBAVCODEC_VERSION_MAJOR, \ > >>>>> diff --git a/libavformat/dump.c b/libavformat/dump.c > >>>>> index 117c681..c5f0bd6 100644 > >>>>> --- a/libavformat/dump.c > >>>>> +++ b/libavformat/dump.c > >>>>> @@ -34,6 +34,7 @@ > >>>>> #include "libavutil/replaygain.h" > >>>>> #include "libavutil/spherical.h" > >>>>> #include "libavutil/stereo3d.h" > >>>>> +#include "libavutil/timecode.h" > >>>>> > >>>>> #include "avformat.h" > >>>>> > >>>>> @@ -402,6 +403,23 @@ static void dump_dovi_conf(void *ctx, AVPacketSideData* sd) > >>>>> dovi->dv_bl_signal_compatibility_id); > >>>>> } > >>>>> > >>>>> +static void dump_s12m_timecode(void *ctx, AVPacketSideData* sd) > >>>>> +{ > >>>>> + uint32_t *tc = (uint32_t*)sd->data; > >>>>> + int m = tc[0] & 3; > >>>>> + > >>>>> + if (sd->size != sizeof(uint32_t) * 4) { > >> > >> You already dereferenced tc before this check which might crash if the > >> size is actually < sizeof(uint32_t). And why do you only look at the > >> lowest two bits of m? Is this designed with future extensions of the > >> timecode in mind? But even then one can't simply use the upper 30 bits > >> to store something else, because that has not been documented. > >> (Moreover, if one allowed extensions, one should allow sd->size to be >= > >> sizeof(uint32_t) * 4.) > > > > It's from the comments for the side data, it used to avoid index overflow, > > for the array is 4. I'll fix to get the m after the checking. > > I know it is to ensure that you don't overread. I'd simply consider it > invalid data if the count were > 3. Andreas, please help to review the updated v3 version, hopes it solves all of your comments. > > > > > comments: > > where the first uint32_t describes how many (1-3) of the other timecodes are used > > > > I think it's for the timecode SEI define 2bits for the count of used timecomes, > > it's not related to SMPTE ST 12-1. vf_showinfo.c have dump AV_FRAME_DATA_S12M_TIMECODE, > > I'm following it as format is same, but change m to & 3 for I think it's more simple. > > > > > >> > >>>>> + av_log(ctx, AV_LOG_ERROR, "invalid data"); > >>>>> + return; > >>>>> + } > >>>>> + > >>>>> + for (int j = 1; j <= m; j++) { > >>>>> + char tcbuf[AV_TIMECODE_STR_SIZE]; > >>>>> + av_timecode_make_smpte_tc_string(tcbuf, tc[j], 0); > >>>>> + av_log(ctx, AV_LOG_INFO, "timecode - %s%s", tcbuf, j != m ? ", " : ""); > >>>>> + } > >>>>> +} > >>>>> + > >> > >> - Andreas > >> > >> [1]: https://www.smpte.org/free-standards-and-publications > >> _______________________________________________ > >> 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". > > > > _______________________________________________ > 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/doc/APIchanges b/doc/APIchanges index 1d6cc36..7cad7fa 100644 --- a/doc/APIchanges +++ b/doc/APIchanges @@ -15,6 +15,9 @@ libavutil: 2017-10-21 API changes, most recent first: +2020-06-xx - xxxxxxxxxx - lavc 58.94.100 - packet.h + Add AV_PKT_DATA_S12M_TIMECODE. + 2020-06-12 - b09fb030c1 - lavu 56.55.100 - pixdesc.h Add AV_PIX_FMT_X2RGB10. diff --git a/libavcodec/avpacket.c b/libavcodec/avpacket.c index dce26cb..a4316a7 100644 --- a/libavcodec/avpacket.c +++ b/libavcodec/avpacket.c @@ -400,6 +400,7 @@ const char *av_packet_side_data_name(enum AVPacketSideDataType type) case AV_PKT_DATA_PRFT: return "Producer Reference Time"; case AV_PKT_DATA_ICC_PROFILE: return "ICC Profile"; case AV_PKT_DATA_DOVI_CONF: return "DOVI configuration record"; + case AV_PKT_DATA_S12M_TIMECODE: return "SMPTE 12-1 timecode"; } return NULL; } diff --git a/libavcodec/decode.c b/libavcodec/decode.c index a4e50c0..a2bf0cc 100644 --- a/libavcodec/decode.c +++ b/libavcodec/decode.c @@ -1699,6 +1699,7 @@ int ff_decode_frame_props(AVCodecContext *avctx, AVFrame *frame) { AV_PKT_DATA_CONTENT_LIGHT_LEVEL, AV_FRAME_DATA_CONTENT_LIGHT_LEVEL }, { AV_PKT_DATA_A53_CC, AV_FRAME_DATA_A53_CC }, { AV_PKT_DATA_ICC_PROFILE, AV_FRAME_DATA_ICC_PROFILE }, + { AV_PKT_DATA_S12M_TIMECODE, AV_FRAME_DATA_S12M_TIMECODE }, }; if (pkt) { diff --git a/libavcodec/packet.h b/libavcodec/packet.h index 41485f4..72cb4f7 100644 --- a/libavcodec/packet.h +++ b/libavcodec/packet.h @@ -283,6 +283,14 @@ enum AVPacketSideDataType { AV_PKT_DATA_DOVI_CONF, /** + * 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. + */ + AV_PKT_DATA_S12M_TIMECODE, + + /** * The number of side data types. * This is not part of the public API/ABI in the sense that it may * change when new side data types are added. diff --git a/libavcodec/version.h b/libavcodec/version.h index 0359302..482cc6d 100644 --- a/libavcodec/version.h +++ b/libavcodec/version.h @@ -28,7 +28,7 @@ #include "libavutil/version.h" #define LIBAVCODEC_VERSION_MAJOR 58 -#define LIBAVCODEC_VERSION_MINOR 93 +#define LIBAVCODEC_VERSION_MINOR 94 #define LIBAVCODEC_VERSION_MICRO 100 #define LIBAVCODEC_VERSION_INT AV_VERSION_INT(LIBAVCODEC_VERSION_MAJOR, \ diff --git a/libavformat/dump.c b/libavformat/dump.c index 117c681..c5f0bd6 100644 --- a/libavformat/dump.c +++ b/libavformat/dump.c @@ -34,6 +34,7 @@ #include "libavutil/replaygain.h" #include "libavutil/spherical.h" #include "libavutil/stereo3d.h" +#include "libavutil/timecode.h" #include "avformat.h" @@ -402,6 +403,23 @@ static void dump_dovi_conf(void *ctx, AVPacketSideData* sd) dovi->dv_bl_signal_compatibility_id); } +static void dump_s12m_timecode(void *ctx, AVPacketSideData* sd) +{ + uint32_t *tc = (uint32_t*)sd->data; + int m = tc[0] & 3; + + if (sd->size != sizeof(uint32_t) * 4) { + av_log(ctx, AV_LOG_ERROR, "invalid data"); + return; + } + + for (int j = 1; j <= m; j++) { + char tcbuf[AV_TIMECODE_STR_SIZE]; + av_timecode_make_smpte_tc_string(tcbuf, tc[j], 0); + av_log(ctx, AV_LOG_INFO, "timecode - %s%s", tcbuf, j != m ? ", " : ""); + } +} + static void dump_sidedata(void *ctx, AVStream *st, const char *indent) { int i; @@ -468,6 +486,10 @@ static void dump_sidedata(void *ctx, AVStream *st, const char *indent) av_log(ctx, AV_LOG_INFO, "DOVI configuration record: "); dump_dovi_conf(ctx, &sd); break; + case AV_PKT_DATA_S12M_TIMECODE: + av_log(ctx, AV_LOG_INFO, "S12M timecode: "); + dump_s12m_timecode(ctx, &sd); + break; default: av_log(ctx, AV_LOG_INFO, "unknown side data type %d (%d bytes)", sd.type, sd.size);