[FFmpeg-devel] [PATCH 02/18] fftools/ffmpeg_filter: refactor setting input timebase

[Michael Niedermayer]

On Sun, Mar 10, 2024 at 07:13:18AM +0100, Anton Khirnov wrote:
> Quoting Michael Niedermayer (2024-03-10 04:36:29)
> > On Fri, Mar 08, 2024 at 06:34:36AM +0100, Anton Khirnov wrote:
> > > Quoting Michael Niedermayer (2024-03-07 21:37:39)
> > > > On Wed, Mar 06, 2024 at 12:03:03PM +0100, Anton Khirnov wrote:
> > > > > Treat it analogously to stream parameters like format/dimensions/etc.
> > > > > This is functionally different from previous code in 2 ways:
> > > > > * for non-CFR video, the frame timebase (set by the decoder) is used
> > > > >   rather than the demuxer timebase
> > > > > * for sub2video, AV_TIME_BASE_Q is used, which is hardcoded by the
> > > > >   subtitle decoding API
> > > > > 
> > > > > These changes should avoid unnecessary and potentially lossy timestamp
> > > > > conversions from decoder timebase into the demuxer one.
> > > > > 
> > > > > Changes the timebases used in sub2video tests.
> > > > > ---
> > > > >  fftools/ffmpeg_filter.c               |  17 ++-
> > > > >  tests/ref/fate/sub2video_basic        | 182 +++++++++++++-------------
> > > > >  tests/ref/fate/sub2video_time_limited |   8 +-
> > > > >  3 files changed, 106 insertions(+), 101 deletions(-)
> > > > 
> > > > breaks:
> > > > 
> > > > ./ffmpeg -i \[a-s\]_full_metal_panic_fumoffu_-_01_-_the_man_from_the_south_-_a_hostage_with_no_compromises__rs2_\[1080p_bd-rip\]\[BBB48A25\].mkv  -filter_complex '[0:s:1]scale=800:600' -t 15 -qscale 2 -y a.avi
> > > > 
> > > 
> > > Use a constant framerate.
> > 
> > why not automatically choose a supported timebase  ?
> > 
> > "[mpeg4 @ 0x55973c869f00] timebase 1/1000000 not supported by MPEG 4 standard, the maximum admitted value for the timebase denominator is 65535"
> 
> Because I don't want ffmpeg CLI to have codec-specific code for a codec
> that's been obsolete for 15+ years. One could also potentially do it
> inside the encoder itself, but it is nontrivial since the computations
> are spread across a number of places in mpeg4videoenc.c and
> mpegvideo_enc.c. And again, it seems like a waste of time - there is no
> reason to encode mpeg4 today.

This is not mpeg4 specific, its just a new additional case that fails

./ffmpeg -i mm-small.mpg test.dv
[dvvideo @ 0x7f868800f100] Found no DV profile for 80x60 yuv420p video. Valid DV profiles are:

IMHO ffmpeg should be able to select supported parameters if the user
indicated thats what he wants

We also do this with pixel formats and not fail and require the user to manually
specify it

thx

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

When the tyrant has disposed of foreign enemies by conquest or treaty, and
there is nothing more to fear from them, then he is always stirring up
some war or other, in order that the people may require a leader. -- Plato
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 195 bytes
Desc: not available
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20240310/952c3c76/attachment.sig>