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

[Michael Niedermayer]

On Sun, Mar 10, 2024 at 11:49:12PM +0100, Anton Khirnov wrote:
> Quoting James Almer (2024-03-10 23:29:27)
> > On 3/10/2024 7:24 PM, Anton Khirnov wrote:
> > > Quoting Michael Niedermayer (2024-03-10 20:21:47)
> > >> 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
> > > 
> > > The case you reported is mpeg4 specific.
> > > 
> > >> ./ffmpeg -i mm-small.mpg test.dv
> > >> [dvvideo @ 0x7f868800f100] Found no DV profile for 80x60 yuv420p video. Valid DV profiles are:
> > > 
> > > There is no mechanism for an encoder to export supported time bases.
> > 
> > Could it be added as an extension to AVProfile, or AVCodec?
> 
> The two cases are actually pretty different:
> * mpeg4 has a constraint on the range of timebases, and actually does
>   some perverted computations with the timestamps
> * DV just needs your video to be CFR, with a list of supported
>   framerates; dvenc should probably read AVCodecContext.framerate
>   instead of time_base
> 
> But most importantly, is there an actual current use case for either of
> those encoders? They have both been obsolete for close to two decades.
> It seems silly to add new API that won't actually be useful to anyone.

iam not sugesting to add API specific to mpeg4, rather that maybe
it can be done as part of some more generic solution

thx

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

Good people do not need laws to tell them to act responsibly, while bad
people will find a way around the laws. -- 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/20240311/6bcad1ce/attachment.sig>