Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
 help / color / mirror / Atom feed
From: Niklas Haas <ffmpeg@haasn.xyz>
To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
Subject: Re: [FFmpeg-devel] [PATCH v2 16/17] fftools/ffmpeg_filter: propagate codec yuv metadata to filters
Date: Wed, 10 Apr 2024 15:10:14 +0200
Message-ID: <20240410151014.GB48658@haasn.xyz> (raw)
In-Reply-To: <20240410125915.GN6420@pb2>

On Wed, 10 Apr 2024 14:59:15 +0200 Michael Niedermayer <michael@niedermayer.cc> wrote:
> On Wed, Apr 10, 2024 at 12:29:06PM +0200, Niklas Haas wrote:
> > On Wed, 10 Apr 2024 03:25:45 +0200 Michael Niedermayer <michael@niedermayer.cc> wrote:
> > > On Mon, Apr 08, 2024 at 02:57:20PM +0200, Niklas Haas wrote:
> > > > From: Niklas Haas <git@haasn.dev>
> > > > 
> > > > To convert between color spaces/ranges, if needed by the codec
> > > > properties.
> > > > ---
> > > >  fftools/ffmpeg_filter.c | 34 ++++++++++++++++++++++++++++++++++
> > > >  1 file changed, 34 insertions(+)
> > > 
> > > I presume this is intended to change some cases
> > > iam asking because it does
> > > for example, this one
> > > -i aletrek.mkv -t 1 -bitexact  /tmp/file-aletrek-palette.mkv
> > > has the output file change slightly
> > > 
> > > https://trac.ffmpeg.org/attachment/ticket/5071/aletrek.mkv
> > > 
> > > also given fate does not change, it would make sense to add a testcase
> > > to fate that does cover this
> > 
> > Two notes:
> > 
> > 1. The only difference between the `master` behavior and the new
> >    behavior is that the file is marked as limited range instead of as
> >    unspecified. However, this is the correct tagging, as the actual
> >    output *is* limited range.
> > 
> > 2. While not *broken* per se, this is actually still a bug - in this
> >    case, since the input is basically full range, we should actually try
> >    and output full range by default.
> > 
> > The reason it doesn't output full range here is because a consequence of
> > the fact that format reduction happens *before* the logic in
> > pick_format() fixes all non-YUV links to be tagged as PC/RGB-only. So
> > the format reduction logic just sees that vf_scale can output any range
> > in this scenario (true) and picks TV range output as the default,
> > resulting in a conversion from the PC range input to a TV range output.
> > 
> > The easiest solution would be to not blindly pick the first here, but to
> > instead try and pick a colorspace and range matching the input (if one
> > exists). But this may still break in more complicated scenarios where
> > the dependence on the forced format spans several filters.
> > 
> > The more correct solution would probably be to explicitly reduce only
> > the format first (going through all the steps) and then negotiate
> > everything that depends on the format in an entirely separate step.
> > 
> > I'll see if I can do something about this situation, though it's
> > ultimately not a high priority as it's not a regression compared to the
> > status quo - just something that we could definitely improve.
> 
> I have the feeling the colorspace negotiation has become a bit messy
> IIRC The way it was intended to work originally was to have a
> arbitrary filtergraph and then randomly simplify/merge bits of the
> filtergraph before picking "colorspaces" for each part.
> Then repeat this whole process a few times starting from the unsimplified
> graph. Then pick the best and that would be within a constant factor
> of the optimal solution for any arbitrary complex graph.
> 
> Somehow this was never implemented as things worked okesich with just
> doing a single pass instead of multiple and then picking the best.
> 
> I dont know how much this applies here now but i thought bringing
> up the original intended design was a good idea. Because it seems
> every problem in the negotiation is solved by adjusting the single pass
> steps or its orders while really it should have been multiple randomized
> passes.
> Maybe we never have to deal with complex enough filtergraphs where a single
> pass cant be made to work well and surely this here is not a complex one
> at all.
> But wanted to bring this up anyway because i think many people forgot
> or didnt even know the original idea was to do multiple passes so as to
> handle any arbitrary complex graph well.

I think a greedy algorithm (not requiring juggling multiple random
passes) can still work, just that we need to change the logic from:

  do
      // step 1
  while (progress)
  
  do
      // step 2
  while (progress)

  ...

to:

  do {
      // step 1
      // step 2
      ...
  } while (any_progress)

