Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
 help / color / mirror / Atom feed
From: Anton Khirnov <anton@khirnov.net>
To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
Subject: Re: [FFmpeg-devel] [PATCH 1/2 v2] avutil: add a Tile Grid API
Date: Mon, 22 Jan 2024 11:32:12 +0100
Message-ID: <170591953206.8914.8104636503477555043@lain.khirnov.net> (raw)
In-Reply-To: <8bd854a2-b10d-479b-8a21-0e2f506db08b@gmail.com>

Quoting James Almer (2024-01-21 20:29:58)
> On 1/21/2024 4:02 PM, Anton Khirnov wrote:
> > I still don't see why should it be a good idea to use this struct for
> > generic container cropping. It feels very much like a hammer in search
> > of a nail.
> 
> Because once we support container cropping, we will be defining a 
> stream/packet side data type that will contain a subset of the fields 
> from this struct.

AVCodecParameters is a subset of AVCodecContext. By this logic we should
use AVCodecContext everywhere instead of AVCodecParameters.

There are also issues with storing it in packet side data since it
can contain pointers to malloced data.

> If we reuse this struct, we can export a clap box as an AVTileGrid (Or i 
> can rename it to AVImageGrid, and tile to subrectangle) either as the 
> stream group tile grid specific parameters if HEIF, or as stream side 
> data otherwise.

How does that make the API better? It seems highly counterintuitive to
me to export container cropping information in a struct designed for
tiling.

> > 
> >>>
> >>>> And what do you mean with not supporting describing arbitrary
> >>>> partitioning? Isn't that what variable tile dimensions achieve?
> >>>
> >>> IIUC your tiling scheme still assumes that the partitioning is by rows
> >>> and columns. A completely generic partitioning could be irregular.
> >>
> >> A new tile type that doesn't define rows and columns can be added if
> >> needed. But the current variable tile type can support things like grids
> >> of two rows and two columns where the second row is effectively a single
> >> tile, simply by setting the second tile in said row as having a width of 0.
> > 
> > The problem I see here is that every consumer of this struct then has to
> > explicitly support every type, and adding a new type requires updating
> > all callers. This seems unnecessary when "list of N rectangles" covers
> > all possible partitionings.
> 
> Well, the variable type supports a list of N rectangles where each 
> rectangle has arbitrary dimensions, and you can do things like having 
> three tiles/rectangles that together still form a rectangle, while 
> defining row and column count. So i don't personally see the need for a 
> new type to begin with.

I don't see how is that supposed to work. E.g. consider the following
partitioning:
┌─┬────┬─┐
│ │    ├─┤
├─┤    │ │
│ ├────┤ │
└─┴────┴─┘

How would you represent it in this API?

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

  parent reply	other threads:[~2024-01-22 10:32 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-01-20 22:04 James Almer
2024-01-20 22:04 ` [FFmpeg-devel] [PATCH 2/2 v2] avformat: add a Tile Grid stream group type James Almer
2024-01-21  6:27 ` [FFmpeg-devel] [PATCH 1/2 v2] avutil: add a Tile Grid API Anton Khirnov
2024-01-21 12:06   ` James Almer
2024-01-21 17:29     ` Anton Khirnov
2024-01-21 17:47       ` James Almer
2024-01-21 18:29         ` Anton Khirnov
2024-01-21 18:38           ` James Almer
2024-01-21 19:02             ` Anton Khirnov
2024-01-21 19:29               ` James Almer
2024-01-21 21:03                 ` James Almer
2024-01-22 10:38                   ` Anton Khirnov
2024-01-22 12:12                     ` James Almer
2024-01-22 10:32                 ` Anton Khirnov [this message]
2024-01-22 11:59                   ` James Almer
2024-01-25 17:13                     ` Anton Khirnov
2024-01-25 17:31                       ` James Almer
2024-01-22 12:08                   ` James Almer
2024-01-25 17:17                     ` Anton Khirnov
2024-01-25 17:26                       ` James Almer

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=170591953206.8914.8104636503477555043@lain.khirnov.net \
    --to=anton@khirnov.net \
    --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