| Message ID | 20240823081413.531466-2-nicolas.gaullier@cji.paris |
|---|---|
| State | New |
| Headers | show |
| Series | avformat/mxfenc: Fix guess frame_rate | expand |
| Context | Check | Description |
|---|---|---|
| andriy/make_x86 | success | Make finished |
| andriy/make_fate_x86 | success | Make fate finished |
fre 2024-08-23 klockan 10:14 +0200 skrev Nicolas Gaullier: > The time_base was a bad guess. > > Currently, fate-time_base test data assumed that overriding the input > time_base would affect the frame_rate, but this behaviour is not > documented, so just fix the fate data now that this is fixed. > > Fix regression since 10185e2d4c1e9839bc58a1d6a63c861677b13fd0: > previously, when streamcopying, the time_base was guessed from the > frame_rate considering it is often constant, so guessing the > frame_rate > back from the time_base was often not a problem. > > To reproduce: > ffmpeg -i fate-suite/mpeg2/dvd_still_frame.vob -an -c copy out.mxf > > Signed-off-by: Nicolas Gaullier <nicolas.gaullier@cji.paris> > --- > libavformat/mxfenc.c | 6 +++++- > tests/ref/fate/time_base | 2 +- > 2 files changed, 6 insertions(+), 2 deletions(-) > > diff --git a/libavformat/mxfenc.c b/libavformat/mxfenc.c > index 4ac6a2d715..57f4c674f3 100644 > --- a/libavformat/mxfenc.c > +++ b/libavformat/mxfenc.c > @@ -2894,8 +2894,12 @@ static int mxf_init(AVFormatContext *s) > > if (st->codecpar->codec_type == AVMEDIA_TYPE_VIDEO) { > const AVPixFmtDescriptor *pix_desc = > av_pix_fmt_desc_get(st->codecpar->format); > - // TODO: should be avg_frame_rate > AVRational tbc = st->time_base; > + if (st->avg_frame_rate.num > 0 && st->avg_frame_rate.den > > 0) > + tbc = av_inv_q(st->avg_frame_rate); > + else if (st->r_frame_rate.num > 0 && st- > >r_frame_rate.den > 0) > + tbc = av_inv_q(st->r_frame_rate); This is probably fine for now, but it should be said that frame rate and EditRate are not necessarily the same. We might want an explicit EditRate option. But we can wait for users to actually request that feature /Tomas
>De : ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> De la part de Tomas Härdin >Envoyé : lundi 26 août 2024 11:52 > >This is probably fine for now, but it should be said that frame rate and EditRate are not necessarily the same. We might want an explicit EditRate option. But we can wait for users to actually request that feature > >/Tomas It seems nobody objected, can this patch be applied ? There is also a second patch in this serie for the corresponding fate tests "cleanup"; this is just in my mind to avoid confusing command lines arguments that don't take effect. Can you review it ? Thank you Nicolas
diff --git a/libavformat/mxfenc.c b/libavformat/mxfenc.c index 4ac6a2d715..57f4c674f3 100644 --- a/libavformat/mxfenc.c +++ b/libavformat/mxfenc.c @@ -2894,8 +2894,12 @@ static int mxf_init(AVFormatContext *s) if (st->codecpar->codec_type == AVMEDIA_TYPE_VIDEO) { const AVPixFmtDescriptor *pix_desc = av_pix_fmt_desc_get(st->codecpar->format); - // TODO: should be avg_frame_rate AVRational tbc = st->time_base; + if (st->avg_frame_rate.num > 0 && st->avg_frame_rate.den > 0) + tbc = av_inv_q(st->avg_frame_rate); + else if (st->r_frame_rate.num > 0 && st->r_frame_rate.den > 0) + tbc = av_inv_q(st->r_frame_rate); + // Default component depth to 8 sc->component_depth = 8; sc->h_chroma_sub_sample = 2; diff --git a/tests/ref/fate/time_base b/tests/ref/fate/time_base index fd6cac53fc..23875d1fb8 100644 --- a/tests/ref/fate/time_base +++ b/tests/ref/fate/time_base @@ -1 +1 @@ -d408aba82d62a90ed7f46a1999b014f1 +b28d4ca13029fdc80a114b56467be9d7
The time_base was a bad guess. Currently, fate-time_base test data assumed that overriding the input time_base would affect the frame_rate, but this behaviour is not documented, so just fix the fate data now that this is fixed. Fix regression since 10185e2d4c1e9839bc58a1d6a63c861677b13fd0: previously, when streamcopying, the time_base was guessed from the frame_rate considering it is often constant, so guessing the frame_rate back from the time_base was often not a problem. To reproduce: ffmpeg -i fate-suite/mpeg2/dvd_still_frame.vob -an -c copy out.mxf Signed-off-by: Nicolas Gaullier <nicolas.gaullier@cji.paris> --- libavformat/mxfenc.c | 6 +++++- tests/ref/fate/time_base | 2 +- 2 files changed, 6 insertions(+), 2 deletions(-)