In any case, the realization is clear that we can no longer rely on
negotiating everything at the same time, as we now have a fundamental
interdependency of one negotiated component on another. (And I can
easily see more such dependencies being added in the future)

> 
> thx
> 
> [...]
> -- 
> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
> 
> If you think the mosad wants you dead since a long time then you are either
> wrong or dead since a long time.
> _______________________________________________
> 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".
_______________________________________________
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".

  reply	other threads:[~2024-04-10 13:10 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-04-08 12:57 [FFmpeg-devel] [PATCH v2 00/17] Add avcodec_get_supported_config() Niklas Haas
2024-04-08 12:57 ` [FFmpeg-devel] [PATCH v2 01/17] avcodec/internal: add FFCodec.color_ranges Niklas Haas
2024-04-08 12:57 ` [FFmpeg-devel] [PATCH v2 02/17] avcodec: add avcodec_get_supported_config() Niklas Haas
2024-08-01  1:48   ` Andreas Rheinhardt
2024-04-08 12:57 ` [FFmpeg-devel] [PATCH v2 03/17] avcodec/encode: switch to avcodec_get_supported_config() Niklas Haas
2024-04-08 12:57 ` [FFmpeg-devel] [PATCH v2 04/17] avcodec/allcodecs: add backcompat for new config API Niklas Haas
2024-04-08 12:57 ` [FFmpeg-devel] [PATCH v2 05/17] avcodec/libx265: switch to get_supported_config() Niklas Haas
2024-04-08 12:57 ` [FFmpeg-devel] [PATCH v2 06/17] avcodec/libvpxenc: " Niklas Haas
2024-04-08 12:57 ` [FFmpeg-devel] [PATCH v2 07/17] avcodec/libaomenc: " Niklas Haas
2024-04-08 12:57 ` [FFmpeg-devel] [PATCH v2 08/17] avcodec/mjpegenc: " Niklas Haas
2024-04-08 12:57 ` [FFmpeg-devel] [PATCH v2 09/17] avcodec/codec_internal: nuke init_static_data() Niklas Haas
2024-04-09 13:00   ` Niklas Haas
2024-04-08 12:57 ` [FFmpeg-devel] [PATCH v2 10/17] fftools/opt_common: switch to avcodec_get_supported_config() Niklas Haas
2024-04-08 12:57 ` [FFmpeg-devel] [PATCH v2 11/17] fftools: drop unused/hacky macros Niklas Haas
2024-04-08 12:57 ` [FFmpeg-devel] [PATCH v2 12/17] fftools/ffmpeg_mux_init: switch to avcodec_get_supported_config() Niklas Haas
2024-04-08 12:57 ` [FFmpeg-devel] [PATCH v2 13/17] fftools/ffmpeg_filter: set strict_std_compliance Niklas Haas
2024-04-09 13:01   ` Niklas Haas
2024-04-08 12:57 ` [FFmpeg-devel] [PATCH v2 14/17] fftools/ffmpeg_filter: simplify choose_pix_fmts Niklas Haas
2024-04-10  1:13   ` Michael Niedermayer
2024-04-10 11:23     ` Niklas Haas
2024-04-08 12:57 ` [FFmpeg-devel] [PATCH v2 15/17] fftools/ffmpeg_filter: switch to avcodec_get_supported_config() Niklas Haas
2024-04-08 12:57 ` [FFmpeg-devel] [PATCH v2 16/17] fftools/ffmpeg_filter: propagate codec yuv metadata to filters Niklas Haas
2024-04-10  1:25   ` Michael Niedermayer
2024-04-10 10:29     ` Niklas Haas
2024-04-10 10:31       ` Niklas Haas
2024-04-10 12:59       ` Michael Niedermayer
2024-04-10 13:10         ` Niklas Haas [this message]
2024-04-10 13:12           ` Nicolas George
2024-04-11  7:45             ` Paul B Mahol
2024-04-08 12:57 ` [FFmpeg-devel] [PATCH v2 17/17] fftools/ffmpeg_filter: remove YUVJ hack Niklas Haas

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240410151014.GB48658@haasn.xyz \
    --to=ffmpeg@haasn.xyz \
    --cc=ffmpeg-devel@ffmpeg.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \
		ffmpegdev@gitmailbox.com
	public-inbox-index ffmpegdev

Example config snippet for mirrors.


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git