diff mbox series

[FFmpeg-devel,2/3] avutil/timecode: fix av_timecode_get_smpte_from_framenum with 50/60 fps

Message ID 20200720210439.1809-2-cus@passwd.hu
State New
Headers show
Series [FFmpeg-devel,1/3] avformat/dvenc: do not set binary group and biphase polarity flags
Related show

Checks

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

Commit Message

Marton Balint July 20, 2020, 9:04 p.m. UTC
50/60 fps timecode is using the field bit (which is the same as the phase
correction bit) to signal the least significant bit of a 50/60 fps timecode.
See SMPTE ST 12-1:2014 section 12.1.

Let's add support for this by using the recently added av_timecode_get_smpte
function which handles this properly.

This change affects DV and MXF encoder, MXF has no fate for 50/60fps content,
DV does, therefore the changes. It also affects decklink indev.

MediaInfo (a recent version) confirms that half-frame timecode must be
inserted. (although MediaInfo does not seem to check the field flag).
MXFInspect confirms valid timecode insertion to the System Item of MXF files.

Signed-off-by: Marton Balint <cus@passwd.hu>
---
 libavutil/timecode.c               | 15 +--------------
 libavutil/timecode.h               |  7 +++----
 tests/ref/vsynth/vsynth1-dv-hd     |  2 +-
 tests/ref/vsynth/vsynth2-dv-hd     |  2 +-
 tests/ref/vsynth/vsynth3-dv-hd     |  2 +-
 tests/ref/vsynth/vsynth_lena-dv-hd |  2 +-
 6 files changed, 8 insertions(+), 22 deletions(-)

Comments

Limin Wang July 20, 2020, 11:49 p.m. UTC | #1
On Mon, Jul 20, 2020 at 11:04:38PM +0200, Marton Balint wrote:
> 50/60 fps timecode is using the field bit (which is the same as the phase
> correction bit) to signal the least significant bit of a 50/60 fps timecode.
> See SMPTE ST 12-1:2014 section 12.1.
> 
> Let's add support for this by using the recently added av_timecode_get_smpte
> function which handles this properly.
> 
> This change affects DV and MXF encoder, MXF has no fate for 50/60fps content,
> DV does, therefore the changes. It also affects decklink indev.
> 
> MediaInfo (a recent version) confirms that half-frame timecode must be
> inserted. (although MediaInfo does not seem to check the field flag).
> MXFInspect confirms valid timecode insertion to the System Item of MXF files.
> 
> Signed-off-by: Marton Balint <cus@passwd.hu>
> ---
>  libavutil/timecode.c               | 15 +--------------
>  libavutil/timecode.h               |  7 +++----
>  tests/ref/vsynth/vsynth1-dv-hd     |  2 +-
>  tests/ref/vsynth/vsynth2-dv-hd     |  2 +-
>  tests/ref/vsynth/vsynth3-dv-hd     |  2 +-
>  tests/ref/vsynth/vsynth_lena-dv-hd |  2 +-
>  6 files changed, 8 insertions(+), 22 deletions(-)
> 
> diff --git a/libavutil/timecode.c b/libavutil/timecode.c
> index cca53d73c4..cb916970ef 100644
> --- a/libavutil/timecode.c
> +++ b/libavutil/timecode.c
> @@ -65,20 +65,7 @@ uint32_t av_timecode_get_smpte_from_framenum(const AVTimecode *tc, int framenum)
>      ss = framenum / fps      % 60;
>      mm = framenum / (fps*60) % 60;
>      hh = framenum / (fps*3600) % 24;
> -    return 0         << 31 | // color frame flag (0: unsync mode, 1: sync mode)
> -           drop      << 30 | // drop  frame flag (0: non drop,    1: drop)
> -           (ff / 10) << 28 | // tens  of frames
> -           (ff % 10) << 24 | // units of frames
> -           0         << 23 | // PC (NTSC) or BGF0 (PAL)
> -           (ss / 10) << 20 | // tens  of seconds
> -           (ss % 10) << 16 | // units of seconds
> -           0         << 15 | // BGF0 (NTSC) or BGF2 (PAL)
> -           (mm / 10) << 12 | // tens  of minutes
> -           (mm % 10) <<  8 | // units of minutes
> -           0         <<  7 | // BGF2 (NTSC) or PC (PAL)
> -           0         <<  6 | // BGF1
> -           (hh / 10) <<  4 | // tens  of hours
> -           (hh % 10);        // units of hours
> +    return av_timecode_get_smpte(tc->rate, drop, hh, mm, ss, ff);

av_timecode_get_smpte() is used by the h264/hevc, and the specs reserved more bits for ff, so in case it's
> 30fps, so av_timecode_get_smpte() will divide ff by 2 to get frame pair. But for SMPTE ST 12 specs, the
ff reserved 2 bits for tens, so for > 30fps, it's supported by frame pair and ff
is divide by 2 already by the spec, so you'll get unexpected results if divide
by 2 again(four frame pair if test) 


>  }
>  
>  uint32_t av_timecode_get_smpte(AVRational rate, int drop, int hh, int mm, int ss, int ff)
> diff --git a/libavutil/timecode.h b/libavutil/timecode.h
> index 5801330921..e54b116e93 100644
> --- a/libavutil/timecode.h
> +++ b/libavutil/timecode.h
> @@ -66,11 +66,11 @@ int av_timecode_adjust_ntsc_framenum2(int framenum, int fps);
>   * the format description as follows:
>   * bits 0-5:   hours, in BCD(6bits)
>   * bits 6:     BGF1
> - * bits 7:     BGF2 (NTSC) or PC (PAL)
> + * bits 7:     BGF2 (NTSC) or FIELD (PAL)
>   * bits 8-14:  minutes, in BCD(7bits)
>   * bits 15:    BGF0 (NTSC) or BGF2 (PAL)
>   * bits 16-22: seconds, in BCD(7bits)
> - * bits 23:    PC (NTSC) or BGF0 (PAL)
> + * bits 23:    FIELD (NTSC) or BGF0 (PAL)
>   * bits 24-29: frames, in BCD(6bits)
>   * bits 30:    drop  frame flag (0: non drop,    1: drop)
>   * bits 31:    color frame flag (0: unsync mode, 1: sync mode)
> @@ -78,8 +78,7 @@ int av_timecode_adjust_ntsc_framenum2(int framenum, int fps);
>   * @note Frame number adjustment is automatically done in case of drop timecode,
>   *       you do NOT have to call av_timecode_adjust_ntsc_framenum2().
>   * @note The frame number is relative to tc->start.
> - * @note Color frame (CF), binary group flags (BGF) and biphase mark polarity
> - *       correction (PC) bits are set to zero.
> + * @note Color frame (CF) and binary group flags (BGF) bits are set to zero.
>   */
>  uint32_t av_timecode_get_smpte_from_framenum(const AVTimecode *tc, int framenum);
>  
> diff --git a/tests/ref/vsynth/vsynth1-dv-hd b/tests/ref/vsynth/vsynth1-dv-hd
> index e22f28c704..a93ce50ec0 100644
> --- a/tests/ref/vsynth/vsynth1-dv-hd
> +++ b/tests/ref/vsynth/vsynth1-dv-hd
> @@ -1,4 +1,4 @@
> -77f146c73a24495495c649f1e0d26fee *tests/data/fate/vsynth1-dv-hd.dv
> +ce9f887af2391edbfcb21675962db54c *tests/data/fate/vsynth1-dv-hd.dv
>  14400000 tests/data/fate/vsynth1-dv-hd.dv
>  34b78cf725346c7f819c9d6209b8299a *tests/data/fate/vsynth1-dv-hd.out.rawvideo
>  stddev:    4.30 PSNR: 35.45 MAXDIFF:   74 bytes:  7603200/  7603200
> diff --git a/tests/ref/vsynth/vsynth2-dv-hd b/tests/ref/vsynth/vsynth2-dv-hd
> index 5c4ed86fe7..9552c31948 100644
> --- a/tests/ref/vsynth/vsynth2-dv-hd
> +++ b/tests/ref/vsynth/vsynth2-dv-hd
> @@ -1,4 +1,4 @@
> -168f80951daf5eeefacac33fd06db87d *tests/data/fate/vsynth2-dv-hd.dv
> +c152c7c3f9ef8f404b4bb7a388b947be *tests/data/fate/vsynth2-dv-hd.dv
>  14400000 tests/data/fate/vsynth2-dv-hd.dv
>  15dbe911532aca81c67bdd2846419027 *tests/data/fate/vsynth2-dv-hd.out.rawvideo
>  stddev:    1.75 PSNR: 43.26 MAXDIFF:   34 bytes:  7603200/  7603200
> diff --git a/tests/ref/vsynth/vsynth3-dv-hd b/tests/ref/vsynth/vsynth3-dv-hd
> index ed94d08155..d0420d6f07 100644
> --- a/tests/ref/vsynth/vsynth3-dv-hd
> +++ b/tests/ref/vsynth/vsynth3-dv-hd
> @@ -1,4 +1,4 @@
> -1211b843d8dde81c583023e15beff35a *tests/data/fate/vsynth3-dv-hd.dv
> +973b0856d1d94793bd6e8951284e642d *tests/data/fate/vsynth3-dv-hd.dv
>  14400000 tests/data/fate/vsynth3-dv-hd.dv
>  a038ad7c3c09f776304ef7accdea9c74 *tests/data/fate/vsynth3-dv-hd.out.rawvideo
>  stddev:    0.00 PSNR:999.99 MAXDIFF:    0 bytes:    86700/    86700
> diff --git a/tests/ref/vsynth/vsynth_lena-dv-hd b/tests/ref/vsynth/vsynth_lena-dv-hd
> index 6764f03c35..8b87da40ba 100644
> --- a/tests/ref/vsynth/vsynth_lena-dv-hd
> +++ b/tests/ref/vsynth/vsynth_lena-dv-hd
> @@ -1,4 +1,4 @@
> -fa8df53531f9e58f90bc9a33e5c78ca8 *tests/data/fate/vsynth_lena-dv-hd.dv
> +30652b3740a32fa8bcee3b06a66b1dd2 *tests/data/fate/vsynth_lena-dv-hd.dv
>  14400000 tests/data/fate/vsynth_lena-dv-hd.dv
>  4db4175c80ea1f16b7ec303611b8873a *tests/data/fate/vsynth_lena-dv-hd.out.rawvideo
>  stddev:    1.49 PSNR: 44.66 MAXDIFF:   27 bytes:  7603200/  7603200
> -- 
> 2.26.2
> 
> _______________________________________________
> 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 21, 2020, 9:07 a.m. UTC | #2
On Tue, 21 Jul 2020, lance.lmwang@gmail.com wrote:

> On Mon, Jul 20, 2020 at 11:04:38PM +0200, Marton Balint wrote:
>> 50/60 fps timecode is using the field bit (which is the same as the phase
>> correction bit) to signal the least significant bit of a 50/60 fps timecode.
>> See SMPTE ST 12-1:2014 section 12.1.
>> 
>> Let's add support for this by using the recently added av_timecode_get_smpte
>> function which handles this properly.
>> 
>> This change affects DV and MXF encoder, MXF has no fate for 50/60fps content,
>> DV does, therefore the changes. It also affects decklink indev.
>> 
>> MediaInfo (a recent version) confirms that half-frame timecode must be
>> inserted. (although MediaInfo does not seem to check the field flag).
>> MXFInspect confirms valid timecode insertion to the System Item of MXF files.
>> 
>> Signed-off-by: Marton Balint <cus@passwd.hu>
>> ---
>>  libavutil/timecode.c               | 15 +--------------
>>  libavutil/timecode.h               |  7 +++----
>>  tests/ref/vsynth/vsynth1-dv-hd     |  2 +-
>>  tests/ref/vsynth/vsynth2-dv-hd     |  2 +-
>>  tests/ref/vsynth/vsynth3-dv-hd     |  2 +-
>>  tests/ref/vsynth/vsynth_lena-dv-hd |  2 +-
>>  6 files changed, 8 insertions(+), 22 deletions(-)
>> 
>> diff --git a/libavutil/timecode.c b/libavutil/timecode.c
>> index cca53d73c4..cb916970ef 100644
>> --- a/libavutil/timecode.c
>> +++ b/libavutil/timecode.c
>> @@ -65,20 +65,7 @@ uint32_t av_timecode_get_smpte_from_framenum(const AVTimecode *tc, int framenum)
>>      ss = framenum / fps      % 60;
>>      mm = framenum / (fps*60) % 60;
>>      hh = framenum / (fps*3600) % 24;
>> -    return 0         << 31 | // color frame flag (0: unsync mode, 1: sync mode)
>> -           drop      << 30 | // drop  frame flag (0: non drop,    1: drop)
>> -           (ff / 10) << 28 | // tens  of frames
>> -           (ff % 10) << 24 | // units of frames
>> -           0         << 23 | // PC (NTSC) or BGF0 (PAL)
>> -           (ss / 10) << 20 | // tens  of seconds
>> -           (ss % 10) << 16 | // units of seconds
>> -           0         << 15 | // BGF0 (NTSC) or BGF2 (PAL)
>> -           (mm / 10) << 12 | // tens  of minutes
>> -           (mm % 10) <<  8 | // units of minutes
>> -           0         <<  7 | // BGF2 (NTSC) or PC (PAL)
>> -           0         <<  6 | // BGF1
>> -           (hh / 10) <<  4 | // tens  of hours
>> -           (hh % 10);        // units of hours
>> +    return av_timecode_get_smpte(tc->rate, drop, hh, mm, ss, ff);
>
> av_timecode_get_smpte() is used by the h264/hevc, and the specs reserved more bits for ff, so in case it's
>> 30fps, so av_timecode_get_smpte() will divide ff by 2 to get frame pair. But for SMPTE ST 12 specs, the
> ff reserved 2 bits for tens, so for > 30fps, it's supported by frame pair and ff
> is divide by 2 already by the spec, so you'll get unexpected results if divide
> by 2 again(four frame pair if test)

I don't understand what you want to say. 
av_timecode_get_smpte_from_framenum is a function for which the number of 
frames (as a single integer) is passed. Clearly the number of frames as a 
single integer is not divided by anything, so there cannot be double 
division when av_timecode_get_smpte_from_framenum() calls 
av_timecode_get_smpte().

Regards,
Marton

>
>
>>  }
>>
>>  uint32_t av_timecode_get_smpte(AVRational rate, int drop, int hh, int mm, int ss, int ff)
>> diff --git a/libavutil/timecode.h b/libavutil/timecode.h
>> index 5801330921..e54b116e93 100644
>> --- a/libavutil/timecode.h
>> +++ b/libavutil/timecode.h
>> @@ -66,11 +66,11 @@ int av_timecode_adjust_ntsc_framenum2(int framenum, int fps);
>>   * the format description as follows:
>>   * bits 0-5:   hours, in BCD(6bits)
>>   * bits 6:     BGF1
>> - * bits 7:     BGF2 (NTSC) or PC (PAL)
>> + * bits 7:     BGF2 (NTSC) or FIELD (PAL)
>>   * bits 8-14:  minutes, in BCD(7bits)
>>   * bits 15:    BGF0 (NTSC) or BGF2 (PAL)
>>   * bits 16-22: seconds, in BCD(7bits)
>> - * bits 23:    PC (NTSC) or BGF0 (PAL)
>> + * bits 23:    FIELD (NTSC) or BGF0 (PAL)
>>   * bits 24-29: frames, in BCD(6bits)
>>   * bits 30:    drop  frame flag (0: non drop,    1: drop)
>>   * bits 31:    color frame flag (0: unsync mode, 1: sync mode)
>> @@ -78,8 +78,7 @@ int av_timecode_adjust_ntsc_framenum2(int framenum, int fps);
>>   * @note Frame number adjustment is automatically done in case of drop timecode,
>>   *       you do NOT have to call av_timecode_adjust_ntsc_framenum2().
>>   * @note The frame number is relative to tc->start.
>> - * @note Color frame (CF), binary group flags (BGF) and biphase mark polarity
>> - *       correction (PC) bits are set to zero.
>> + * @note Color frame (CF) and binary group flags (BGF) bits are set to zero.
>>   */
>>  uint32_t av_timecode_get_smpte_from_framenum(const AVTimecode *tc, int framenum);
>> 
>> diff --git a/tests/ref/vsynth/vsynth1-dv-hd b/tests/ref/vsynth/vsynth1-dv-hd
>> index e22f28c704..a93ce50ec0 100644
>> --- a/tests/ref/vsynth/vsynth1-dv-hd
>> +++ b/tests/ref/vsynth/vsynth1-dv-hd
>> @@ -1,4 +1,4 @@
>> -77f146c73a24495495c649f1e0d26fee *tests/data/fate/vsynth1-dv-hd.dv
>> +ce9f887af2391edbfcb21675962db54c *tests/data/fate/vsynth1-dv-hd.dv
>>  14400000 tests/data/fate/vsynth1-dv-hd.dv
>>  34b78cf725346c7f819c9d6209b8299a *tests/data/fate/vsynth1-dv-hd.out.rawvideo
>>  stddev:    4.30 PSNR: 35.45 MAXDIFF:   74 bytes:  7603200/  7603200
>> diff --git a/tests/ref/vsynth/vsynth2-dv-hd b/tests/ref/vsynth/vsynth2-dv-hd
>> index 5c4ed86fe7..9552c31948 100644
>> --- a/tests/ref/vsynth/vsynth2-dv-hd
>> +++ b/tests/ref/vsynth/vsynth2-dv-hd
>> @@ -1,4 +1,4 @@
>> -168f80951daf5eeefacac33fd06db87d *tests/data/fate/vsynth2-dv-hd.dv
>> +c152c7c3f9ef8f404b4bb7a388b947be *tests/data/fate/vsynth2-dv-hd.dv
>>  14400000 tests/data/fate/vsynth2-dv-hd.dv
>>  15dbe911532aca81c67bdd2846419027 *tests/data/fate/vsynth2-dv-hd.out.rawvideo
>>  stddev:    1.75 PSNR: 43.26 MAXDIFF:   34 bytes:  7603200/  7603200
>> diff --git a/tests/ref/vsynth/vsynth3-dv-hd b/tests/ref/vsynth/vsynth3-dv-hd
>> index ed94d08155..d0420d6f07 100644
>> --- a/tests/ref/vsynth/vsynth3-dv-hd
>> +++ b/tests/ref/vsynth/vsynth3-dv-hd
>> @@ -1,4 +1,4 @@
>> -1211b843d8dde81c583023e15beff35a *tests/data/fate/vsynth3-dv-hd.dv
>> +973b0856d1d94793bd6e8951284e642d *tests/data/fate/vsynth3-dv-hd.dv
>>  14400000 tests/data/fate/vsynth3-dv-hd.dv
>>  a038ad7c3c09f776304ef7accdea9c74 *tests/data/fate/vsynth3-dv-hd.out.rawvideo
>>  stddev:    0.00 PSNR:999.99 MAXDIFF:    0 bytes:    86700/    86700
>> diff --git a/tests/ref/vsynth/vsynth_lena-dv-hd b/tests/ref/vsynth/vsynth_lena-dv-hd
>> index 6764f03c35..8b87da40ba 100644
>> --- a/tests/ref/vsynth/vsynth_lena-dv-hd
>> +++ b/tests/ref/vsynth/vsynth_lena-dv-hd
>> @@ -1,4 +1,4 @@
>> -fa8df53531f9e58f90bc9a33e5c78ca8 *tests/data/fate/vsynth_lena-dv-hd.dv
>> +30652b3740a32fa8bcee3b06a66b1dd2 *tests/data/fate/vsynth_lena-dv-hd.dv
>>  14400000 tests/data/fate/vsynth_lena-dv-hd.dv
>>  4db4175c80ea1f16b7ec303611b8873a *tests/data/fate/vsynth_lena-dv-hd.out.rawvideo
>>  stddev:    1.49 PSNR: 44.66 MAXDIFF:   27 bytes:  7603200/  7603200
>> -- 
>> 2.26.2
>> 
>> _______________________________________________
>> 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".
Limin Wang July 21, 2020, 10:21 a.m. UTC | #3
On Tue, Jul 21, 2020 at 11:07:29AM +0200, Marton Balint wrote:
> 
> 
> On Tue, 21 Jul 2020, lance.lmwang@gmail.com wrote:
> 
> > On Mon, Jul 20, 2020 at 11:04:38PM +0200, Marton Balint wrote:
> > > 50/60 fps timecode is using the field bit (which is the same as the phase
> > > correction bit) to signal the least significant bit of a 50/60 fps timecode.
> > > See SMPTE ST 12-1:2014 section 12.1.
> > > 
> > > Let's add support for this by using the recently added av_timecode_get_smpte
> > > function which handles this properly.
> > > 
> > > This change affects DV and MXF encoder, MXF has no fate for 50/60fps content,
> > > DV does, therefore the changes. It also affects decklink indev.
> > > 
> > > MediaInfo (a recent version) confirms that half-frame timecode must be
> > > inserted. (although MediaInfo does not seem to check the field flag).
> > > MXFInspect confirms valid timecode insertion to the System Item of MXF files.
> > > 
> > > Signed-off-by: Marton Balint <cus@passwd.hu>
> > > ---
> > >  libavutil/timecode.c               | 15 +--------------
> > >  libavutil/timecode.h               |  7 +++----
> > >  tests/ref/vsynth/vsynth1-dv-hd     |  2 +-
> > >  tests/ref/vsynth/vsynth2-dv-hd     |  2 +-
> > >  tests/ref/vsynth/vsynth3-dv-hd     |  2 +-
> > >  tests/ref/vsynth/vsynth_lena-dv-hd |  2 +-
> > >  6 files changed, 8 insertions(+), 22 deletions(-)
> > > 
> > > diff --git a/libavutil/timecode.c b/libavutil/timecode.c
> > > index cca53d73c4..cb916970ef 100644
> > > --- a/libavutil/timecode.c
> > > +++ b/libavutil/timecode.c
> > > @@ -65,20 +65,7 @@ uint32_t av_timecode_get_smpte_from_framenum(const AVTimecode *tc, int framenum)
> > >      ss = framenum / fps      % 60;
> > >      mm = framenum / (fps*60) % 60;
> > >      hh = framenum / (fps*3600) % 24;
> > > -    return 0         << 31 | // color frame flag (0: unsync mode, 1: sync mode)
> > > -           drop      << 30 | // drop  frame flag (0: non drop,    1: drop)
> > > -           (ff / 10) << 28 | // tens  of frames
> > > -           (ff % 10) << 24 | // units of frames
> > > -           0         << 23 | // PC (NTSC) or BGF0 (PAL)
> > > -           (ss / 10) << 20 | // tens  of seconds
> > > -           (ss % 10) << 16 | // units of seconds
> > > -           0         << 15 | // BGF0 (NTSC) or BGF2 (PAL)
> > > -           (mm / 10) << 12 | // tens  of minutes
> > > -           (mm % 10) <<  8 | // units of minutes
> > > -           0         <<  7 | // BGF2 (NTSC) or PC (PAL)
> > > -           0         <<  6 | // BGF1
> > > -           (hh / 10) <<  4 | // tens  of hours
> > > -           (hh % 10);        // units of hours
> > > +    return av_timecode_get_smpte(tc->rate, drop, hh, mm, ss, ff);
> > 
> > av_timecode_get_smpte() is used by the h264/hevc, and the specs reserved more bits for ff, so in case it's
> > > 30fps, so av_timecode_get_smpte() will divide ff by 2 to get frame pair. But for SMPTE ST 12 specs, the
> > ff reserved 2 bits for tens, so for > 30fps, it's supported by frame pair and ff
> > is divide by 2 already by the spec, so you'll get unexpected results if divide
> > by 2 again(four frame pair if test)
> 
> I don't understand what you want to say. av_timecode_get_smpte_from_framenum
> is a function for which the number of frames (as a single integer) is
> passed. Clearly the number of frames as a single integer is not divided by
> anything, so there cannot be double division when
> av_timecode_get_smpte_from_framenum() calls av_timecode_get_smpte().

