On 1/22/2024 7:32 AM, Anton Khirnov wrote:
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?

That's two rows and three columns. Lets assume the smallest rectangle there is 1x1:

1x2 2x3 1x1
1x2 2x1 1x3

Meaning

tile_width[] = { 1, 2, 1, 1, 2, 1 };
tile_height[] = { 2, 3, 1, 2, 1, 3 };

As long as the sum of widths on every row and the sum of heights on every column is the same (To ensure you get a rectangle), it can be represented.

If what you're trying to say is "What about offsets?", they can be inferred based on dimensions and position of previous tiles within the grid. I don't think adding yet another array to store offsets is worth it. But maybe a helper function that will build it?
_______________________________________________
ffmpeg-devel mailing list
[email protected]
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
[email protected] with subject "unsubscribe".

Reply via email to