Re: [FFmpeg-devel] [PATCH 04/13] lavf: use AV_CODEC_PROP_FIELDS where appropriate

From: Michael Niedermayer <michael@niedermayer.cc>
To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
Subject: Re: [FFmpeg-devel] [PATCH 04/13] lavf: use AV_CODEC_PROP_FIELDS where appropriate
Date: Mon, 15 May 2023 20:59:42 +0200
Message-ID: <20230515185942.GO1391451@pb2> (raw)
In-Reply-To: <168362189020.3843.14417317264187506189@lain.khirnov.net>

[-- Attachment #1.1: Type: text/plain, Size: 2389 bytes --]

On Tue, May 09, 2023 at 10:44:50AM +0200, Anton Khirnov wrote:
> Quoting Michael Niedermayer (2023-05-08 16:15:42)
> > On Sun, May 07, 2023 at 03:32:46PM +0200, Anton Khirnov wrote:
> > > H.264 and mpeg12 parsers need to be adjusted at the same time to stop
> > > using the value of AVCodecContext.ticks_per_frame, because it is not set
> > > correctly unless the codec has been opened. Previously this would result
> > > in both the parser and lavf seeing the same incorrect value, which would
> > > cancel out.
> > > Updating lavf and not the parsers would result in correct value in lavf,
> > > but the wrong one in parsers, which would break some tests.
> > > ---
> > >  libavcodec/h264_parser.c      |  4 ++--
> > >  libavcodec/mpegvideo_parser.c |  2 +-
> > >  libavformat/avformat.c        |  9 ++++++---
> > >  libavformat/demux.c           | 29 +++++++++++++++++++----------
> > >  libavformat/internal.h        |  3 +++
> > >  5 files changed, 31 insertions(+), 16 deletions(-)
> > 
> > Doesnt this sort of change need a major ABI bump ?
> > it sounds like lavc and lavf interdepend here both ways
> 
> No, we do not guarantee bug compatibility.
> 
> Libavformat seeing ticks_per_frame=1 for codecs that set it to 2 upon
> being opened is a bug. Same for the parser.
> 
> It just so happens that libavformat AND its internal parser instance see
> the same incorrect value and this cancels out in cases that are tested
> by FATE (it would break if we had more thorough tests for repeating
> single fields).

This patch seems to change tbr
./ffmpeg -i fate-suite//h264/lossless.h264
Stream #0:0: Video: h264 (High 4:4:4 Predictive), yuv420p(progressive), 640x480, 25 fps, 60 tbr, 1200k tbn
vs.
Stream #0:0: Video: h264 (High 4:4:4 Predictive), yuv420p(progressive), 640x480, 25 fps, 120 tbr, 1200k tbn

with 
./ffmpeg -i fate-suite//h264/lossless.h264  -f framecrc -

The output uses 1/60 thats odd because if every frame can be represented in
1/60 then tbr is 1/60 or more course
OTOH if tbr is finer than 1/60 then not every frame can be represented in 1/60

maybe iam missing something but the new value seems worse and also
not consistent with what ffmpeg actually uses

thx

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Avoid a single point of failure, be that a person or equipment.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
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".