For example, when I'm print out the tc from timecode->GetString(&decklink_tc) in
decklink_dec.cpp for fps = 50, the tc will be:

00:00:00:00
00:00:00:00  -> repeat TC for frame pair
00:00:00:01
00:00:00:01  -> repeat
....

00:00:00:24
00:00:00:24  -> repeat, 50 frame now

00:00:00:00
00:00:00:00  -> repeat

If the av_timecode_get_smpte_from_framenum(), the ff will be divided by 2 after change, you'll get below:

00:00:00:00
00:00:00:00
00:00:00:00
00:00:00:00
00:00:00:01
00:00:00:01
00:00:00:01
00:00:00:01
....

00:00:00:12
00:00:00:12
00:00:00:12
00:00:00:12  -> 50 frame now, repeat 4 times

00:00:00:00
00:00:00:00
00:00:00:00
00:00:00:00

I'm not sure whether it's clear to explain my meaning.


> 
> Regards,
> Marton
> 
> > 
> > 
> > >  }
> > > 
> > >  uint32_t av_timecode_get_smpte(AVRational rate, int drop, int hh, int mm, int ss, int ff)
> > > diff --git a/libavutil/timecode.h b/libavutil/timecode.h
> > > index 5801330921..e54b116e93 100644
> > > --- a/libavutil/timecode.h
> > > +++ b/libavutil/timecode.h
> > > @@ -66,11 +66,11 @@ int av_timecode_adjust_ntsc_framenum2(int framenum, int fps);
> > >   * the format description as follows:
> > >   * bits 0-5:   hours, in BCD(6bits)
> > >   * bits 6:     BGF1
> > > - * bits 7:     BGF2 (NTSC) or PC (PAL)
> > > + * bits 7:     BGF2 (NTSC) or FIELD (PAL)
> > >   * bits 8-14:  minutes, in BCD(7bits)
> > >   * bits 15:    BGF0 (NTSC) or BGF2 (PAL)
> > >   * bits 16-22: seconds, in BCD(7bits)
> > > - * bits 23:    PC (NTSC) or BGF0 (PAL)
> > > + * bits 23:    FIELD (NTSC) or BGF0 (PAL)
> > >   * bits 24-29: frames, in BCD(6bits)
> > >   * bits 30:    drop  frame flag (0: non drop,    1: drop)
> > >   * bits 31:    color frame flag (0: unsync mode, 1: sync mode)
> > > @@ -78,8 +78,7 @@ int av_timecode_adjust_ntsc_framenum2(int framenum, int fps);
> > >   * @note Frame number adjustment is automatically done in case of drop timecode,
> > >   *       you do NOT have to call av_timecode_adjust_ntsc_framenum2().
> > >   * @note The frame number is relative to tc->start.
> > > - * @note Color frame (CF), binary group flags (BGF) and biphase mark polarity
> > > - *       correction (PC) bits are set to zero.
> > > + * @note Color frame (CF) and binary group flags (BGF) bits are set to zero.
> > >   */
> > >  uint32_t av_timecode_get_smpte_from_framenum(const AVTimecode *tc, int framenum);
> > > 
> > > diff --git a/tests/ref/vsynth/vsynth1-dv-hd b/tests/ref/vsynth/vsynth1-dv-hd
> > > index e22f28c704..a93ce50ec0 100644
> > > --- a/tests/ref/vsynth/vsynth1-dv-hd
> > > +++ b/tests/ref/vsynth/vsynth1-dv-hd
> > > @@ -1,4 +1,4 @@
> > > -77f146c73a24495495c649f1e0d26fee *tests/data/fate/vsynth1-dv-hd.dv
> > > +ce9f887af2391edbfcb21675962db54c *tests/data/fate/vsynth1-dv-hd.dv
> > >  14400000 tests/data/fate/vsynth1-dv-hd.dv
> > >  34b78cf725346c7f819c9d6209b8299a *tests/data/fate/vsynth1-dv-hd.out.rawvideo
> > >  stddev:    4.30 PSNR: 35.45 MAXDIFF:   74 bytes:  7603200/  7603200
> > > diff --git a/tests/ref/vsynth/vsynth2-dv-hd b/tests/ref/vsynth/vsynth2-dv-hd
> > > index 5c4ed86fe7..9552c31948 100644
> > > --- a/tests/ref/vsynth/vsynth2-dv-hd
> > > +++ b/tests/ref/vsynth/vsynth2-dv-hd
> > > @@ -1,4 +1,4 @@
> > > -168f80951daf5eeefacac33fd06db87d *tests/data/fate/vsynth2-dv-hd.dv
> > > +c152c7c3f9ef8f404b4bb7a388b947be *tests/data/fate/vsynth2-dv-hd.dv
> > >  14400000 tests/data/fate/vsynth2-dv-hd.dv
> > >  15dbe911532aca81c67bdd2846419027 *tests/data/fate/vsynth2-dv-hd.out.rawvideo
> > >  stddev:    1.75 PSNR: 43.26 MAXDIFF:   34 bytes:  7603200/  7603200
> > > diff --git a/tests/ref/vsynth/vsynth3-dv-hd b/tests/ref/vsynth/vsynth3-dv-hd
> > > index ed94d08155..d0420d6f07 100644
> > > --- a/tests/ref/vsynth/vsynth3-dv-hd
> > > +++ b/tests/ref/vsynth/vsynth3-dv-hd
> > > @@ -1,4 +1,4 @@
> > > -1211b843d8dde81c583023e15beff35a *tests/data/fate/vsynth3-dv-hd.dv
> > > +973b0856d1d94793bd6e8951284e642d *tests/data/fate/vsynth3-dv-hd.dv
> > >  14400000 tests/data/fate/vsynth3-dv-hd.dv
> > >  a038ad7c3c09f776304ef7accdea9c74 *tests/data/fate/vsynth3-dv-hd.out.rawvideo
> > >  stddev:    0.00 PSNR:999.99 MAXDIFF:    0 bytes:    86700/    86700
> > > diff --git a/tests/ref/vsynth/vsynth_lena-dv-hd b/tests/ref/vsynth/vsynth_lena-dv-hd
> > > index 6764f03c35..8b87da40ba 100644
> > > --- a/tests/ref/vsynth/vsynth_lena-dv-hd
> > > +++ b/tests/ref/vsynth/vsynth_lena-dv-hd
> > > @@ -1,4 +1,4 @@
> > > -fa8df53531f9e58f90bc9a33e5c78ca8 *tests/data/fate/vsynth_lena-dv-hd.dv
> > > +30652b3740a32fa8bcee3b06a66b1dd2 *tests/data/fate/vsynth_lena-dv-hd.dv
> > >  14400000 tests/data/fate/vsynth_lena-dv-hd.dv
> > >  4db4175c80ea1f16b7ec303611b8873a *tests/data/fate/vsynth_lena-dv-hd.out.rawvideo
> > >  stddev:    1.49 PSNR: 44.66 MAXDIFF:   27 bytes:  7603200/  7603200
> > > -- 
> > > 2.26.2
> > > 
> > > _______________________________________________
> > > 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".
Marton Balint July 21, 2020, 10:42 a.m. UTC | #4
On Tue, 21 Jul 2020, lance.lmwang@gmail.com wrote:

