diff mbox series

[FFmpeg-devel,v8,3/3] avdevice/decklink_dec: export timecode with s12m side data

Message ID 1594508198-13821-1-git-send-email-lance.lmwang@gmail.com
State Superseded
Headers show
Series None | expand

Commit Message

Limin Wang July 11, 2020, 10:56 p.m. UTC
From: Limin Wang <lance.lmwang@gmail.com>

Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
---
 libavdevice/decklink_dec.cpp | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

Comments

Marton Balint July 12, 2020, 12:34 a.m. UTC | #1
On Sun, 12 Jul 2020, lance.lmwang@gmail.com wrote:

> From: Limin Wang <lance.lmwang@gmail.com>
>
> Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
> ---
> libavdevice/decklink_dec.cpp | 14 ++++++++++++++
> 1 file changed, 14 insertions(+)
>
> diff --git a/libavdevice/decklink_dec.cpp b/libavdevice/decklink_dec.cpp
> index a499972..146f56f 100644
> --- a/libavdevice/decklink_dec.cpp
> +++ b/libavdevice/decklink_dec.cpp
> @@ -42,6 +42,7 @@ extern "C" {
> #include "libavutil/imgutils.h"
> #include "libavutil/intreadwrite.h"
> #include "libavutil/time.h"
> +#include "libavutil/timecode.h"
> #include "libavutil/mathematics.h"
> #include "libavutil/reverse.h"
> #include "avdevice.h"
> @@ -882,6 +883,19 @@ HRESULT decklink_input_callback::VideoInputFrameArrived(
>                         AVDictionary* metadata_dict = NULL;
>                         int metadata_len;
>                         uint8_t* packed_metadata;
> +                        AVTimecode tcr;
> +
> +                        if (av_timecode_init_from_string(&tcr, ctx->video_st->r_frame_rate, tc, ctx) >= 0) {
> +                            uint32_t tc_data = av_timecode_get_smpte_from_framenum(&tcr, 0);
> +                            int size = sizeof(uint32_t) * 4;
> +                            uint8_t *sd = av_packet_new_side_data(&pkt, AV_PKT_DATA_S12M_TIMECODE, size);
> +
> +                            if (sd) {
> +                                AV_WL32(sd, 1); // one TC ;
> +                                AV_WL32(sd + 4, tc_data); // TC;
> +                            }
> +                        }
> +
>                         if (av_dict_set(&metadata_dict, "timecode", tc, AV_DICT_DONT_STRDUP_VAL) >= 0) {
>                             packed_metadata = av_packet_pack_dictionary(metadata_dict, &metadata_len);
>                             av_dict_free(&metadata_dict);

LGTM, thanks.

Marton
Andreas Rheinhardt July 12, 2020, 1:07 a.m. UTC | #2
lance.lmwang@gmail.com:
> From: Limin Wang <lance.lmwang@gmail.com>
> 
> Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
> ---
>  libavdevice/decklink_dec.cpp | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/libavdevice/decklink_dec.cpp b/libavdevice/decklink_dec.cpp
> index a499972..146f56f 100644
> --- a/libavdevice/decklink_dec.cpp
> +++ b/libavdevice/decklink_dec.cpp
> @@ -42,6 +42,7 @@ extern "C" {
>  #include "libavutil/imgutils.h"
>  #include "libavutil/intreadwrite.h"
>  #include "libavutil/time.h"
> +#include "libavutil/timecode.h"
>  #include "libavutil/mathematics.h"
>  #include "libavutil/reverse.h"
>  #include "avdevice.h"
> @@ -882,6 +883,19 @@ HRESULT decklink_input_callback::VideoInputFrameArrived(
>                          AVDictionary* metadata_dict = NULL;
>                          int metadata_len;
>                          uint8_t* packed_metadata;
> +                        AVTimecode tcr;
> +
> +                        if (av_timecode_init_from_string(&tcr, ctx->video_st->r_frame_rate, tc, ctx) >= 0) {
> +                            uint32_t tc_data = av_timecode_get_smpte_from_framenum(&tcr, 0);
> +                            int size = sizeof(uint32_t) * 4;
> +                            uint8_t *sd = av_packet_new_side_data(&pkt, AV_PKT_DATA_S12M_TIMECODE, size);
> +
> +                            if (sd) {
> +                                AV_WL32(sd, 1); // one TC ;

Nit: why is there a space after TC?

> +                                AV_WL32(sd + 4, tc_data); // TC;

This contradicts the documentation of AV_PKT_DATA_S12M_TIMECODE, because
you are always using little endian here. But the documentation [1] does
not specify an endianness, it uses native endianness (in accordance with
what AV_FRAME_DATA_S12M_TIMECODE does). This very same patch also uses
native endianness in libavformat/dump.c.

I consider it a mistake to use native endianness for the frame side data
and an uint32_t in av_timecode_get_smpte (none of the fields cross a
byte boundary, so each entry could easily have been an uint8_t[4]), as
this will make it harder to test this side data via fate; but I also see
the advantage of using the same format and the same helper functions for
both. What do others think about this? After all, we can still change
the format for the packet side data.

- Andreas

[1]:
https://patchwork.ffmpeg.org/project/ffmpeg/patch/1593610798-21093-2-git-send-email-lance.lmwang@gmail.com/
(Is this even the latest version? I haven't paid close attention to this
patchset tbh.)
Limin Wang July 12, 2020, 9:45 a.m. UTC | #3
On Sun, Jul 12, 2020 at 03:07:18AM +0200, Andreas Rheinhardt wrote:
> lance.lmwang@gmail.com:
> > From: Limin Wang <lance.lmwang@gmail.com>
> > 
> > Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
> > ---
> >  libavdevice/decklink_dec.cpp | 14 ++++++++++++++
> >  1 file changed, 14 insertions(+)
> > 
> > diff --git a/libavdevice/decklink_dec.cpp b/libavdevice/decklink_dec.cpp
> > index a499972..146f56f 100644
> > --- a/libavdevice/decklink_dec.cpp
> > +++ b/libavdevice/decklink_dec.cpp
> > @@ -42,6 +42,7 @@ extern "C" {
> >  #include "libavutil/imgutils.h"
> >  #include "libavutil/intreadwrite.h"
> >  #include "libavutil/time.h"
> > +#include "libavutil/timecode.h"
> >  #include "libavutil/mathematics.h"
> >  #include "libavutil/reverse.h"
> >  #include "avdevice.h"
> > @@ -882,6 +883,19 @@ HRESULT decklink_input_callback::VideoInputFrameArrived(
> >                          AVDictionary* metadata_dict = NULL;
> >                          int metadata_len;
> >                          uint8_t* packed_metadata;
> > +                        AVTimecode tcr;
> > +
> > +                        if (av_timecode_init_from_string(&tcr, ctx->video_st->r_frame_rate, tc, ctx) >= 0) {
> > +                            uint32_t tc_data = av_timecode_get_smpte_from_framenum(&tcr, 0);
> > +                            int size = sizeof(uint32_t) * 4;
> > +                            uint8_t *sd = av_packet_new_side_data(&pkt, AV_PKT_DATA_S12M_TIMECODE, size);
> > +
> > +                            if (sd) {
> > +                                AV_WL32(sd, 1); // one TC ;
> 
> Nit: why is there a space after TC?

Will remove it, it's not intentional.

> 
> > +                                AV_WL32(sd + 4, tc_data); // TC;
> 
> This contradicts the documentation of AV_PKT_DATA_S12M_TIMECODE, because
> you are always using little endian here. But the documentation [1] does
> not specify an endianness, it uses native endianness (in accordance with
> what AV_FRAME_DATA_S12M_TIMECODE does). This very same patch also uses
> native endianness in libavformat/dump.c.

good catch, at first, I think it's little endian order, and Nicolas comment
uint32_t is native, so I remove the commens for the byte order. I'll change
to use AV_WB32() for native write. 

> 
> I consider it a mistake to use native endianness for the frame side data
> and an uint32_t in av_timecode_get_smpte (none of the fields cross a
> byte boundary, so each entry could easily have been an uint8_t[4]), as
> this will make it harder to test this side data via fate; but I also see
> the advantage of using the same format and the same helper functions for
> both. What do others think about this? After all, we can still change
> the format for the packet side data.

I didn't get the SMPTE S314M:2005 specs for checking, it's not free. I think
use native format is fine for local system, but if the side data is packed into
or read from specific format like DV, AV_WB32() and AV_RB32() had to be used.
For the fate testing, I think we can't test the side data binary only, if needed,
we can test the format which use the sidedata like dv.

I have add fate timecode testing for h264/hevc and haven't submit yet. But if
the frame rate > 30, I got one unexpected result after map SMPTE ST 12-1:2014
side data to HEVC timecode, the frame is 6bit only(2bit for tens of frame), 
so to framerate > 30, it'll be divided by 2 with same timecode, but HEVC timecode
frame number can use 9bit and expect the frame > 30.

> 
> - Andreas
> 
> [1]:
> https://patchwork.ffmpeg.org/project/ffmpeg/patch/1593610798-21093-2-git-send-email-lance.lmwang@gmail.com/
> (Is this even the latest version? I haven't paid close attention to this
> patchset tbh.)

Yes, in my local system, I change the version related for conflict.

> _______________________________________________
> 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".
Marton Balint July 12, 2020, 11:32 a.m. UTC | #4
On Sun, 12 Jul 2020, Andreas Rheinhardt wrote:

> lance.lmwang@gmail.com:
>> From: Limin Wang <lance.lmwang@gmail.com>
>> 
>> Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
>> ---
>>  libavdevice/decklink_dec.cpp | 14 ++++++++++++++
>>  1 file changed, 14 insertions(+)
>> 
>> diff --git a/libavdevice/decklink_dec.cpp b/libavdevice/decklink_dec.cpp
>> index a499972..146f56f 100644
>> --- a/libavdevice/decklink_dec.cpp
>> +++ b/libavdevice/decklink_dec.cpp
>> @@ -42,6 +42,7 @@ extern "C" {
>>  #include "libavutil/imgutils.h"
>>  #include "libavutil/intreadwrite.h"
>>  #include "libavutil/time.h"
>> +#include "libavutil/timecode.h"
>>  #include "libavutil/mathematics.h"
>>  #include "libavutil/reverse.h"
>>  #include "avdevice.h"
>> @@ -882,6 +883,19 @@ HRESULT decklink_input_callback::VideoInputFrameArrived(
>>                          AVDictionary* metadata_dict = NULL;
>>                          int metadata_len;
>>                          uint8_t* packed_metadata;
>> +                        AVTimecode tcr;
>> +
>> +                        if (av_timecode_init_from_string(&tcr, ctx->video_st->r_frame_rate, tc, ctx) >= 0) {
>> +                            uint32_t tc_data = av_timecode_get_smpte_from_framenum(&tcr, 0);
>> +                            int size = sizeof(uint32_t) * 4;
>> +                            uint8_t *sd = av_packet_new_side_data(&pkt, AV_PKT_DATA_S12M_TIMECODE, size);
>> +
>> +                            if (sd) {
>> +                                AV_WL32(sd, 1); // one TC ;
>
> Nit: why is there a space after TC?
>
>> +                                AV_WL32(sd + 4, tc_data); // TC;
>
> This contradicts the documentation of AV_PKT_DATA_S12M_TIMECODE, because
> you are always using little endian here. But the documentation [1] does
> not specify an endianness, it uses native endianness (in accordance with
> what AV_FRAME_DATA_S12M_TIMECODE does). This very same patch also uses
> native endianness in libavformat/dump.c.
>
> I consider it a mistake to use native endianness for the frame side data
> and an uint32_t in av_timecode_get_smpte (none of the fields cross a
> byte boundary, so each entry could easily have been an uint8_t[4]), as
> this will make it harder to test this side data via fate; but I also see
> the advantage of using the same format and the same helper functions for
> both. What do others think about this? After all, we can still change
> the format for the packet side data.

I think using possibly different endianness for the same type of frame 
side data and packet side data would be a very bad idea, a consistent API 
is a lot more important. Thanks for spotting this.

Regards,
Marton
Andreas Rheinhardt July 12, 2020, 4:03 p.m. UTC | #5
lance.lmwang@gmail.com:
> On Sun, Jul 12, 2020 at 03:07:18AM +0200, Andreas Rheinhardt wrote:
>> lance.lmwang@gmail.com:
>>> From: Limin Wang <lance.lmwang@gmail.com>
>>>
>>> Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
>>> ---
>>>  libavdevice/decklink_dec.cpp | 14 ++++++++++++++
>>>  1 file changed, 14 insertions(+)
>>>
>>> diff --git a/libavdevice/decklink_dec.cpp b/libavdevice/decklink_dec.cpp
>>> index a499972..146f56f 100644
>>> --- a/libavdevice/decklink_dec.cpp
>>> +++ b/libavdevice/decklink_dec.cpp
>>> @@ -42,6 +42,7 @@ extern "C" {
>>>  #include "libavutil/imgutils.h"
>>>  #include "libavutil/intreadwrite.h"
>>>  #include "libavutil/time.h"
>>> +#include "libavutil/timecode.h"
>>>  #include "libavutil/mathematics.h"
>>>  #include "libavutil/reverse.h"
>>>  #include "avdevice.h"
>>> @@ -882,6 +883,19 @@ HRESULT decklink_input_callback::VideoInputFrameArrived(
>>>                          AVDictionary* metadata_dict = NULL;
>>>                          int metadata_len;
>>>                          uint8_t* packed_metadata;
>>> +                        AVTimecode tcr;
>>> +
>>> +                        if (av_timecode_init_from_string(&tcr, ctx->video_st->r_frame_rate, tc, ctx) >= 0) {
>>> +                            uint32_t tc_data = av_timecode_get_smpte_from_framenum(&tcr, 0);
>>> +                            int size = sizeof(uint32_t) * 4;
>>> +                            uint8_t *sd = av_packet_new_side_data(&pkt, AV_PKT_DATA_S12M_TIMECODE, size);
>>> +
>>> +                            if (sd) {
>>> +                                AV_WL32(sd, 1); // one TC ;
>>
>> Nit: why is there a space after TC?
> 
> Will remove it, it's not intentional.
> 
>>
>>> +                                AV_WL32(sd + 4, tc_data); // TC;
>>
>> This contradicts the documentation of AV_PKT_DATA_S12M_TIMECODE, because
>> you are always using little endian here. But the documentation [1] does
>> not specify an endianness, it uses native endianness (in accordance with
>> what AV_FRAME_DATA_S12M_TIMECODE does). This very same patch also uses
>> native endianness in libavformat/dump.c.
> 
> good catch, at first, I think it's little endian order, and Nicolas comment
> uint32_t is native, so I remove the commens for the byte order. I'll change
> to use AV_WB32() for native write. 
> 

AV_WB uses big-endian. Instead you could simply use an uint32_t* to
write the numbers.

>>
>> I consider it a mistake to use native endianness for the frame side data
>> and an uint32_t in av_timecode_get_smpte (none of the fields cross a
>> byte boundary, so each entry could easily have been an uint8_t[4]), as
>> this will make it harder to test this side data via fate; but I also see
>> the advantage of using the same format and the same helper functions for
>> both. What do others think about this? After all, we can still change
>> the format for the packet side data.
> 
> I didn't get the SMPTE S314M:2005 specs for checking, it's not free. I think
> use native format is fine for local system, but if the side data is packed into
> or read from specific format like DV, AV_WB32() and AV_RB32() had to be used.
> For the fate testing, I think we can't test the side data binary only, if needed,
> we can test the format which use the sidedata like dv.

The framecrc muxer calculates a checksum of every side data. There is
already a special way of calculating the side-data for palettes in
big-endian and it seems that we can simply reuse this (as both are based
on uint32_t in native format). So it seems it won't be much of a problem.

- Andreas
Limin Wang July 13, 2020, 1:43 a.m. UTC | #6
On Sun, Jul 12, 2020 at 06:03:04PM +0200, Andreas Rheinhardt wrote:
> lance.lmwang@gmail.com:
> > On Sun, Jul 12, 2020 at 03:07:18AM +0200, Andreas Rheinhardt wrote:
> >> lance.lmwang@gmail.com:
> >>> From: Limin Wang <lance.lmwang@gmail.com>
> >>>
> >>> Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
> >>> ---
> >>>  libavdevice/decklink_dec.cpp | 14 ++++++++++++++
> >>>  1 file changed, 14 insertions(+)
> >>>
> >>> diff --git a/libavdevice/decklink_dec.cpp b/libavdevice/decklink_dec.cpp
> >>> index a499972..146f56f 100644
> >>> --- a/libavdevice/decklink_dec.cpp
> >>> +++ b/libavdevice/decklink_dec.cpp
> >>> @@ -42,6 +42,7 @@ extern "C" {
> >>>  #include "libavutil/imgutils.h"
> >>>  #include "libavutil/intreadwrite.h"
> >>>  #include "libavutil/time.h"
> >>> +#include "libavutil/timecode.h"
> >>>  #include "libavutil/mathematics.h"
> >>>  #include "libavutil/reverse.h"
> >>>  #include "avdevice.h"
> >>> @@ -882,6 +883,19 @@ HRESULT decklink_input_callback::VideoInputFrameArrived(
> >>>                          AVDictionary* metadata_dict = NULL;
> >>>                          int metadata_len;
> >>>                          uint8_t* packed_metadata;
> >>> +                        AVTimecode tcr;
> >>> +
> >>> +                        if (av_timecode_init_from_string(&tcr, ctx->video_st->r_frame_rate, tc, ctx) >= 0) {
> >>> +                            uint32_t tc_data = av_timecode_get_smpte_from_framenum(&tcr, 0);
> >>> +                            int size = sizeof(uint32_t) * 4;
> >>> +                            uint8_t *sd = av_packet_new_side_data(&pkt, AV_PKT_DATA_S12M_TIMECODE, size);
> >>> +
> >>> +                            if (sd) {
> >>> +                                AV_WL32(sd, 1); // one TC ;
> >>
> >> Nit: why is there a space after TC?
> > 
> > Will remove it, it's not intentional.
> > 
> >>
> >>> +                                AV_WL32(sd + 4, tc_data); // TC;
> >>
> >> This contradicts the documentation of AV_PKT_DATA_S12M_TIMECODE, because
> >> you are always using little endian here. But the documentation [1] does
> >> not specify an endianness, it uses native endianness (in accordance with
> >> what AV_FRAME_DATA_S12M_TIMECODE does). This very same patch also uses
> >> native endianness in libavformat/dump.c.
> > 
> > good catch, at first, I think it's little endian order, and Nicolas comment
> > uint32_t is native, so I remove the commens for the byte order. I'll change
> > to use AV_WB32() for native write. 
> > 
> 
> AV_WB uses big-endian. Instead you could simply use an uint32_t* to
> write the numbers.

OK, I'll change it.

> 
> >>
> >> I consider it a mistake to use native endianness for the frame side data
> >> and an uint32_t in av_timecode_get_smpte (none of the fields cross a
> >> byte boundary, so each entry could easily have been an uint8_t[4]), as
> >> this will make it harder to test this side data via fate; but I also see
> >> the advantage of using the same format and the same helper functions for
> >> both. What do others think about this? After all, we can still change
> >> the format for the packet side data.
> > 
> > I didn't get the SMPTE S314M:2005 specs for checking, it's not free. I think
> > use native format is fine for local system, but if the side data is packed into
> > or read from specific format like DV, AV_WB32() and AV_RB32() had to be used.
> > For the fate testing, I think we can't test the side data binary only, if needed,
> > we can test the format which use the sidedata like dv.
> 
> The framecrc muxer calculates a checksum of every side data. There is
> already a special way of calculating the side-data for palettes in
> big-endian and it seems that we can simply reuse this (as both are based
> on uint32_t in native format). So it seems it won't be much of a problem.
> 
> - Andreas
> _______________________________________________
> 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".
Devin Heitmueller July 13, 2020, 2:24 p.m. UTC | #7
On Sun, Jul 12, 2020 at 6:16 AM <lance.lmwang@gmail.com> wrote:
> I have add fate timecode testing for h264/hevc and haven't submit yet. But if
> the frame rate > 30, I got one unexpected result after map SMPTE ST 12-1:2014
> side data to HEVC timecode, the frame is 6bit only(2bit for tens of frame),
> so to framerate > 30, it'll be divided by 2 with same timecode, but HEVC timecode
> frame number can use 9bit and expect the frame > 30.

You should still divide it by 2, even though HEVC supports more bits
for the frame number.  This is to be consistent with SMPTE 12-1
timecode, and downstream muxers/outputs aren't going to know whether
the upstream decoder was H.264 or HEVC.  On the encode side, you can
multiple by 2 and use the field bit to determine whether to then add
1.  See SMPTE ST 12-1:2014 Sec 12.2.

There's an argument that we should add support for higher framerates
to support SMPTE 12-3 (which supports fps > 60), but that should be a
different side data field.  Right now the most important thing is that
the behavior be consistent across decoders.

Devin
Limin Wang July 13, 2020, 10:12 p.m. UTC | #8
On Mon, Jul 13, 2020 at 10:24:11AM -0400, Devin Heitmueller wrote:
> On Sun, Jul 12, 2020 at 6:16 AM <lance.lmwang@gmail.com> wrote:
> > I have add fate timecode testing for h264/hevc and haven't submit yet. But if
> > the frame rate > 30, I got one unexpected result after map SMPTE ST 12-1:2014
> > side data to HEVC timecode, the frame is 6bit only(2bit for tens of frame),
> > so to framerate > 30, it'll be divided by 2 with same timecode, but HEVC timecode
> > frame number can use 9bit and expect the frame > 30.
> 
> You should still divide it by 2, even though HEVC supports more bits
> for the frame number.  This is to be consistent with SMPTE 12-1
> timecode, and downstream muxers/outputs aren't going to know whether
> the upstream decoder was H.264 or HEVC.  On the encode side, you can
> multiple by 2 and use the field bit to determine whether to then add
> 1.  See SMPTE ST 12-1:2014 Sec 12.2.

Thanks for the comments, it's helpful, I'll try to fix it on encoder side.

> 
> There's an argument that we should add support for higher framerates
> to support SMPTE 12-3 (which supports fps > 60), but that should be a
> different side data field.  Right now the most important thing is that
> the behavior be consistent across decoders.
> 
> Devin
> 
> -- 
> Devin Heitmueller, Senior Software Engineer
> LTN Global Communications
> o: +1 (301) 363-1001
> w: https://ltnglobal.com  e: devin.heitmueller@ltnglobal.com
> _______________________________________________
> 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/libavdevice/decklink_dec.cpp b/libavdevice/decklink_dec.cpp
index a499972..146f56f 100644
--- a/libavdevice/decklink_dec.cpp
+++ b/libavdevice/decklink_dec.cpp
@@ -42,6 +42,7 @@  extern "C" {
 #include "libavutil/imgutils.h"
 #include "libavutil/intreadwrite.h"
 #include "libavutil/time.h"
+#include "libavutil/timecode.h"
 #include "libavutil/mathematics.h"
 #include "libavutil/reverse.h"
 #include "avdevice.h"
@@ -882,6 +883,19 @@  HRESULT decklink_input_callback::VideoInputFrameArrived(
                         AVDictionary* metadata_dict = NULL;
                         int metadata_len;
                         uint8_t* packed_metadata;
+                        AVTimecode tcr;
+
+                        if (av_timecode_init_from_string(&tcr, ctx->video_st->r_frame_rate, tc, ctx) >= 0) {
+                            uint32_t tc_data = av_timecode_get_smpte_from_framenum(&tcr, 0);
+                            int size = sizeof(uint32_t) * 4;
+                            uint8_t *sd = av_packet_new_side_data(&pkt, AV_PKT_DATA_S12M_TIMECODE, size);
+
+                            if (sd) {
+                                AV_WL32(sd, 1); // one TC ;
+                                AV_WL32(sd + 4, tc_data); // TC;
+                            }
+                        }
+
                         if (av_dict_set(&metadata_dict, "timecode", tc, AV_DICT_DONT_STRDUP_VAL) >= 0) {
                             packed_metadata = av_packet_pack_dictionary(metadata_dict, &metadata_len);
                             av_dict_free(&metadata_dict);