Quoting James Almer (2024-01-21 19:38:50) > On 1/21/2024 3:29 PM, Anton Khirnov wrote: > > Quoting James Almer (2024-01-21 18:47:43) > >> On 1/21/2024 2:29 PM, Anton Khirnov wrote: > >>> 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? > >> > >> 1. It needs to be usable as a Stream Group type, so a struct is > >> required. Said struct needs an allocator unless we want to have its size > >> be part of the ABI. I can remove the free function, but then the caller > >> needs to manually free any internal data. > > > > If the struct lives in lavf and is always allocated as a part of > > AVStreamGroup then you don't need a public constructor/destructor and > > can still extend the struct. > > Yes, but that would be the case if it's only meant to be allocated by > AVStreamGroup and nothing else.
That is the case right now, no? If that ever changes then the constructor can be added. > > > >> 2. We need tile dimensions (Width and height) plus row and column count, > >> which give you the final size of the grid, then offsets x and y to get > >> the actual image within the grid meant for presentation. > >> 3. I want to support uniform tiles as well as variable tile dimensions, > >> hence multiple tile types. The latter currently has no use case, but > >> eventually might. I can if you prefer not include said type at first, > >> but i want to keep the union in place so it and other extensions can be > >> added. > >> 4. It's in lavu because its meant to be generic. It can also be used to > >> transport tiling and cropping information as stream and packet side > >> data, which can't depend on something defined in lavf. > > > > When would you have tiling information associated with a specific > > stream? > > Can't think of an example for tiling, but i can for cropping. If you > insist on not reusing this for non-HEIF cropping usage in mp4/matroska, > then ok, I'll move it to lavf. 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. > > > >> 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. That does not mean you actually have to store it that way - the struct could be a list of N rectangles logically, while actually being represented more efficiently (in the same way a channel layout is always logically a list of channels, even though it's often represented by an uint64 rather than a malloced array). -- 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-requ...@ffmpeg.org with subject "unsubscribe".