> On Tue, Jul 21, 2020 at 11:07:29AM +0200, Marton Balint wrote:
>> 
>> 
>> On Tue, 21 Jul 2020, lance.lmwang@gmail.com wrote:
>> 
>> > On Mon, Jul 20, 2020 at 11:04:38PM +0200, Marton Balint wrote:
>> > > 50/60 fps timecode is using the field bit (which is the same as the phase
>> > > correction bit) to signal the least significant bit of a 50/60 fps timecode.
>> > > See SMPTE ST 12-1:2014 section 12.1.
>> > > 
>> > > Let's add support for this by using the recently added av_timecode_get_smpte
>> > > function which handles this properly.
>> > > 
>> > > This change affects DV and MXF encoder, MXF has no fate for 50/60fps content,
>> > > DV does, therefore the changes. It also affects decklink indev.
>> > > 
>> > > MediaInfo (a recent version) confirms that half-frame timecode must be
>> > > inserted. (although MediaInfo does not seem to check the field flag).
>> > > MXFInspect confirms valid timecode insertion to the System Item of MXF files.
>> > > 
>> > > Signed-off-by: Marton Balint <cus@passwd.hu>
>> > > ---
>> > >  libavutil/timecode.c               | 15 +--------------
>> > >  libavutil/timecode.h               |  7 +++----
>> > >  tests/ref/vsynth/vsynth1-dv-hd     |  2 +-
>> > >  tests/ref/vsynth/vsynth2-dv-hd     |  2 +-
>> > >  tests/ref/vsynth/vsynth3-dv-hd     |  2 +-
>> > >  tests/ref/vsynth/vsynth_lena-dv-hd |  2 +-
>> > >  6 files changed, 8 insertions(+), 22 deletions(-)
>> > > 
>> > > diff --git a/libavutil/timecode.c b/libavutil/timecode.c
>> > > index cca53d73c4..cb916970ef 100644
>> > > --- a/libavutil/timecode.c
>> > > +++ b/libavutil/timecode.c
>> > > @@ -65,20 +65,7 @@ uint32_t av_timecode_get_smpte_from_framenum(const AVTimecode *tc, int framenum)
>> > >      ss = framenum / fps      % 60;
>> > >      mm = framenum / (fps*60) % 60;
>> > >      hh = framenum / (fps*3600) % 24;
>> > > -    return 0         << 31 | // color frame flag (0: unsync mode, 1: sync mode)
>> > > -           drop      << 30 | // drop  frame flag (0: non drop,    1: drop)
>> > > -           (ff / 10) << 28 | // tens  of frames
>> > > -           (ff % 10) << 24 | // units of frames
>> > > -           0         << 23 | // PC (NTSC) or BGF0 (PAL)
>> > > -           (ss / 10) << 20 | // tens  of seconds
>> > > -           (ss % 10) << 16 | // units of seconds
>> > > -           0         << 15 | // BGF0 (NTSC) or BGF2 (PAL)
>> > > -           (mm / 10) << 12 | // tens  of minutes
>> > > -           (mm % 10) <<  8 | // units of minutes
>> > > -           0         <<  7 | // BGF2 (NTSC) or PC (PAL)
>> > > -           0         <<  6 | // BGF1
>> > > -           (hh / 10) <<  4 | // tens  of hours
>> > > -           (hh % 10);        // units of hours
>> > > +    return av_timecode_get_smpte(tc->rate, drop, hh, mm, ss, ff);
>> > 
>> > av_timecode_get_smpte() is used by the h264/hevc, and the specs reserved more bits for ff, so in case it's
>> > > 30fps, so av_timecode_get_smpte() will divide ff by 2 to get frame pair. But for SMPTE ST 12 specs, the
>> > ff reserved 2 bits for tens, so for > 30fps, it's supported by frame pair and ff
>> > is divide by 2 already by the spec, so you'll get unexpected results if divide
>> > by 2 again(four frame pair if test)
>> 
>> I don't understand what you want to say. av_timecode_get_smpte_from_framenum
>> is a function for which the number of frames (as a single integer) is
>> passed. Clearly the number of frames as a single integer is not divided by
>> anything, so there cannot be double division when
>> av_timecode_get_smpte_from_framenum() calls av_timecode_get_smpte().
>
> For example, when I'm print out the tc from timecode->GetString(&decklink_tc) in
> decklink_dec.cpp for fps = 50, the tc will be:
>
> 00:00:00:00
> 00:00:00:00  -> repeat TC for frame pair
> 00:00:00:01
> 00:00:00:01  -> repeat
> ....
>
> 00:00:00:24
> 00:00:00:24  -> repeat, 50 frame now
>
> 00:00:00:00
> 00:00:00:00  -> repeat

That's because your input is containing the wrong timecode, as I suggested 
in the other thread.

>
> If the av_timecode_get_smpte_from_framenum(), the ff will be divided by 2 after change, you'll get below:
>
> 00:00:00:00
> 00:00:00:00
> 00:00:00:00
> 00:00:00:00
> 00:00:00:01
> 00:00:00:01
> 00:00:00:01
> 00:00:00:01
> ....
>
> 00:00:00:12
> 00:00:00:12
> 00:00:00:12
> 00:00:00:12  -> 50 frame now, repeat 4 times
>
> 00:00:00:00
> 00:00:00:00
> 00:00:00:00
> 00:00:00:00

And this is because av_timecode_make_smpte_tc_string does not handle 50fps 
timecodes properly.

Regards,
Marton

