[FFmpeg-devel] [PATCH 1/2 v2] avutil: add a Tile Grid API

Anton Khirnov anton at khirnov.net
Mon Jan 22 12:32:12 EET 2024


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


More information about the ffmpeg-devel mailing list