| Message ID | 20200905182217.28037-1-cus@passwd.hu |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [FFmpeg-devel,1/8] avutil/timecode: fix av_timecode_get_smpte_from_framenum with 50/60 fps | expand |
| Context | Check | Description |
|---|---|---|
| andriy/default | pending | |
| andriy/make | success | Make finished |
| andriy/make_fate | success | Make fate finished |
On Sat, 5 Sep 2020, Marton Balint wrote: > SMPTE 12M timecode can only count frames up to 39, because the tens-of-frames > value is stored in 2 bit. In order to resolve this 50/60 fps SMPTE timecode is > using the field bit (which is the same bit 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. > > Therefore we slightly change the format of the return value of > av_timecode_get_smpte_from_framenum and AV_FRAME_DATA_S12M_TIMECODE and start > using the previously unused Phase Correction bit as Field bit. (As the SMPTE > standard suggests) > > We add 50/60 fps support to av_timecode_get_smpte_from_framenum by calling the > recently added av_timecode_get_smpte function in it which already handles this > properly. > > This change affects the decklink indev and the DV and MXF muxers. MXF has no > fate test for 50/60fps content, DV does, therefore the changes. > > MediaInfo (a recent version) confirms that half-frame timecode must be inserted > to DV. MXFInspect confirms valid timecode insertion to the System Item of MXF > files. For MXF, also see EBU R122. > > Note that for DV the field flag is not used because in the HDV specs (SMPTE > 370M) it is still defined as biphase mark polarity correction flag. So it > should not matter that the DV muxer overrides the field bit. Will apply the series soon. Thanks, Marton > > 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) > * 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 6b6d6e8159..3f2f9cc843 100644 > --- a/tests/ref/vsynth/vsynth1-dv-hd > +++ b/tests/ref/vsynth/vsynth1-dv-hd > @@ -1,4 +1,4 @@ > -22d1d62a834fe8416fe79c51760012c1 *tests/data/fate/vsynth1-dv-hd.dv > +b2bcafc74dec5f9ca516cb25dd07459b *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 c6dcb5a953..0a5046e4ec 100644 > --- a/tests/ref/vsynth/vsynth2-dv-hd > +++ b/tests/ref/vsynth/vsynth2-dv-hd > @@ -1,4 +1,4 @@ > -4270e5d552e0a05193f44bff75c2d271 *tests/data/fate/vsynth2-dv-hd.dv > +a9a4c750f7720e83d538d36c318be787 *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 d069e6980e..11a51b18b8 100644 > --- a/tests/ref/vsynth/vsynth3-dv-hd > +++ b/tests/ref/vsynth/vsynth3-dv-hd > @@ -1,4 +1,4 @@ > -2f81f3ccec178ba2fd9d3e3b46f33670 *tests/data/fate/vsynth3-dv-hd.dv > +63512193a0da09e15815c1be1b9c4fa5 *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 aba756c5f1..8711b6fa2f 100644 > --- a/tests/ref/vsynth/vsynth_lena-dv-hd > +++ b/tests/ref/vsynth/vsynth_lena-dv-hd > @@ -1,4 +1,4 @@ > -01a61c53943a421fa6a5e03dbc205972 *tests/data/fate/vsynth_lena-dv-hd.dv > +f17abb38ed2f416d57f3b956fae5dc82 *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".
On Sat, 12 Sep 2020, Marton Balint wrote: > > > On Sat, 5 Sep 2020, Marton Balint wrote: > >> SMPTE 12M timecode can only count frames up to 39, because the > tens-of-frames >> value is stored in 2 bit. In order to resolve this 50/60 fps SMPTE timecode > is >> using the field bit (which is the same bit 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. >> >> Therefore we slightly change the format of the return value of >> av_timecode_get_smpte_from_framenum and AV_FRAME_DATA_S12M_TIMECODE and > start >> using the previously unused Phase Correction bit as Field bit. (As the > SMPTE >> standard suggests) >> >> We add 50/60 fps support to av_timecode_get_smpte_from_framenum by calling > the >> recently added av_timecode_get_smpte function in it which already handles > this >> properly. >> >> This change affects the decklink indev and the DV and MXF muxers. MXF has > no >> fate test for 50/60fps content, DV does, therefore the changes. >> >> MediaInfo (a recent version) confirms that half-frame timecode must be > inserted >> to DV. MXFInspect confirms valid timecode insertion to the System Item of > MXF >> files. For MXF, also see EBU R122. >> >> Note that for DV the field flag is not used because in the HDV specs (SMPTE >> 370M) it is still defined as biphase mark polarity correction flag. So it >> should not matter that the DV muxer overrides the field bit. > > Will apply the series soon. Applied. Regards, Marton
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 6b6d6e8159..3f2f9cc843 100644 --- a/tests/ref/vsynth/vsynth1-dv-hd +++ b/tests/ref/vsynth/vsynth1-dv-hd @@ -1,4 +1,4 @@ -22d1d62a834fe8416fe79c51760012c1 *tests/data/fate/vsynth1-dv-hd.dv +b2bcafc74dec5f9ca516cb25dd07459b *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 c6dcb5a953..0a5046e4ec 100644 --- a/tests/ref/vsynth/vsynth2-dv-hd +++ b/tests/ref/vsynth/vsynth2-dv-hd @@ -1,4 +1,4 @@ -4270e5d552e0a05193f44bff75c2d271 *tests/data/fate/vsynth2-dv-hd.dv +a9a4c750f7720e83d538d36c318be787 *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 d069e6980e..11a51b18b8 100644 --- a/tests/ref/vsynth/vsynth3-dv-hd +++ b/tests/ref/vsynth/vsynth3-dv-hd @@ -1,4 +1,4 @@ -2f81f3ccec178ba2fd9d3e3b46f33670 *tests/data/fate/vsynth3-dv-hd.dv +63512193a0da09e15815c1be1b9c4fa5 *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 aba756c5f1..8711b6fa2f 100644 --- a/tests/ref/vsynth/vsynth_lena-dv-hd +++ b/tests/ref/vsynth/vsynth_lena-dv-hd @@ -1,4 +1,4 @@ -01a61c53943a421fa6a5e03dbc205972 *tests/data/fate/vsynth_lena-dv-hd.dv +f17abb38ed2f416d57f3b956fae5dc82 *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
SMPTE 12M timecode can only count frames up to 39, because the tens-of-frames value is stored in 2 bit. In order to resolve this 50/60 fps SMPTE timecode is using the field bit (which is the same bit 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. Therefore we slightly change the format of the return value of av_timecode_get_smpte_from_framenum and AV_FRAME_DATA_S12M_TIMECODE and start using the previously unused Phase Correction bit as Field bit. (As the SMPTE standard suggests) We add 50/60 fps support to av_timecode_get_smpte_from_framenum by calling the recently added av_timecode_get_smpte function in it which already handles this properly. This change affects the decklink indev and the DV and MXF muxers. MXF has no fate test for 50/60fps content, DV does, therefore the changes. MediaInfo (a recent version) confirms that half-frame timecode must be inserted to DV. MXFInspect confirms valid timecode insertion to the System Item of MXF files. For MXF, also see EBU R122. Note that for DV the field flag is not used because in the HDV specs (SMPTE 370M) it is still defined as biphase mark polarity correction flag. So it should not matter that the DV muxer overrides the field bit. 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(-)