>
> I'm not sure whether it's clear to explain my meaning.
>
>
>> 
>> Regards,
>> Marton
>> 
>> > 
>> > 
>> > >  }
>> > > 
>> > >  uint32_t av_timecode_get_smpte(AVRational rate, int drop, int hh, int mm, int ss, int ff)
>> > > diff --git a/libavutil/timecode.h b/libavutil/timecode.h
>> > > index 5801330921..e54b116e93 100644
>> > > --- a/libavutil/timecode.h
>> > > +++ b/libavutil/timecode.h
>> > > @@ -66,11 +66,11 @@ int av_timecode_adjust_ntsc_framenum2(int framenum, int fps);
>> > >   * the format description as follows:
>> > >   * bits 0-5:   hours, in BCD(6bits)
>> > >   * bits 6:     BGF1
>> > > - * bits 7:     BGF2 (NTSC) or PC (PAL)
>> > > + * bits 7:     BGF2 (NTSC) or FIELD (PAL)
>> > >   * bits 8-14:  minutes, in BCD(7bits)
>> > >   * bits 15:    BGF0 (NTSC) or BGF2 (PAL)
>> > >   * bits 16-22: seconds, in BCD(7bits)
>> > > - * bits 23:    PC (NTSC) or BGF0 (PAL)
>> > > + * bits 23:    FIELD (NTSC) or BGF0 (PAL)
>> > >   * bits 24-29: frames, in BCD(6bits)
>> > >   * bits 30:    drop  frame flag (0: non drop,    1: drop)
>> > >   * bits 31:    color frame flag (0: unsync mode, 1: sync mode)
>> > > @@ -78,8 +78,7 @@ int av_timecode_adjust_ntsc_framenum2(int framenum, int fps);
>> > >   * @note Frame number adjustment is automatically done in case of drop timecode,
>> > >   *       you do NOT have to call av_timecode_adjust_ntsc_framenum2().
>> > >   * @note The frame number is relative to tc->start.
>> > > - * @note Color frame (CF), binary group flags (BGF) and biphase mark polarity
>> > > - *       correction (PC) bits are set to zero.
>> > > + * @note Color frame (CF) and binary group flags (BGF) bits are set to zero.
>> > >   */
>> > >  uint32_t av_timecode_get_smpte_from_framenum(const AVTimecode *tc, int framenum);
>> > > 
>> > > diff --git a/tests/ref/vsynth/vsynth1-dv-hd b/tests/ref/vsynth/vsynth1-dv-hd
>> > > index e22f28c704..a93ce50ec0 100644
>> > > --- a/tests/ref/vsynth/vsynth1-dv-hd
>> > > +++ b/tests/ref/vsynth/vsynth1-dv-hd
>> > > @@ -1,4 +1,4 @@
>> > > -77f146c73a24495495c649f1e0d26fee *tests/data/fate/vsynth1-dv-hd.dv
>> > > +ce9f887af2391edbfcb21675962db54c *tests/data/fate/vsynth1-dv-hd.dv
>> > >  14400000 tests/data/fate/vsynth1-dv-hd.dv
>> > >  34b78cf725346c7f819c9d6209b8299a *tests/data/fate/vsynth1-dv-hd.out.rawvideo
>> > >  stddev:    4.30 PSNR: 35.45 MAXDIFF:   74 bytes:  7603200/  7603200
>> > > diff --git a/tests/ref/vsynth/vsynth2-dv-hd b/tests/ref/vsynth/vsynth2-dv-hd
>> > > index 5c4ed86fe7..9552c31948 100644
>> > > --- a/tests/ref/vsynth/vsynth2-dv-hd
>> > > +++ b/tests/ref/vsynth/vsynth2-dv-hd
>> > > @@ -1,4 +1,4 @@
>> > > -168f80951daf5eeefacac33fd06db87d *tests/data/fate/vsynth2-dv-hd.dv
>> > > +c152c7c3f9ef8f404b4bb7a388b947be *tests/data/fate/vsynth2-dv-hd.dv
>> > >  14400000 tests/data/fate/vsynth2-dv-hd.dv
>> > >  15dbe911532aca81c67bdd2846419027 *tests/data/fate/vsynth2-dv-hd.out.rawvideo
>> > >  stddev:    1.75 PSNR: 43.26 MAXDIFF:   34 bytes:  7603200/  7603200
>> > > diff --git a/tests/ref/vsynth/vsynth3-dv-hd b/tests/ref/vsynth/vsynth3-dv-hd
>> > > index ed94d08155..d0420d6f07 100644
>> > > --- a/tests/ref/vsynth/vsynth3-dv-hd
>> > > +++ b/tests/ref/vsynth/vsynth3-dv-hd
>> > > @@ -1,4 +1,4 @@
>> > > -1211b843d8dde81c583023e15beff35a *tests/data/fate/vsynth3-dv-hd.dv
>> > > +973b0856d1d94793bd6e8951284e642d *tests/data/fate/vsynth3-dv-hd.dv
>> > >  14400000 tests/data/fate/vsynth3-dv-hd.dv
>> > >  a038ad7c3c09f776304ef7accdea9c74 *tests/data/fate/vsynth3-dv-hd.out.rawvideo
>> > >  stddev:    0.00 PSNR:999.99 MAXDIFF:    0 bytes:    86700/    86700
>> > > diff --git a/tests/ref/vsynth/vsynth_lena-dv-hd b/tests/ref/vsynth/vsynth_lena-dv-hd
>> > > index 6764f03c35..8b87da40ba 100644
>> > > --- a/tests/ref/vsynth/vsynth_lena-dv-hd
>> > > +++ b/tests/ref/vsynth/vsynth_lena-dv-hd
>> > > @@ -1,4 +1,4 @@
>> > > -fa8df53531f9e58f90bc9a33e5c78ca8 *tests/data/fate/vsynth_lena-dv-hd.dv
>> > > +30652b3740a32fa8bcee3b06a66b1dd2 *tests/data/fate/vsynth_lena-dv-hd.dv
>> > >  14400000 tests/data/fate/vsynth_lena-dv-hd.dv
>> > >  4db4175c80ea1f16b7ec303611b8873a *tests/data/fate/vsynth_lena-dv-hd.out.rawvideo
>> > >  stddev:    1.49 PSNR: 44.66 MAXDIFF:   27 bytes:  7603200/  7603200
>> > > -- 
>> > > 2.26.2
>> > > 
>> > > _______________________________________________
>> > > 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".
>
> -- 
> 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".
Limin Wang July 21, 2020, 2:20 p.m. UTC | #5
On Tue, Jul 21, 2020 at 12:42:03PM +0200, Marton Balint wrote:
> 
> 
> On Tue, 21 Jul 2020, lance.lmwang@gmail.com wrote:
> 
> > On Tue, Jul 21, 2020 at 11:07:29AM +0200, Marton Balint wrote:
> > > 
> > > 
> > > On Tue, 21 Jul 2020, lance.lmwang@gmail.com wrote:
> > > 
> > > > On Mon, Jul 20, 2020 at 11:04:38PM +0200, Marton Balint wrote:
> > > > > 50/60 fps timecode is using the field bit (which is the same as the phase
> > > > > correction bit) to signal the least significant bit of a 50/60 fps timecode.
> > > > > See SMPTE ST 12-1:2014 section 12.1.
> > > > > > > Let's add support for this by using the recently added
> > > av_timecode_get_smpte
> > > > > function which handles this properly.
> > > > > > > This change affects DV and MXF encoder, MXF has no fate for
> > > 50/60fps content,
> > > > > DV does, therefore the changes. It also affects decklink indev.
> > > > > > > MediaInfo (a recent version) confirms that half-frame
> > > timecode must be
> > > > > inserted. (although MediaInfo does not seem to check the field flag).
> > > > > MXFInspect confirms valid timecode insertion to the System Item of MXF files.
> > > > > > > Signed-off-by: Marton Balint <cus@passwd.hu>
> > > > > ---
> > > > >  libavutil/timecode.c               | 15 +--------------
> > > > >  libavutil/timecode.h               |  7 +++----
> > > > >  tests/ref/vsynth/vsynth1-dv-hd     |  2 +-
> > > > >  tests/ref/vsynth/vsynth2-dv-hd     |  2 +-
> > > > >  tests/ref/vsynth/vsynth3-dv-hd     |  2 +-
> > > > >  tests/ref/vsynth/vsynth_lena-dv-hd |  2 +-
> > > > >  6 files changed, 8 insertions(+), 22 deletions(-)
> > > > > > > diff --git a/libavutil/timecode.c b/libavutil/timecode.c
> > > > > index cca53d73c4..cb916970ef 100644
> > > > > --- a/libavutil/timecode.c
> > > > > +++ b/libavutil/timecode.c
> > > > > @@ -65,20 +65,7 @@ uint32_t av_timecode_get_smpte_from_framenum(const AVTimecode *tc, int framenum)
> > > > >      ss = framenum / fps      % 60;
> > > > >      mm = framenum / (fps*60) % 60;
> > > > >      hh = framenum / (fps*3600) % 24;
> > > > > -    return 0         << 31 | // color frame flag (0: unsync mode, 1: sync mode)
> > > > > -           drop      << 30 | // drop  frame flag (0: non drop,    1: drop)
> > > > > -           (ff / 10) << 28 | // tens  of frames
> > > > > -           (ff % 10) << 24 | // units of frames
> > > > > -           0         << 23 | // PC (NTSC) or BGF0 (PAL)
> > > > > -           (ss / 10) << 20 | // tens  of seconds
> > > > > -           (ss % 10) << 16 | // units of seconds
> > > > > -           0         << 15 | // BGF0 (NTSC) or BGF2 (PAL)
> > > > > -           (mm / 10) << 12 | // tens  of minutes
> > > > > -           (mm % 10) <<  8 | // units of minutes
> > > > > -           0         <<  7 | // BGF2 (NTSC) or PC (PAL)
> > > > > -           0         <<  6 | // BGF1
> > > > > -           (hh / 10) <<  4 | // tens  of hours
> > > > > -           (hh % 10);        // units of hours
> > > > > +    return av_timecode_get_smpte(tc->rate, drop, hh, mm, ss, ff);
> > > > > av_timecode_get_smpte() is used by the h264/hevc, and the specs
> > > reserved more bits for ff, so in case it's
> > > > > 30fps, so av_timecode_get_smpte() will divide ff by 2 to get frame pair. But for SMPTE ST 12 specs, the
> > > > ff reserved 2 bits for tens, so for > 30fps, it's supported by frame pair and ff
> > > > is divide by 2 already by the spec, so you'll get unexpected results if divide
> > > > by 2 again(four frame pair if test)
> > > 
> > > I don't understand what you want to say. av_timecode_get_smpte_from_framenum
> > > is a function for which the number of frames (as a single integer) is
> > > passed. Clearly the number of frames as a single integer is not divided by
> > > anything, so there cannot be double division when
> > > av_timecode_get_smpte_from_framenum() calls av_timecode_get_smpte().
> > 
> > For example, when I'm print out the tc from timecode->GetString(&decklink_tc) in
> > decklink_dec.cpp for fps = 50, the tc will be:
> > 
> > 00:00:00:00
> > 00:00:00:00  -> repeat TC for frame pair
> > 00:00:00:01
> > 00:00:00:01  -> repeat
> > ....
> > 
> > 00:00:00:24
> > 00:00:00:24  -> repeat, 50 frame now
> > 
> > 00:00:00:00
> > 00:00:00:00  -> repeat
> 
> That's because your input is containing the wrong timecode, as I suggested
> in the other thread.

OK, I hopes it's true, it make me confusing about the result.

