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

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: Sun, 21 Jan 2024 18:29:20 +0100
Message-ID: <170585816018.8914.1005492749396712320@lain.khirnov.net> (raw)
In-Reply-To: <d9ed4d45-fb0c-43a5-931a-844a3f88825c@gmail.com>

Quoting James Almer (2024-01-21 13:06:28)
> On 1/21/2024 3:27 AM, Anton Khirnov wrote:
> > Quoting James Almer (2024-01-20 23:04:06)
> >> This includes a struct and helpers. It will be used to support container level
> >> cropping and tiled image formats, but should be generic enough for general
> >> usage.
> >>
> >> Signed-off-by: James Almer <jamrial@gmail.com>
> >> ---
> >> Extended to include fields used for cropping. Should make the struct reusable
> >> even for non tiled images, e.g. setting both rows and tiles to 1, in which case
> >> tile width and height would become analogous to coded_{witdh,height}.
> > 
> > But why? What does cropping have to do with tiling? What advantage is
> > there to handling them in one struct?
> 
> The struct does not need to be used for non tiled image scenarios, but 
> could if we decide we don't want to add another struct that would only 
> contain a subset of the fields present here.
> 
> As to why said fields here present here, HEIF may use a clap box to 
> define cropping for the final image, not for the tiles. This needs to be 
> propagated, and the previous version of this API, which only defined 
> cropping from right and bottom edges if output dimensions were smaller 
> than the grid (standard case for tiled heif with no clap box), was not 
> enough. Hence this change.
> 
> I can rename this struct to Image Grid or something else, which might 
> make it feel less awkward if we decide to reuse it. We still need to 
> propagate container cropping from clap boxes and from Matroska elements 
> after all.

Honestly this whole new API strikes me as massively overthinking it. All
you should need to describe an arbitrary partition of an image into
sub-rectangles is an array of (x, y, width, height). Instead you're
proposing a new public header, struct, three functions, multiple "tile
types", and if I'm not mistaken it still cannot describe an arbitrary
partitioning. Plus it's in libavutil for some reason, even though
libavformat seems to be the only intended user.

Is all this complexity really warranted?

-- 
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".