diff mbox series

[FFmpeg-devel,3/6] avcodec/h264_slice: export S12M timecode side data

Message ID 1594769402-22526-3-git-send-email-lance.lmwang@gmail.com
State New
Headers show
Series [FFmpeg-devel,1/6] avcodec: add AV_CODEC_EXPORT_DATA_S12M_TC flag to export S12M timecode | expand

Checks

Context Check Description
andriy/default pending
andriy/make success Make finished
andriy/make_fate success Make fate finished

Commit Message

Lance Wang July 14, 2020, 11:29 p.m. UTC 
From: Limin Wang <lance.lmwang@gmail.com>

Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
---
 libavcodec/h264_slice.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

Comments

Kieran Kunhya July 14, 2020, 11:54 p.m. UTC | #1 
On Wed, 15 Jul 2020 at 00:36, <lance.lmwang@gmail.com> wrote:

> From: Limin Wang <lance.lmwang@gmail.com>
>
> Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
> ---
>  libavcodec/h264_slice.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
>
> diff --git a/libavcodec/h264_slice.c b/libavcodec/h264_slice.c
> index c7b2764..db720de 100644
> --- a/libavcodec/h264_slice.c
> +++ b/libavcodec/h264_slice.c
> @@ -1307,6 +1307,7 @@ static int h264_export_frame_props(H264Context *h)
>      if (h->sei.picture_timing.timecode_cnt > 0) {
>          uint32_t *tc_sd;
>          char tcbuf[AV_TIMECODE_STR_SIZE];
> +        uint32_t *s12m_sd;
>
>          AVFrameSideData *tcside = av_frame_new_side_data(out,
>
> AV_FRAME_DATA_S12M_TIMECODE,
> @@ -1316,6 +1317,12 @@ static int h264_export_frame_props(H264Context *h)
>
>          tc_sd = (uint32_t*)tcside->data;
>          tc_sd[0] = h->sei.picture_timing.timecode_cnt;
> +        if (h->avctx->export_side_data & AV_CODEC_EXPORT_DATA_S12M_TC) {
> +            s12m_sd = ff_add_s12m_timecode_side_data(h->avctx);
> +            if (!s12m_sd)
> +                return AVERROR(ENOMEM);
> +            s12m_sd[0] = tc_sd[0];
> +        }
>
>          for (int i = 0; i < tc_sd[0]; i++) {
>              int drop = h->sei.picture_timing.timecode[i].dropframe;
> @@ -1325,6 +1332,9 @@ static int h264_export_frame_props(H264Context *h)
>              int   ff = h->sei.picture_timing.timecode[i].frame;
>
>              tc_sd[i + 1] = av_timecode_get_smpte(h->avctx->framerate,
> drop, hh, mm, ss, ff);
> +            if (h->avctx->export_side_data &
> AV_CODEC_EXPORT_DATA_S12M_TC) {
> +                s12m_sd[i + 1] = tc_sd[i + 1];
> +            }
>              av_timecode_make_smpte_tc_string(tcbuf, tc_sd[i + 1], 0);
>              av_dict_set(&out->metadata, "timecode", tcbuf, 0);
>          }
> --


Does this not duplicate the existing logic?

Kieran
Lance Wang July 15, 2020, 1:19 a.m. UTC | #2 
On Wed, Jul 15, 2020 at 12:54:35AM +0100, Kieran Kunhya wrote:
> On Wed, 15 Jul 2020 at 00:36, <lance.lmwang@gmail.com> wrote:
> 
> > From: Limin Wang <lance.lmwang@gmail.com>
> >
> > Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
> > ---
> >  libavcodec/h264_slice.c | 10 ++++++++++
> >  1 file changed, 10 insertions(+)
> >
> > diff --git a/libavcodec/h264_slice.c b/libavcodec/h264_slice.c
> > index c7b2764..db720de 100644
> > --- a/libavcodec/h264_slice.c
> > +++ b/libavcodec/h264_slice.c
> > @@ -1307,6 +1307,7 @@ static int h264_export_frame_props(H264Context *h)
> >      if (h->sei.picture_timing.timecode_cnt > 0) {
> >          uint32_t *tc_sd;
> >          char tcbuf[AV_TIMECODE_STR_SIZE];
> > +        uint32_t *s12m_sd;
> >
> >          AVFrameSideData *tcside = av_frame_new_side_data(out,
> >
> > AV_FRAME_DATA_S12M_TIMECODE,
> > @@ -1316,6 +1317,12 @@ static int h264_export_frame_props(H264Context *h)
> >
> >          tc_sd = (uint32_t*)tcside->data;
> >          tc_sd[0] = h->sei.picture_timing.timecode_cnt;
> > +        if (h->avctx->export_side_data & AV_CODEC_EXPORT_DATA_S12M_TC) {
> > +            s12m_sd = ff_add_s12m_timecode_side_data(h->avctx);
> > +            if (!s12m_sd)
> > +                return AVERROR(ENOMEM);
> > +            s12m_sd[0] = tc_sd[0];
> > +        }
> >
> >          for (int i = 0; i < tc_sd[0]; i++) {
> >              int drop = h->sei.picture_timing.timecode[i].dropframe;
> > @@ -1325,6 +1332,9 @@ static int h264_export_frame_props(H264Context *h)
> >              int   ff = h->sei.picture_timing.timecode[i].frame;
> >
> >              tc_sd[i + 1] = av_timecode_get_smpte(h->avctx->framerate,
> > drop, hh, mm, ss, ff);
> > +            if (h->avctx->export_side_data &
> > AV_CODEC_EXPORT_DATA_S12M_TC) {
> > +                s12m_sd[i + 1] = tc_sd[i + 1];
> > +            }
> >              av_timecode_make_smpte_tc_string(tcbuf, tc_sd[i + 1], 0);
> >              av_dict_set(&out->metadata, "timecode", tcbuf, 0);
> >          }
> > --
> 
> 
> Does this not duplicate the existing logic?

It's export packet timecode sidedata, so that you can export the TC side data to stream 
level. patch#5 show the command how to use it, before the patch, you had to input timecode
to export the TC to tmcd track for mp4. Please give advise if you have better way to get it,
thanks. 

> 
> Kieran
Kieran Kunhya July 15, 2020, 8:16 a.m. UTC | #3 
On Wed, 15 Jul 2020 at 02:19, <lance.lmwang@gmail.com> wrote:

> On Wed, Jul 15, 2020 at 12:54:35AM +0100, Kieran Kunhya wrote:
> > On Wed, 15 Jul 2020 at 00:36, <lance.lmwang@gmail.com> wrote:
> >
> > > From: Limin Wang <lance.lmwang@gmail.com>
> > >
> > > Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
> > > ---
> > >  libavcodec/h264_slice.c | 10 ++++++++++
> > >  1 file changed, 10 insertions(+)
> > >
> > > diff --git a/libavcodec/h264_slice.c b/libavcodec/h264_slice.c
> > > index c7b2764..db720de 100644
> > > --- a/libavcodec/h264_slice.c
> > > +++ b/libavcodec/h264_slice.c
> > > @@ -1307,6 +1307,7 @@ static int h264_export_frame_props(H264Context
> *h)
> > >      if (h->sei.picture_timing.timecode_cnt > 0) {
> > >          uint32_t *tc_sd;
> > >          char tcbuf[AV_TIMECODE_STR_SIZE];
> > > +        uint32_t *s12m_sd;
> > >
> > >          AVFrameSideData *tcside = av_frame_new_side_data(out,
> > >
> > > AV_FRAME_DATA_S12M_TIMECODE,
> > > @@ -1316,6 +1317,12 @@ static int h264_export_frame_props(H264Context
> *h)
> > >
> > >          tc_sd = (uint32_t*)tcside->data;
> > >          tc_sd[0] = h->sei.picture_timing.timecode_cnt;
> > > +        if (h->avctx->export_side_data &
> AV_CODEC_EXPORT_DATA_S12M_TC) {
> > > +            s12m_sd = ff_add_s12m_timecode_side_data(h->avctx);
> > > +            if (!s12m_sd)
> > > +                return AVERROR(ENOMEM);
> > > +            s12m_sd[0] = tc_sd[0];
> > > +        }
> > >
> > >          for (int i = 0; i < tc_sd[0]; i++) {
> > >              int drop = h->sei.picture_timing.timecode[i].dropframe;
> > > @@ -1325,6 +1332,9 @@ static int h264_export_frame_props(H264Context
> *h)
> > >              int   ff = h->sei.picture_timing.timecode[i].frame;
> > >
> > >              tc_sd[i + 1] = av_timecode_get_smpte(h->avctx->framerate,
> > > drop, hh, mm, ss, ff);
> > > +            if (h->avctx->export_side_data &
> > > AV_CODEC_EXPORT_DATA_S12M_TC) {
> > > +                s12m_sd[i + 1] = tc_sd[i + 1];
> > > +            }
> > >              av_timecode_make_smpte_tc_string(tcbuf, tc_sd[i + 1], 0);
> > >              av_dict_set(&out->metadata, "timecode", tcbuf, 0);
> > >          }
> > > --
> >
> >
> > Does this not duplicate the existing logic?
>
> It's export packet timecode sidedata, so that you can export the TC side
> data to stream
> level. patch#5 show the command how to use it, before the patch, you had
> to input timecode
> to export the TC to tmcd track for mp4. Please give advise if you have
> better way to get it,
> thanks.
>

I don't understand, timecode side data is already exported from H.264.
Why do you need to create two side datas?

Kieran
Lance Wang July 15, 2020, 1:44 p.m. UTC | #4 
On Wed, Jul 15, 2020 at 09:16:16AM +0100, Kieran Kunhya wrote:
> On Wed, 15 Jul 2020 at 02:19, <lance.lmwang@gmail.com> wrote:
> 
> > On Wed, Jul 15, 2020 at 12:54:35AM +0100, Kieran Kunhya wrote:
> > > On Wed, 15 Jul 2020 at 00:36, <lance.lmwang@gmail.com> wrote:
> > >
> > > > From: Limin Wang <lance.lmwang@gmail.com>
> > > >
> > > > Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
> > > > ---
> > > >  libavcodec/h264_slice.c | 10 ++++++++++
> > > >  1 file changed, 10 insertions(+)
> > > >
> > > > diff --git a/libavcodec/h264_slice.c b/libavcodec/h264_slice.c
> > > > index c7b2764..db720de 100644
> > > > --- a/libavcodec/h264_slice.c
> > > > +++ b/libavcodec/h264_slice.c
> > > > @@ -1307,6 +1307,7 @@ static int h264_export_frame_props(H264Context
> > *h)
> > > >      if (h->sei.picture_timing.timecode_cnt > 0) {
> > > >          uint32_t *tc_sd;
> > > >          char tcbuf[AV_TIMECODE_STR_SIZE];
> > > > +        uint32_t *s12m_sd;
> > > >
> > > >          AVFrameSideData *tcside = av_frame_new_side_data(out,
> > > >
> > > > AV_FRAME_DATA_S12M_TIMECODE,
> > > > @@ -1316,6 +1317,12 @@ static int h264_export_frame_props(H264Context
> > *h)
> > > >
> > > >          tc_sd = (uint32_t*)tcside->data;
> > > >          tc_sd[0] = h->sei.picture_timing.timecode_cnt;
> > > > +        if (h->avctx->export_side_data &
> > AV_CODEC_EXPORT_DATA_S12M_TC) {
> > > > +            s12m_sd = ff_add_s12m_timecode_side_data(h->avctx);
> > > > +            if (!s12m_sd)
> > > > +                return AVERROR(ENOMEM);
> > > > +            s12m_sd[0] = tc_sd[0];
> > > > +        }
> > > >
> > > >          for (int i = 0; i < tc_sd[0]; i++) {
> > > >              int drop = h->sei.picture_timing.timecode[i].dropframe;
> > > > @@ -1325,6 +1332,9 @@ static int h264_export_frame_props(H264Context
> > *h)
> > > >              int   ff = h->sei.picture_timing.timecode[i].frame;
> > > >
> > > >              tc_sd[i + 1] = av_timecode_get_smpte(h->avctx->framerate,
> > > > drop, hh, mm, ss, ff);
> > > > +            if (h->avctx->export_side_data &
> > > > AV_CODEC_EXPORT_DATA_S12M_TC) {
> > > > +                s12m_sd[i + 1] = tc_sd[i + 1];
> > > > +            }
> > > >              av_timecode_make_smpte_tc_string(tcbuf, tc_sd[i + 1], 0);
> > > >              av_dict_set(&out->metadata, "timecode", tcbuf, 0);
> > > >          }
> > > > --
> > >
> > >
> > > Does this not duplicate the existing logic?
> >
> > It's export packet timecode sidedata, so that you can export the TC side
> > data to stream
> > level. patch#5 show the command how to use it, before the patch, you had
> > to input timecode
> > to export the TC to tmcd track for mp4. Please give advise if you have
> > better way to get it,
> > thanks.
> >
> 
> I don't understand, timecode side data is already exported from H.264.
> Why do you need to create two side datas?

yes, it's better to have a function to copy the side data from frame to
packet level, I'm not sure where is the proper place and how to make the
frame and packet is sync by the timecode. Maybe libavcodec/encode.c is a
good place, but it didn't make sense if the video is copy directly. 

> 
> Kieran
> _______________________________________________
> 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".
Kieran Kunhya July 15, 2020, 3:16 p.m. UTC | #5 
If you look a few lines above, we already set the timecode side data.

Sent from my mobile device

On Wed, 15 Jul 2020, 14:44 , <lance.lmwang@gmail.com> wrote:

> On Wed, Jul 15, 2020 at 09:16:16AM +0100, Kieran Kunhya wrote:
> > On Wed, 15 Jul 2020 at 02:19, <lance.lmwang@gmail.com> wrote:
> >
> > > On Wed, Jul 15, 2020 at 12:54:35AM +0100, Kieran Kunhya wrote:
> > > > On Wed, 15 Jul 2020 at 00:36, <lance.lmwang@gmail.com> wrote:
> > > >
> > > > > From: Limin Wang <lance.lmwang@gmail.com>
> > > > >
> > > > > Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
> > > > > ---
> > > > >  libavcodec/h264_slice.c | 10 ++++++++++
> > > > >  1 file changed, 10 insertions(+)
> > > > >
> > > > > diff --git a/libavcodec/h264_slice.c b/libavcodec/h264_slice.c
> > > > > index c7b2764..db720de 100644
> > > > > --- a/libavcodec/h264_slice.c
> > > > > +++ b/libavcodec/h264_slice.c
> > > > > @@ -1307,6 +1307,7 @@ static int
> h264_export_frame_props(H264Context
> > > *h)
> > > > >      if (h->sei.picture_timing.timecode_cnt > 0) {
> > > > >          uint32_t *tc_sd;
> > > > >          char tcbuf[AV_TIMECODE_STR_SIZE];
> > > > > +        uint32_t *s12m_sd;
> > > > >
> > > > >          AVFrameSideData *tcside = av_frame_new_side_data(out,
> > > > >
> > > > > AV_FRAME_DATA_S12M_TIMECODE,
> > > > > @@ -1316,6 +1317,12 @@ static int
> h264_export_frame_props(H264Context
> > > *h)
> > > > >
> > > > >          tc_sd = (uint32_t*)tcside->data;
> > > > >          tc_sd[0] = h->sei.picture_timing.timecode_cnt;
> > > > > +        if (h->avctx->export_side_data &
> > > AV_CODEC_EXPORT_DATA_S12M_TC) {
> > > > > +            s12m_sd = ff_add_s12m_timecode_side_data(h->avctx);
> > > > > +            if (!s12m_sd)
> > > > > +                return AVERROR(ENOMEM);
> > > > > +            s12m_sd[0] = tc_sd[0];
> > > > > +        }
> > > > >
> > > > >          for (int i = 0; i < tc_sd[0]; i++) {
> > > > >              int drop =
> h->sei.picture_timing.timecode[i].dropframe;
> > > > > @@ -1325,6 +1332,9 @@ static int
> h264_export_frame_props(H264Context
> > > *h)
> > > > >              int   ff = h->sei.picture_timing.timecode[i].frame;
> > > > >
> > > > >              tc_sd[i + 1] =
> av_timecode_get_smpte(h->avctx->framerate,
> > > > > drop, hh, mm, ss, ff);
> > > > > +            if (h->avctx->export_side_data &
> > > > > AV_CODEC_EXPORT_DATA_S12M_TC) {
> > > > > +                s12m_sd[i + 1] = tc_sd[i + 1];
> > > > > +            }
> > > > >              av_timecode_make_smpte_tc_string(tcbuf, tc_sd[i + 1],
> 0);
> > > > >              av_dict_set(&out->metadata, "timecode", tcbuf, 0);
> > > > >          }
> > > > > --
> > > >
> > > >
> > > > Does this not duplicate the existing logic?
> > >
> > > It's export packet timecode sidedata, so that you can export the TC
> side
> > > data to stream
> > > level. patch#5 show the command how to use it, before the patch, you
> had
> > > to input timecode
> > > to export the TC to tmcd track for mp4. Please give advise if you have
> > > better way to get it,
> > > thanks.
> > >
> >
> > I don't understand, timecode side data is already exported from H.264.
> > Why do you need to create two side datas?
>
> yes, it's better to have a function to copy the side data from frame to
> packet level, I'm not sure where is the proper place and how to make the
> frame and packet is sync by the timecode. Maybe libavcodec/encode.c is a
> good place, but it didn't make sense if the video is copy directly.
>
> >
> > Kieran
> > _______________________________________________
> > 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".
>
> --
> 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-request@ffmpeg.org with subject "unsubscribe".
Lance Wang July 15, 2020, 3:52 p.m. UTC | #6 
On Wed, Jul 15, 2020 at 04:16:38PM +0100, Kieran Kunhya wrote:
> If you look a few lines above, we already set the timecode side data.
> 

are you saying av_frame_new_side_data() above?  that's total different, in fact,
I'm try to use the same way with ff_add_cpb_side_data(), it'll put TC in the codec
level: avctx->nb_coded_side_data instead of frame level. By this way, I can export
the correct TC in the following patch for movenc, dv file format. Please refer to:
libavformat/utils.c line 4154 add_coded_side_data() is used to copy all AVPacketSideData
side data to st level. If someone have better way to reuse the frame side
data, please give your advice.

> Sent from my mobile device
> 
> On Wed, 15 Jul 2020, 14:44 , <lance.lmwang@gmail.com> wrote:
> 
> > On Wed, Jul 15, 2020 at 09:16:16AM +0100, Kieran Kunhya wrote:
> > > On Wed, 15 Jul 2020 at 02:19, <lance.lmwang@gmail.com> wrote:
> > >
> > > > On Wed, Jul 15, 2020 at 12:54:35AM +0100, Kieran Kunhya wrote:
> > > > > On Wed, 15 Jul 2020 at 00:36, <lance.lmwang@gmail.com> wrote:
> > > > >
> > > > > > From: Limin Wang <lance.lmwang@gmail.com>
> > > > > >
> > > > > > Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
> > > > > > ---
> > > > > >  libavcodec/h264_slice.c | 10 ++++++++++
> > > > > >  1 file changed, 10 insertions(+)
> > > > > >
> > > > > > diff --git a/libavcodec/h264_slice.c b/libavcodec/h264_slice.c
> > > > > > index c7b2764..db720de 100644
> > > > > > --- a/libavcodec/h264_slice.c
> > > > > > +++ b/libavcodec/h264_slice.c
> > > > > > @@ -1307,6 +1307,7 @@ static int
> > h264_export_frame_props(H264Context
> > > > *h)
> > > > > >      if (h->sei.picture_timing.timecode_cnt > 0) {
> > > > > >          uint32_t *tc_sd;
> > > > > >          char tcbuf[AV_TIMECODE_STR_SIZE];
> > > > > > +        uint32_t *s12m_sd;
> > > > > >
> > > > > >          AVFrameSideData *tcside = av_frame_new_side_data(out,
> > > > > >
> > > > > > AV_FRAME_DATA_S12M_TIMECODE,
> > > > > > @@ -1316,6 +1317,12 @@ static int
> > h264_export_frame_props(H264Context
> > > > *h)
> > > > > >
> > > > > >          tc_sd = (uint32_t*)tcside->data;
> > > > > >          tc_sd[0] = h->sei.picture_timing.timecode_cnt;
> > > > > > +        if (h->avctx->export_side_data &
> > > > AV_CODEC_EXPORT_DATA_S12M_TC) {
> > > > > > +            s12m_sd = ff_add_s12m_timecode_side_data(h->avctx);
> > > > > > +            if (!s12m_sd)
> > > > > > +                return AVERROR(ENOMEM);
> > > > > > +            s12m_sd[0] = tc_sd[0];
> > > > > > +        }
> > > > > >
> > > > > >          for (int i = 0; i < tc_sd[0]; i++) {
> > > > > >              int drop =
> > h->sei.picture_timing.timecode[i].dropframe;
> > > > > > @@ -1325,6 +1332,9 @@ static int
> > h264_export_frame_props(H264Context
> > > > *h)
> > > > > >              int   ff = h->sei.picture_timing.timecode[i].frame;
> > > > > >
> > > > > >              tc_sd[i + 1] =
> > av_timecode_get_smpte(h->avctx->framerate,
> > > > > > drop, hh, mm, ss, ff);
> > > > > > +            if (h->avctx->export_side_data &
> > > > > > AV_CODEC_EXPORT_DATA_S12M_TC) {
> > > > > > +                s12m_sd[i + 1] = tc_sd[i + 1];
> > > > > > +            }
> > > > > >              av_timecode_make_smpte_tc_string(tcbuf, tc_sd[i + 1],
> > 0);
> > > > > >              av_dict_set(&out->metadata, "timecode", tcbuf, 0);
> > > > > >          }
> > > > > > --
> > > > >
> > > > >
> > > > > Does this not duplicate the existing logic?
> > > >
> > > > It's export packet timecode sidedata, so that you can export the TC
> > side
> > > > data to stream
> > > > level. patch#5 show the command how to use it, before the patch, you
> > had
> > > > to input timecode
> > > > to export the TC to tmcd track for mp4. Please give advise if you have
> > > > better way to get it,
> > > > thanks.
> > > >
> > >
> > > I don't understand, timecode side data is already exported from H.264.
> > > Why do you need to create two side datas?
> >
> > yes, it's better to have a function to copy the side data from frame to
> > packet level, I'm not sure where is the proper place and how to make the
> > frame and packet is sync by the timecode. Maybe libavcodec/encode.c is a
> > good place, but it didn't make sense if the video is copy directly.
> >
> > >
> > > Kieran
> > > _______________________________________________
> > > 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".
> >
> > --
> > 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-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".
Hendrik Leppkes July 15, 2020, 5:01 p.m. UTC | #7 
On Wed, Jul 15, 2020 at 5:52 PM <lance.lmwang@gmail.com> wrote:
>
> On Wed, Jul 15, 2020 at 04:16:38PM +0100, Kieran Kunhya wrote:
> > If you look a few lines above, we already set the timecode side data.
> >
>
> are you saying av_frame_new_side_data() above?  that's total different, in fact,
> I'm try to use the same way with ff_add_cpb_side_data(), it'll put TC in the codec
> level: avctx->nb_coded_side_data instead of frame level. By this way, I can export
> the correct TC in the following patch for movenc, dv file format. Please refer to:
> libavformat/utils.c line 4154 add_coded_side_data() is used to copy all AVPacketSideData
> side data to st level. If someone have better way to reuse the frame side
> data, please give your advice.
>

CBP is a stream-level property, it doesn't usually change.
Timecodes are a per-frame property. There is no reason to store them
globally in avctx.

There is a big difference between those two. TC should be a per-frame
property, and if a muxer needs it, then the muxer should figure it out
from packets, not from a global store.

- Hendrik
Kieran Kunhya July 15, 2020, 5:02 p.m. UTC | #8 
On Wed, 15 Jul 2020 at 16:52, <lance.lmwang@gmail.com> wrote:

> On Wed, Jul 15, 2020 at 04:16:38PM +0100, Kieran Kunhya wrote:
> > If you look a few lines above, we already set the timecode side data.
> >
>
> are you saying av_frame_new_side_data() above?  that's total different, in
> fact,
> I'm try to use the same way with ff_add_cpb_side_data(), it'll put TC in
> the codec
> level: avctx->nb_coded_side_data instead of frame level. By this way, I
> can export
> the correct TC in the following patch for movenc, dv file format. Please
> refer to:
> libavformat/utils.c line 4154 add_coded_side_data() is used to copy all
> AVPacketSideData
> side data to st level. If someone have better way to reuse the frame side
> data, please give your advice.
>

Sorry this makes no sense at all.
What has decoding got to do with muxing?

Kieran
Lance Wang July 16, 2020, 12:15 a.m. UTC | #9 
On Wed, Jul 15, 2020 at 07:01:33PM +0200, Hendrik Leppkes wrote:
> On Wed, Jul 15, 2020 at 5:52 PM <lance.lmwang@gmail.com> wrote:
> >
> > On Wed, Jul 15, 2020 at 04:16:38PM +0100, Kieran Kunhya wrote:
> > > If you look a few lines above, we already set the timecode side data.
> > >
> >
> > are you saying av_frame_new_side_data() above?  that's total different, in fact,
> > I'm try to use the same way with ff_add_cpb_side_data(), it'll put TC in the codec
> > level: avctx->nb_coded_side_data instead of frame level. By this way, I can export
> > the correct TC in the following patch for movenc, dv file format. Please refer to:
> > libavformat/utils.c line 4154 add_coded_side_data() is used to copy all AVPacketSideData
> > side data to st level. If someone have better way to reuse the frame side
> > data, please give your advice.
> >
> 
> CBP is a stream-level property, it doesn't usually change.
> Timecodes are a per-frame property. There is no reason to store them
> globally in avctx.
> 
> There is a big difference between those two. TC should be a per-frame
> property, and if a muxer needs it, then the muxer should figure it out
> from packets, not from a global store.

The problem is most of muxer with TC support had used the timecode in stream-level, they're used
the first timecode for start always(so you can input the first timecode by
option also). So even I copy it to packet level, it doesn't used yet, that's why
I choose the global store.

For the TC need to sync with frame or packet strictly, so I think it's better
to add the the side copy(frame level to packet level) after encode_frame(), now
only v210enc need the packet level, so it'll copy in encode_frame().


> 
> - Hendrik
> _______________________________________________
> 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 Wang July 16, 2020, 12:38 a.m. UTC | #10 
On Wed, Jul 15, 2020 at 06:02:43PM +0100, Kieran Kunhya wrote:
> On Wed, 15 Jul 2020 at 16:52, <lance.lmwang@gmail.com> wrote:
> 
> > On Wed, Jul 15, 2020 at 04:16:38PM +0100, Kieran Kunhya wrote:
> > > If you look a few lines above, we already set the timecode side data.
> > >
> >
> > are you saying av_frame_new_side_data() above?  that's total different, in
> > fact,
> > I'm try to use the same way with ff_add_cpb_side_data(), it'll put TC in
> > the codec
> > level: avctx->nb_coded_side_data instead of frame level. By this way, I
> > can export
> > the correct TC in the following patch for movenc, dv file format. Please
> > refer to:
> > libavformat/utils.c line 4154 add_coded_side_data() is used to copy all
> > AVPacketSideData
> > side data to st level. If someone have better way to reuse the frame side
> > data, please give your advice.
> >
> 
> Sorry this makes no sense at all.
> What has decoding got to do with muxing?

No, it's used for copy the TC if you want to copy a video with frame-level TC to 
a stream level file. It looks duplicated for I haven't figured out where is
proper place to copy the frame-level side data to packet-level side data or global
level, if no good place, I'll do the copy in the encode_frame() like v210enc.

> 
> Kieran
> _______________________________________________
> 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 mbox series

Patch

diff --git a/libavcodec/h264_slice.c b/libavcodec/h264_slice.c
index c7b2764..db720de 100644
--- a/libavcodec/h264_slice.c
+++ b/libavcodec/h264_slice.c
@@ -1307,6 +1307,7 @@  static int h264_export_frame_props(H264Context *h)
     if (h->sei.picture_timing.timecode_cnt > 0) {
         uint32_t *tc_sd;
         char tcbuf[AV_TIMECODE_STR_SIZE];
+        uint32_t *s12m_sd;
 
         AVFrameSideData *tcside = av_frame_new_side_data(out,
                                                          AV_FRAME_DATA_S12M_TIMECODE,
@@ -1316,6 +1317,12 @@  static int h264_export_frame_props(H264Context *h)
 
         tc_sd = (uint32_t*)tcside->data;
         tc_sd[0] = h->sei.picture_timing.timecode_cnt;
+        if (h->avctx->export_side_data & AV_CODEC_EXPORT_DATA_S12M_TC) {
+            s12m_sd = ff_add_s12m_timecode_side_data(h->avctx);
+            if (!s12m_sd)
+                return AVERROR(ENOMEM);
+            s12m_sd[0] = tc_sd[0];
+        }
 
         for (int i = 0; i < tc_sd[0]; i++) {
             int drop = h->sei.picture_timing.timecode[i].dropframe;
@@ -1325,6 +1332,9 @@  static int h264_export_frame_props(H264Context *h)
             int   ff = h->sei.picture_timing.timecode[i].frame;
 
             tc_sd[i + 1] = av_timecode_get_smpte(h->avctx->framerate, drop, hh, mm, ss, ff);
+            if (h->avctx->export_side_data & AV_CODEC_EXPORT_DATA_S12M_TC) {
+                s12m_sd[i + 1] = tc_sd[i + 1];
+            }
             av_timecode_make_smpte_tc_string(tcbuf, tc_sd[i + 1], 0);
             av_dict_set(&out->metadata, "timecode", tcbuf, 0);
         }