> 
> > 
> > If the av_timecode_get_smpte_from_framenum(), the ff will be divided by 2 after change, you'll get below:
> > 
> > 00:00:00:00
> > 00:00:00:00
> > 00:00:00:00
> > 00:00:00:00
> > 00:00:00:01
> > 00:00:00:01
> > 00:00:00:01
> > 00:00:00:01
> > ....
> > 
> > 00:00:00:12
> > 00:00:00:12
> > 00:00:00:12
> > 00:00:00:12  -> 50 frame now, repeat 4 times
> > 
> > 00:00:00:00
> > 00:00:00:00
> > 00:00:00:00
> > 00:00:00:00
> 
> And this is because av_timecode_make_smpte_tc_string does not handle 50fps
> timecodes properly.
> 
> Regards,
> Marton
> 
> > 
> > I'm not sure whether it's clear to explain my meaning.
> > 
> > 
> > > 
> > > Regards,
> > > Marton
> > > 
> > > > > > >  }
> > > > > > >  uint32_t av_timecode_get_smpte(AVRational rate, int drop,
> > > int hh, int mm, int ss, int ff)
> > > > > diff --git a/libavutil/timecode.h b/libavutil/timecode.h
> > > > > index 5801330921..e54b116e93 100644
> > > > > --- a/libavutil/timecode.h
> > > > > +++ b/libavutil/timecode.h
> > > > > @@ -66,11 +66,11 @@ int av_timecode_adjust_ntsc_framenum2(int framenum, int fps);
> > > > >   * the format description as follows:
> > > > >   * bits 0-5:   hours, in BCD(6bits)
> > > > >   * bits 6:     BGF1
> > > > > - * bits 7:     BGF2 (NTSC) or PC (PAL)
> > > > > + * bits 7:     BGF2 (NTSC) or FIELD (PAL)
> > > > >   * bits 8-14:  minutes, in BCD(7bits)
> > > > >   * bits 15:    BGF0 (NTSC) or BGF2 (PAL)
> > > > >   * bits 16-22: seconds, in BCD(7bits)
> > > > > - * bits 23:    PC (NTSC) or BGF0 (PAL)
> > > > > + * bits 23:    FIELD (NTSC) or BGF0 (PAL)
> > > > >   * bits 24-29: frames, in BCD(6bits)
> > > > >   * bits 30:    drop  frame flag (0: non drop,    1: drop)
> > > > >   * bits 31:    color frame flag (0: unsync mode, 1: sync mode)
> > > > > @@ -78,8 +78,7 @@ int av_timecode_adjust_ntsc_framenum2(int framenum, int fps);
> > > > >   * @note Frame number adjustment is automatically done in case of drop timecode,
> > > > >   *       you do NOT have to call av_timecode_adjust_ntsc_framenum2().
> > > > >   * @note The frame number is relative to tc->start.
> > > > > - * @note Color frame (CF), binary group flags (BGF) and biphase mark polarity
> > > > > - *       correction (PC) bits are set to zero.
> > > > > + * @note Color frame (CF) and binary group flags (BGF) bits are set to zero.
> > > > >   */
> > > > >  uint32_t av_timecode_get_smpte_from_framenum(const AVTimecode *tc, int framenum);
> > > > > > > diff --git a/tests/ref/vsynth/vsynth1-dv-hd
> > > b/tests/ref/vsynth/vsynth1-dv-hd
> > > > > index e22f28c704..a93ce50ec0 100644
> > > > > --- a/tests/ref/vsynth/vsynth1-dv-hd
> > > > > +++ b/tests/ref/vsynth/vsynth1-dv-hd
> > > > > @@ -1,4 +1,4 @@
> > > > > -77f146c73a24495495c649f1e0d26fee *tests/data/fate/vsynth1-dv-hd.dv
> > > > > +ce9f887af2391edbfcb21675962db54c *tests/data/fate/vsynth1-dv-hd.dv
> > > > >  14400000 tests/data/fate/vsynth1-dv-hd.dv
> > > > >  34b78cf725346c7f819c9d6209b8299a *tests/data/fate/vsynth1-dv-hd.out.rawvideo
> > > > >  stddev:    4.30 PSNR: 35.45 MAXDIFF:   74 bytes:  7603200/  7603200
> > > > > diff --git a/tests/ref/vsynth/vsynth2-dv-hd b/tests/ref/vsynth/vsynth2-dv-hd
> > > > > index 5c4ed86fe7..9552c31948 100644
> > > > > --- a/tests/ref/vsynth/vsynth2-dv-hd
> > > > > +++ b/tests/ref/vsynth/vsynth2-dv-hd
> > > > > @@ -1,4 +1,4 @@
> > > > > -168f80951daf5eeefacac33fd06db87d *tests/data/fate/vsynth2-dv-hd.dv
> > > > > +c152c7c3f9ef8f404b4bb7a388b947be *tests/data/fate/vsynth2-dv-hd.dv
> > > > >  14400000 tests/data/fate/vsynth2-dv-hd.dv
> > > > >  15dbe911532aca81c67bdd2846419027 *tests/data/fate/vsynth2-dv-hd.out.rawvideo
> > > > >  stddev:    1.75 PSNR: 43.26 MAXDIFF:   34 bytes:  7603200/  7603200
> > > > > diff --git a/tests/ref/vsynth/vsynth3-dv-hd b/tests/ref/vsynth/vsynth3-dv-hd
> > > > > index ed94d08155..d0420d6f07 100644
> > > > > --- a/tests/ref/vsynth/vsynth3-dv-hd
> > > > > +++ b/tests/ref/vsynth/vsynth3-dv-hd
> > > > > @@ -1,4 +1,4 @@
> > > > > -1211b843d8dde81c583023e15beff35a *tests/data/fate/vsynth3-dv-hd.dv
> > > > > +973b0856d1d94793bd6e8951284e642d *tests/data/fate/vsynth3-dv-hd.dv
> > > > >  14400000 tests/data/fate/vsynth3-dv-hd.dv
> > > > >  a038ad7c3c09f776304ef7accdea9c74 *tests/data/fate/vsynth3-dv-hd.out.rawvideo
> > > > >  stddev:    0.00 PSNR:999.99 MAXDIFF:    0 bytes:    86700/    86700
> > > > > diff --git a/tests/ref/vsynth/vsynth_lena-dv-hd b/tests/ref/vsynth/vsynth_lena-dv-hd
> > > > > index 6764f03c35..8b87da40ba 100644
> > > > > --- a/tests/ref/vsynth/vsynth_lena-dv-hd
> > > > > +++ b/tests/ref/vsynth/vsynth_lena-dv-hd
> > > > > @@ -1,4 +1,4 @@
> > > > > -fa8df53531f9e58f90bc9a33e5c78ca8 *tests/data/fate/vsynth_lena-dv-hd.dv
> > > > > +30652b3740a32fa8bcee3b06a66b1dd2 *tests/data/fate/vsynth_lena-dv-hd.dv
> > > > >  14400000 tests/data/fate/vsynth_lena-dv-hd.dv
> > > > >  4db4175c80ea1f16b7ec303611b8873a *tests/data/fate/vsynth_lena-dv-hd.out.rawvideo
> > > > >  stddev:    1.49 PSNR: 44.66 MAXDIFF:   27 bytes:  7603200/  7603200
> > > > > -- > > 2.26.2
> > > > > > > _______________________________________________
> > > > > 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".
> > 
> > -- 
> > 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".
Limin Wang July 21, 2020, 11:12 p.m. UTC | #6
On Mon, Jul 20, 2020 at 11:04:38PM +0200, Marton Balint wrote:
> 50/60 fps timecode is using the field bit (which is the same as the phase
> correction bit) to signal the least significant bit of a 50/60 fps timecode.
> See SMPTE ST 12-1:2014 section 12.1.
> 
> Let's add support for this by using the recently added av_timecode_get_smpte
> function which handles this properly.
> 
> This change affects DV and MXF encoder, MXF has no fate for 50/60fps content,
> DV does, therefore the changes. It also affects decklink indev.
> 
> MediaInfo (a recent version) confirms that half-frame timecode must be
> inserted. (although MediaInfo does not seem to check the field flag).
> MXFInspect confirms valid timecode insertion to the System Item of MXF files.
> 
> Signed-off-by: Marton Balint <cus@passwd.hu>
> ---
>  libavutil/timecode.c               | 15 +--------------
>  libavutil/timecode.h               |  7 +++----
>  tests/ref/vsynth/vsynth1-dv-hd     |  2 +-
>  tests/ref/vsynth/vsynth2-dv-hd     |  2 +-
>  tests/ref/vsynth/vsynth3-dv-hd     |  2 +-
>  tests/ref/vsynth/vsynth_lena-dv-hd |  2 +-
>  6 files changed, 8 insertions(+), 22 deletions(-)
> 
> diff --git a/libavutil/timecode.c b/libavutil/timecode.c
> index cca53d73c4..cb916970ef 100644
> --- a/libavutil/timecode.c
> +++ b/libavutil/timecode.c
> @@ -65,20 +65,7 @@ uint32_t av_timecode_get_smpte_from_framenum(const AVTimecode *tc, int framenum)
>      ss = framenum / fps      % 60;
>      mm = framenum / (fps*60) % 60;
>      hh = framenum / (fps*3600) % 24;
> -    return 0         << 31 | // color frame flag (0: unsync mode, 1: sync mode)
> -           drop      << 30 | // drop  frame flag (0: non drop,    1: drop)
> -           (ff / 10) << 28 | // tens  of frames
> -           (ff % 10) << 24 | // units of frames
> -           0         << 23 | // PC (NTSC) or BGF0 (PAL)
> -           (ss / 10) << 20 | // tens  of seconds
> -           (ss % 10) << 16 | // units of seconds
> -           0         << 15 | // BGF0 (NTSC) or BGF2 (PAL)
> -           (mm / 10) << 12 | // tens  of minutes
> -           (mm % 10) <<  8 | // units of minutes
> -           0         <<  7 | // BGF2 (NTSC) or PC (PAL)
> -           0         <<  6 | // BGF1
> -           (hh / 10) <<  4 | // tens  of hours
> -           (hh % 10);        // units of hours
> +    return av_timecode_get_smpte(tc->rate, drop, hh, mm, ss, ff);
>  }
>  
>  uint32_t av_timecode_get_smpte(AVRational rate, int drop, int hh, int mm, int ss, int ff)
> diff --git a/libavutil/timecode.h b/libavutil/timecode.h
> index 5801330921..e54b116e93 100644
> --- a/libavutil/timecode.h
> +++ b/libavutil/timecode.h
> @@ -66,11 +66,11 @@ int av_timecode_adjust_ntsc_framenum2(int framenum, int fps);
>   * the format description as follows:
>   * bits 0-5:   hours, in BCD(6bits)
>   * bits 6:     BGF1
> - * bits 7:     BGF2 (NTSC) or PC (PAL)
> + * bits 7:     BGF2 (NTSC) or FIELD (PAL)

If the format is coming from SMPTE 314m-1999, 4.4.2.2.1 Time code pack (TC), then
it use PC instead of FIELD for the interpretation of the bits.

PC: Biphase mark polarity correction
0 = even
1 = odd

About the bits interpretation:
FIELD is used by VITC
https://en.wikipedia.org/wiki/Vertical_interval_timecode

PC is used by LTC
https://en.wikipedia.org/wiki/Linear_timecode

So I prefer to change to PC/FIELD instead of FIELD only if want to update the
interpretation of the bits.


