[FFmpeg-devel] [PATCH 2/3] avutil/timecode: fix av_timecode_get_smpte_from_framenum with 50/60 fps
Marton Balint
cus at passwd.hu
Wed Jul 22 11:46:50 EEST 2020
On Wed, 22 Jul 2020, lance.lmwang at 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 at 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 at ffmpeg.org
>> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>
>> To unsubscribe, visit link above, or email
>> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".
>
> --
> Thanks,
> Limin Wang
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".
More information about the ffmpeg-devel
mailing list