Message ID | 20210117014845.81268-1-mindmark@gmail.com |
---|---|
State | Accepted |
Headers | show |
Series | [FFmpeg-devel,v2] avformat/mov: fix timecode with counter mode flag set | expand |
Context | Check | Description |
---|---|---|
andriy/x86_make | success | Make finished |
andriy/x86_make_fate | success | Make fate finished |
andriy/PPC64_make | success | Make finished |
andriy/PPC64_make_fate | success | Make fate finished |
On Sat, Jan 16, 2021 at 5:48 PM <mindmark@gmail.com> wrote: > From: Mark Reid <mindmark@gmail.com> > > The current behaviour ends up squaring the avg_frame_rate if the conter > mode flag is set. > This messes up the timecode calculation, and looks to me as a regression > that > seems to have been introduced 428b4aac. > > Upon further testing is seems that no special case is need for having the > counter flag set. > av_timecode_init appears to handles the timecode correctly, at least in > the sample files > I have. > > Here is a sample mov file with the counter flag set > https://www.dropbox.com/s/5l4fucb9lhq523s/timecode_counter_mode.mov > > before the patch ffmpeg will report the timecode as: > 00:37:11:97 and warns that the timecode framerate is 576000000/1002001 > > after patch: > 14:50:55:02 > > --- > libavformat/mov.c | 13 ------------- > 1 file changed, 13 deletions(-) > > diff --git a/libavformat/mov.c b/libavformat/mov.c > index 3215b53636..f8856a43dd 100644 > --- a/libavformat/mov.c > +++ b/libavformat/mov.c > @@ -2350,19 +2350,6 @@ FF_DISABLE_DEPRECATION_WARNINGS > st->codec->time_base = av_inv_q(st->avg_frame_rate); > FF_ENABLE_DEPRECATION_WARNINGS > #endif > - /* adjust for per frame dur in counter mode */ > - if (tmcd_ctx->tmcd_flags & 0x0008) { > - int timescale = AV_RB32(st->codecpar->extradata + 8); > - int framedur = AV_RB32(st->codecpar->extradata + 12); > - st->avg_frame_rate.num *= timescale; > - st->avg_frame_rate.den *= framedur; > -#if FF_API_LAVF_AVCTX > -FF_DISABLE_DEPRECATION_WARNINGS > - st->codec->time_base.den *= timescale; > - st->codec->time_base.num *= framedur; > -FF_ENABLE_DEPRECATION_WARNINGS > -#endif > - } > if (size > 30) { > uint32_t len = AV_RB32(st->codecpar->extradata + 18); /* > name atom length */ > uint32_t format = AV_RB32(st->codecpar->extradata + 22); > -- > 2.21.1 (Apple Git-122.3) > > Ping
On Sun, Jan 31, 2021 at 1:27 AM Mark Reid <mindmark@gmail.com> wrote: > > > On Sat, Jan 16, 2021 at 5:48 PM <mindmark@gmail.com> wrote: > >> From: Mark Reid <mindmark@gmail.com> >> >> The current behaviour ends up squaring the avg_frame_rate if the conter >> mode flag is set. >> This messes up the timecode calculation, and looks to me as a regression >> that >> seems to have been introduced 428b4aac. >> >> Upon further testing is seems that no special case is need for having the >> counter flag set. >> av_timecode_init appears to handles the timecode correctly, at least in >> the sample files >> I have. >> >> Here is a sample mov file with the counter flag set >> https://www.dropbox.com/s/5l4fucb9lhq523s/timecode_counter_mode.mov >> >> before the patch ffmpeg will report the timecode as: >> 00:37:11:97 and warns that the timecode framerate is 576000000/1002001 >> >> after patch: >> 14:50:55:02 >> >> --- >> libavformat/mov.c | 13 ------------- >> 1 file changed, 13 deletions(-) >> >> diff --git a/libavformat/mov.c b/libavformat/mov.c >> index 3215b53636..f8856a43dd 100644 >> --- a/libavformat/mov.c >> +++ b/libavformat/mov.c >> @@ -2350,19 +2350,6 @@ FF_DISABLE_DEPRECATION_WARNINGS >> st->codec->time_base = av_inv_q(st->avg_frame_rate); >> FF_ENABLE_DEPRECATION_WARNINGS >> #endif >> - /* adjust for per frame dur in counter mode */ >> - if (tmcd_ctx->tmcd_flags & 0x0008) { >> - int timescale = AV_RB32(st->codecpar->extradata + 8); >> - int framedur = AV_RB32(st->codecpar->extradata + 12); >> - st->avg_frame_rate.num *= timescale; >> - st->avg_frame_rate.den *= framedur; >> -#if FF_API_LAVF_AVCTX >> -FF_DISABLE_DEPRECATION_WARNINGS >> - st->codec->time_base.den *= timescale; >> - st->codec->time_base.num *= framedur; >> -FF_ENABLE_DEPRECATION_WARNINGS >> -#endif >> - } >> if (size > 30) { >> uint32_t len = AV_RB32(st->codecpar->extradata + 18); /* >> name atom length */ >> uint32_t format = AV_RB32(st->codecpar->extradata + 22); >> -- >> 2.21.1 (Apple Git-122.3) >> >> > Ping > ping
queued, will push in a bit
diff --git a/libavformat/mov.c b/libavformat/mov.c index 3215b53636..f8856a43dd 100644 --- a/libavformat/mov.c +++ b/libavformat/mov.c @@ -2350,19 +2350,6 @@ FF_DISABLE_DEPRECATION_WARNINGS st->codec->time_base = av_inv_q(st->avg_frame_rate); FF_ENABLE_DEPRECATION_WARNINGS #endif - /* adjust for per frame dur in counter mode */ - if (tmcd_ctx->tmcd_flags & 0x0008) { - int timescale = AV_RB32(st->codecpar->extradata + 8); - int framedur = AV_RB32(st->codecpar->extradata + 12); - st->avg_frame_rate.num *= timescale; - st->avg_frame_rate.den *= framedur; -#if FF_API_LAVF_AVCTX -FF_DISABLE_DEPRECATION_WARNINGS - st->codec->time_base.den *= timescale; - st->codec->time_base.num *= framedur; -FF_ENABLE_DEPRECATION_WARNINGS -#endif - } if (size > 30) { uint32_t len = AV_RB32(st->codecpar->extradata + 18); /* name atom length */ uint32_t format = AV_RB32(st->codecpar->extradata + 22);
From: Mark Reid <mindmark@gmail.com> The current behaviour ends up squaring the avg_frame_rate if the conter mode flag is set. This messes up the timecode calculation, and looks to me as a regression that seems to have been introduced 428b4aac. Upon further testing is seems that no special case is need for having the counter flag set. av_timecode_init appears to handles the timecode correctly, at least in the sample files I have. Here is a sample mov file with the counter flag set https://www.dropbox.com/s/5l4fucb9lhq523s/timecode_counter_mode.mov before the patch ffmpeg will report the timecode as: 00:37:11:97 and warns that the timecode framerate is 576000000/1002001 after patch: 14:50:55:02 --- libavformat/mov.c | 13 ------------- 1 file changed, 13 deletions(-)