>   * bits 8-14:  minutes, in BCD(7bits)
>   * bits 15:    BGF0 (NTSC) or BGF2 (PAL)
>   * bits 16-22: seconds, in BCD(7bits)
> - * bits 23:    PC (NTSC) or BGF0 (PAL)
> + * bits 23:    FIELD (NTSC) or BGF0 (PAL)
>   * bits 24-29: frames, in BCD(6bits)
>   * bits 30:    drop  frame flag (0: non drop,    1: drop)
>   * bits 31:    color frame flag (0: unsync mode, 1: sync mode)
> @@ -78,8 +78,7 @@ int av_timecode_adjust_ntsc_framenum2(int framenum, int fps);
>   * @note Frame number adjustment is automatically done in case of drop timecode,
>   *       you do NOT have to call av_timecode_adjust_ntsc_framenum2().
>   * @note The frame number is relative to tc->start.
> - * @note Color frame (CF), binary group flags (BGF) and biphase mark polarity
> - *       correction (PC) bits are set to zero.
> + * @note Color frame (CF) and binary group flags (BGF) bits are set to zero.
>   */
>  uint32_t av_timecode_get_smpte_from_framenum(const AVTimecode *tc, int framenum);
>  
> diff --git a/tests/ref/vsynth/vsynth1-dv-hd b/tests/ref/vsynth/vsynth1-dv-hd
> index e22f28c704..a93ce50ec0 100644
> --- a/tests/ref/vsynth/vsynth1-dv-hd
> +++ b/tests/ref/vsynth/vsynth1-dv-hd
> @@ -1,4 +1,4 @@
> -77f146c73a24495495c649f1e0d26fee *tests/data/fate/vsynth1-dv-hd.dv
> +ce9f887af2391edbfcb21675962db54c *tests/data/fate/vsynth1-dv-hd.dv
>  14400000 tests/data/fate/vsynth1-dv-hd.dv
>  34b78cf725346c7f819c9d6209b8299a *tests/data/fate/vsynth1-dv-hd.out.rawvideo
>  stddev:    4.30 PSNR: 35.45 MAXDIFF:   74 bytes:  7603200/  7603200
> diff --git a/tests/ref/vsynth/vsynth2-dv-hd b/tests/ref/vsynth/vsynth2-dv-hd
> index 5c4ed86fe7..9552c31948 100644
> --- a/tests/ref/vsynth/vsynth2-dv-hd
> +++ b/tests/ref/vsynth/vsynth2-dv-hd
> @@ -1,4 +1,4 @@
> -168f80951daf5eeefacac33fd06db87d *tests/data/fate/vsynth2-dv-hd.dv
> +c152c7c3f9ef8f404b4bb7a388b947be *tests/data/fate/vsynth2-dv-hd.dv
>  14400000 tests/data/fate/vsynth2-dv-hd.dv
>  15dbe911532aca81c67bdd2846419027 *tests/data/fate/vsynth2-dv-hd.out.rawvideo
>  stddev:    1.75 PSNR: 43.26 MAXDIFF:   34 bytes:  7603200/  7603200
> diff --git a/tests/ref/vsynth/vsynth3-dv-hd b/tests/ref/vsynth/vsynth3-dv-hd
> index ed94d08155..d0420d6f07 100644
> --- a/tests/ref/vsynth/vsynth3-dv-hd
> +++ b/tests/ref/vsynth/vsynth3-dv-hd
> @@ -1,4 +1,4 @@
> -1211b843d8dde81c583023e15beff35a *tests/data/fate/vsynth3-dv-hd.dv
> +973b0856d1d94793bd6e8951284e642d *tests/data/fate/vsynth3-dv-hd.dv
>  14400000 tests/data/fate/vsynth3-dv-hd.dv
>  a038ad7c3c09f776304ef7accdea9c74 *tests/data/fate/vsynth3-dv-hd.out.rawvideo
>  stddev:    0.00 PSNR:999.99 MAXDIFF:    0 bytes:    86700/    86700
> diff --git a/tests/ref/vsynth/vsynth_lena-dv-hd b/tests/ref/vsynth/vsynth_lena-dv-hd
> index 6764f03c35..8b87da40ba 100644
> --- a/tests/ref/vsynth/vsynth_lena-dv-hd
> +++ b/tests/ref/vsynth/vsynth_lena-dv-hd
> @@ -1,4 +1,4 @@
> -fa8df53531f9e58f90bc9a33e5c78ca8 *tests/data/fate/vsynth_lena-dv-hd.dv
> +30652b3740a32fa8bcee3b06a66b1dd2 *tests/data/fate/vsynth_lena-dv-hd.dv
>  14400000 tests/data/fate/vsynth_lena-dv-hd.dv
>  4db4175c80ea1f16b7ec303611b8873a *tests/data/fate/vsynth_lena-dv-hd.out.rawvideo
>  stddev:    1.49 PSNR: 44.66 MAXDIFF:   27 bytes:  7603200/  7603200
> -- 
> 2.26.2
> 
> _______________________________________________
> 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 22, 2020, 8:46 a.m. UTC | #7
On Wed, 22 Jul 2020, lance.lmwang@gmail.com wrote:

> On Mon, Jul 20, 2020 at 11:04:38PM +0200, Marton Balint wrote:
>> 50/60 fps timecode is using the field bit (which is the same as the phase
>> correction bit) to signal the least significant bit of a 50/60 fps timecode.
>> See SMPTE ST 12-1:2014 section 12.1.
>> 
>> Let's add support for this by using the recently added av_timecode_get_smpte
>> function which handles this properly.
>> 
>> This change affects DV and MXF encoder, MXF has no fate for 50/60fps content,
>> DV does, therefore the changes. It also affects decklink indev.
>> 
>> MediaInfo (a recent version) confirms that half-frame timecode must be
>> inserted. (although MediaInfo does not seem to check the field flag).
>> MXFInspect confirms valid timecode insertion to the System Item of MXF files.
>> 
>> Signed-off-by: Marton Balint <cus@passwd.hu>
>> ---
>>  libavutil/timecode.c               | 15 +--------------
>>  libavutil/timecode.h               |  7 +++----
>>  tests/ref/vsynth/vsynth1-dv-hd     |  2 +-
>>  tests/ref/vsynth/vsynth2-dv-hd     |  2 +-
>>  tests/ref/vsynth/vsynth3-dv-hd     |  2 +-
>>  tests/ref/vsynth/vsynth_lena-dv-hd |  2 +-
>>  6 files changed, 8 insertions(+), 22 deletions(-)
>> 
>> diff --git a/libavutil/timecode.c b/libavutil/timecode.c
>> index cca53d73c4..cb916970ef 100644
>> --- a/libavutil/timecode.c
>> +++ b/libavutil/timecode.c
>> @@ -65,20 +65,7 @@ uint32_t av_timecode_get_smpte_from_framenum(const AVTimecode *tc, int framenum)
>>      ss = framenum / fps      % 60;
>>      mm = framenum / (fps*60) % 60;
>>      hh = framenum / (fps*3600) % 24;
>> -    return 0         << 31 | // color frame flag (0: unsync mode, 1: sync mode)
>> -           drop      << 30 | // drop  frame flag (0: non drop,    1: drop)
>> -           (ff / 10) << 28 | // tens  of frames
>> -           (ff % 10) << 24 | // units of frames
>> -           0         << 23 | // PC (NTSC) or BGF0 (PAL)
>> -           (ss / 10) << 20 | // tens  of seconds
>> -           (ss % 10) << 16 | // units of seconds
>> -           0         << 15 | // BGF0 (NTSC) or BGF2 (PAL)
>> -           (mm / 10) << 12 | // tens  of minutes
>> -           (mm % 10) <<  8 | // units of minutes
>> -           0         <<  7 | // BGF2 (NTSC) or PC (PAL)
>> -           0         <<  6 | // BGF1
>> -           (hh / 10) <<  4 | // tens  of hours
>> -           (hh % 10);        // units of hours
>> +    return av_timecode_get_smpte(tc->rate, drop, hh, mm, ss, ff);
>>  }
>>
>>  uint32_t av_timecode_get_smpte(AVRational rate, int drop, int hh, int mm, int ss, int ff)
>> diff --git a/libavutil/timecode.h b/libavutil/timecode.h
>> index 5801330921..e54b116e93 100644
>> --- a/libavutil/timecode.h
>> +++ b/libavutil/timecode.h
>> @@ -66,11 +66,11 @@ int av_timecode_adjust_ntsc_framenum2(int framenum, int fps);
>>   * the format description as follows:
>>   * bits 0-5:   hours, in BCD(6bits)
>>   * bits 6:     BGF1
>> - * bits 7:     BGF2 (NTSC) or PC (PAL)
>> + * bits 7:     BGF2 (NTSC) or FIELD (PAL)
>
> If the format is coming from SMPTE 314m-1999, 4.4.2.2.1 Time code pack (TC), then
> it use PC instead of FIELD for the interpretation of the bits.
>
> PC: Biphase mark polarity correction
> 0 = even
> 1 = odd
>
> About the bits interpretation:
> FIELD is used by VITC
> https://en.wikipedia.org/wiki/Vertical_interval_timecode
>
> PC is used by LTC
> https://en.wikipedia.org/wiki/Linear_timecode
>
> So I prefer to change to PC/FIELD instead of FIELD only if want to update the
> interpretation of the bits.

I don't think it is a good idea, after all this supposed to document the 
format the function returns and not what is in the DV standard. Note that 
this function is used for both DV and MXF and you are also using the same 
term "SMPTE 12M binary representation" in av_timecode_get_smpte() and that 
is already using the PC flag as FIELDS, and it would be confusing for that
function to refer to a different binary representation...

Therefore I'd rather keep things as is.

Regards,
Marton

