Hi, On Thu, Jun 29, 2023 at 5:44 AM Anton Khirnov <an...@khirnov.net> wrote:
> Quoting Dai, Jianhui J (2023-06-29 08:03:18) > > > > > > > -----Original Message----- > > > From: ffmpeg-devel <ffmpeg-devel-boun...@ffmpeg.org> On Behalf Of > > > Anton Khirnov > > > Sent: Wednesday, June 28, 2023 11:25 PM > > > To: ffmpeg-devel@ffmpeg.org > > > Subject: Re: [FFmpeg-devel] [PATCH v2] avformat/ivfenc: Set the > "number of > > > frames" in IVF header > > > > > > Quoting Dai, Jianhui J (2023-06-05 02:53:35) > > > > diff --git a/libavformat/ivfdec.c b/libavformat/ivfdec.c index > > > > 511f2387ed..01012db948 100644 > > > > --- a/libavformat/ivfdec.c > > > > +++ b/libavformat/ivfdec.c > > > > @@ -53,6 +53,7 @@ static int read_header(AVFormatContext *s) > > > > st->codecpar->height = avio_rl16(s->pb); > > > > time_base.den = avio_rl32(s->pb); > > > > time_base.num = avio_rl32(s->pb); > > > > + // Infer duration from "number of frames". > > > > st->duration = avio_rl32(s->pb); > > > > > > This should be setting st->nb_frames then rather than duration. > > > And the muxer should be using that field as well instead of its custom > version. > > > > ACK. > > Do you suggest letting `duration` unset? > > It is interesting that the 'duration' is often right in this way, if > > the time_base.den/time_base.num == fps which is the popular > > configuration. > > Yes, but AFAIU there is no way to tell whether the file is CFR, so then > the value would be unreliable. So I'd prefer to leave it unset. > I see this discussion now... I don't think I agree with the above. First of all, IVF has two fields there (it seems): duration, and n_frames. We have the ability to set both in the muxer, and therefore should set both. Setting only once was silly, but changing it to set only the other is not better. Similarly, the demuxer should set both. I think it's particularly bad that we change a muxer that has been setting a particular field for years (and the accompanying demuxer code to the the AVStream member) to suddenly not set that field/member anymore. This is technically a regression. Ronald _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