>
>
>>   * bits 8-14:  minutes, in BCD(7bits)
>>   * bits 15:    BGF0 (NTSC) or BGF2 (PAL)
>>   * bits 16-22: seconds, in BCD(7bits)
>> - * bits 23:    PC (NTSC) or BGF0 (PAL)
>> + * bits 23:    FIELD (NTSC) or BGF0 (PAL)
>>   * bits 24-29: frames, in BCD(6bits)
>>   * bits 30:    drop  frame flag (0: non drop,    1: drop)
>>   * bits 31:    color frame flag (0: unsync mode, 1: sync mode)
>> @@ -78,8 +78,7 @@ int av_timecode_adjust_ntsc_framenum2(int framenum, int fps);
>>   * @note Frame number adjustment is automatically done in case of drop timecode,
>>   *       you do NOT have to call av_timecode_adjust_ntsc_framenum2().
>>   * @note The frame number is relative to tc->start.
>> - * @note Color frame (CF), binary group flags (BGF) and biphase mark polarity
>> - *       correction (PC) bits are set to zero.
>> + * @note Color frame (CF) and binary group flags (BGF) bits are set to zero.
>>   */
>>  uint32_t av_timecode_get_smpte_from_framenum(const AVTimecode *tc, int framenum);
>> 
>> diff --git a/tests/ref/vsynth/vsynth1-dv-hd b/tests/ref/vsynth/vsynth1-dv-hd
>> index e22f28c704..a93ce50ec0 100644
>> --- a/tests/ref/vsynth/vsynth1-dv-hd
>> +++ b/tests/ref/vsynth/vsynth1-dv-hd
>> @@ -1,4 +1,4 @@
>> -77f146c73a24495495c649f1e0d26fee *tests/data/fate/vsynth1-dv-hd.dv
>> +ce9f887af2391edbfcb21675962db54c *tests/data/fate/vsynth1-dv-hd.dv
>>  14400000 tests/data/fate/vsynth1-dv-hd.dv
>>  34b78cf725346c7f819c9d6209b8299a *tests/data/fate/vsynth1-dv-hd.out.rawvideo
>>  stddev:    4.30 PSNR: 35.45 MAXDIFF:   74 bytes:  7603200/  7603200
>> diff --git a/tests/ref/vsynth/vsynth2-dv-hd b/tests/ref/vsynth/vsynth2-dv-hd
>> index 5c4ed86fe7..9552c31948 100644
>> --- a/tests/ref/vsynth/vsynth2-dv-hd
>> +++ b/tests/ref/vsynth/vsynth2-dv-hd
>> @@ -1,4 +1,4 @@
>> -168f80951daf5eeefacac33fd06db87d *tests/data/fate/vsynth2-dv-hd.dv
>> +c152c7c3f9ef8f404b4bb7a388b947be *tests/data/fate/vsynth2-dv-hd.dv
>>  14400000 tests/data/fate/vsynth2-dv-hd.dv
>>  15dbe911532aca81c67bdd2846419027 *tests/data/fate/vsynth2-dv-hd.out.rawvideo
>>  stddev:    1.75 PSNR: 43.26 MAXDIFF:   34 bytes:  7603200/  7603200
>> diff --git a/tests/ref/vsynth/vsynth3-dv-hd b/tests/ref/vsynth/vsynth3-dv-hd
>> index ed94d08155..d0420d6f07 100644
>> --- a/tests/ref/vsynth/vsynth3-dv-hd
>> +++ b/tests/ref/vsynth/vsynth3-dv-hd
>> @@ -1,4 +1,4 @@
>> -1211b843d8dde81c583023e15beff35a *tests/data/fate/vsynth3-dv-hd.dv
>> +973b0856d1d94793bd6e8951284e642d *tests/data/fate/vsynth3-dv-hd.dv
>>  14400000 tests/data/fate/vsynth3-dv-hd.dv
>>  a038ad7c3c09f776304ef7accdea9c74 *tests/data/fate/vsynth3-dv-hd.out.rawvideo
>>  stddev:    0.00 PSNR:999.99 MAXDIFF:    0 bytes:    86700/    86700
>> diff --git a/tests/ref/vsynth/vsynth_lena-dv-hd b/tests/ref/vsynth/vsynth_lena-dv-hd
>> index 6764f03c35..8b87da40ba 100644
>> --- a/tests/ref/vsynth/vsynth_lena-dv-hd
>> +++ b/tests/ref/vsynth/vsynth_lena-dv-hd
>> @@ -1,4 +1,4 @@
>> -fa8df53531f9e58f90bc9a33e5c78ca8 *tests/data/fate/vsynth_lena-dv-hd.dv
>> +30652b3740a32fa8bcee3b06a66b1dd2 *tests/data/fate/vsynth_lena-dv-hd.dv
>>  14400000 tests/data/fate/vsynth_lena-dv-hd.dv
>>  4db4175c80ea1f16b7ec303611b8873a *tests/data/fate/vsynth_lena-dv-hd.out.rawvideo
>>  stddev:    1.49 PSNR: 44.66 MAXDIFF:   27 bytes:  7603200/  7603200
>> -- 
>> 2.26.2
>> 
>> _______________________________________________
>> 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".
diff mbox series

Patch

diff --git a/libavutil/timecode.c b/libavutil/timecode.c
index cca53d73c4..cb916970ef 100644
--- a/libavutil/timecode.c
+++ b/libavutil/timecode.c
@@ -65,20 +65,7 @@  uint32_t av_timecode_get_smpte_from_framenum(const AVTimecode *tc, int framenum)
     ss = framenum / fps      % 60;
     mm = framenum / (fps*60) % 60;
     hh = framenum / (fps*3600) % 24;
-    return 0         << 31 | // color frame flag (0: unsync mode, 1: sync mode)
-           drop      << 30 | // drop  frame flag (0: non drop,    1: drop)
-           (ff / 10) << 28 | // tens  of frames
-           (ff % 10) << 24 | // units of frames
-           0         << 23 | // PC (NTSC) or BGF0 (PAL)
-           (ss / 10) << 20 | // tens  of seconds
-           (ss % 10) << 16 | // units of seconds
-           0         << 15 | // BGF0 (NTSC) or BGF2 (PAL)
-           (mm / 10) << 12 | // tens  of minutes
-           (mm % 10) <<  8 | // units of minutes
-           0         <<  7 | // BGF2 (NTSC) or PC (PAL)
-           0         <<  6 | // BGF1
-           (hh / 10) <<  4 | // tens  of hours
-           (hh % 10);        // units of hours
+    return av_timecode_get_smpte(tc->rate, drop, hh, mm, ss, ff);
 }
 
 uint32_t av_timecode_get_smpte(AVRational rate, int drop, int hh, int mm, int ss, int ff)
diff --git a/libavutil/timecode.h b/libavutil/timecode.h
index 5801330921..e54b116e93 100644
--- a/libavutil/timecode.h
+++ b/libavutil/timecode.h
@@ -66,11 +66,11 @@  int av_timecode_adjust_ntsc_framenum2(int framenum, int fps);
  * the format description as follows:
  * bits 0-5:   hours, in BCD(6bits)
  * bits 6:     BGF1
- * bits 7:     BGF2 (NTSC) or PC (PAL)
+ * bits 7:     BGF2 (NTSC) or FIELD (PAL)
  * bits 8-14:  minutes, in BCD(7bits)
  * bits 15:    BGF0 (NTSC) or BGF2 (PAL)
  * bits 16-22: seconds, in BCD(7bits)
- * bits 23:    PC (NTSC) or BGF0 (PAL)
+ * bits 23:    FIELD (NTSC) or BGF0 (PAL)
  * bits 24-29: frames, in BCD(6bits)
  * bits 30:    drop  frame flag (0: non drop,    1: drop)
  * bits 31:    color frame flag (0: unsync mode, 1: sync mode)
@@ -78,8 +78,7 @@  int av_timecode_adjust_ntsc_framenum2(int framenum, int fps);
  * @note Frame number adjustment is automatically done in case of drop timecode,
  *       you do NOT have to call av_timecode_adjust_ntsc_framenum2().
  * @note The frame number is relative to tc->start.
- * @note Color frame (CF), binary group flags (BGF) and biphase mark polarity
- *       correction (PC) bits are set to zero.
+ * @note Color frame (CF) and binary group flags (BGF) bits are set to zero.
  */
 uint32_t av_timecode_get_smpte_from_framenum(const AVTimecode *tc, int framenum);
 
diff --git a/tests/ref/vsynth/vsynth1-dv-hd b/tests/ref/vsynth/vsynth1-dv-hd
index e22f28c704..a93ce50ec0 100644
--- a/tests/ref/vsynth/vsynth1-dv-hd
+++ b/tests/ref/vsynth/vsynth1-dv-hd
@@ -1,4 +1,4 @@ 
-77f146c73a24495495c649f1e0d26fee *tests/data/fate/vsynth1-dv-hd.dv
+ce9f887af2391edbfcb21675962db54c *tests/data/fate/vsynth1-dv-hd.dv
 14400000 tests/data/fate/vsynth1-dv-hd.dv
 34b78cf725346c7f819c9d6209b8299a *tests/data/fate/vsynth1-dv-hd.out.rawvideo
 stddev:    4.30 PSNR: 35.45 MAXDIFF:   74 bytes:  7603200/  7603200
diff --git a/tests/ref/vsynth/vsynth2-dv-hd b/tests/ref/vsynth/vsynth2-dv-hd
index 5c4ed86fe7..9552c31948 100644
--- a/tests/ref/vsynth/vsynth2-dv-hd
+++ b/tests/ref/vsynth/vsynth2-dv-hd
@@ -1,4 +1,4 @@ 
-168f80951daf5eeefacac33fd06db87d *tests/data/fate/vsynth2-dv-hd.dv
+c152c7c3f9ef8f404b4bb7a388b947be *tests/data/fate/vsynth2-dv-hd.dv
 14400000 tests/data/fate/vsynth2-dv-hd.dv
 15dbe911532aca81c67bdd2846419027 *tests/data/fate/vsynth2-dv-hd.out.rawvideo
 stddev:    1.75 PSNR: 43.26 MAXDIFF:   34 bytes:  7603200/  7603200
diff --git a/tests/ref/vsynth/vsynth3-dv-hd b/tests/ref/vsynth/vsynth3-dv-hd
index ed94d08155..d0420d6f07 100644
--- a/tests/ref/vsynth/vsynth3-dv-hd
+++ b/tests/ref/vsynth/vsynth3-dv-hd
@@ -1,4 +1,4 @@ 
-1211b843d8dde81c583023e15beff35a *tests/data/fate/vsynth3-dv-hd.dv
+973b0856d1d94793bd6e8951284e642d *tests/data/fate/vsynth3-dv-hd.dv
 14400000 tests/data/fate/vsynth3-dv-hd.dv
 a038ad7c3c09f776304ef7accdea9c74 *tests/data/fate/vsynth3-dv-hd.out.rawvideo
 stddev:    0.00 PSNR:999.99 MAXDIFF:    0 bytes:    86700/    86700
diff --git a/tests/ref/vsynth/vsynth_lena-dv-hd b/tests/ref/vsynth/vsynth_lena-dv-hd
index 6764f03c35..8b87da40ba 100644
--- a/tests/ref/vsynth/vsynth_lena-dv-hd
+++ b/tests/ref/vsynth/vsynth_lena-dv-hd
@@ -1,4 +1,4 @@ 
-fa8df53531f9e58f90bc9a33e5c78ca8 *tests/data/fate/vsynth_lena-dv-hd.dv
+30652b3740a32fa8bcee3b06a66b1dd2 *tests/data/fate/vsynth_lena-dv-hd.dv
 14400000 tests/data/fate/vsynth_lena-dv-hd.dv
 4db4175c80ea1f16b7ec303611b8873a *tests/data/fate/vsynth_lena-dv-hd.out.rawvideo
 stddev:    1.49 PSNR: 44.66 MAXDIFF:   27 bytes:  7603200/  7603200