[FFmpeg-devel] framepool width and potential patch for bug 9873

[Timo Rothenpieler]

On 20.08.2022 13:21, Brendan Hack wrote:
> Hi,
> 
> I reported https://trac.ffmpeg.org/ticket/9873 last week and have since 
> been digging into the code to see if I can get a patch together. I've 
> found the underlying issue in ff_frame_pool_video_init at line 77 of 
> libavfilter/framepool.c:
> 
>          ret = av_image_fill_linesizes(pool->linesize, pool->format,
>                                        FFALIGN(pool->width, align));
> 
> When creating frames for frei0r filtering an align value of 16 gets 
> passed in. This works fine for widths like 400 where FFALIGN just 
> returns 400 since it's a multiple of 16. But for values like 1800 it 
> pushes the width up to 1808 which then generates linesizes for that 
> width which breaks the frei0r requirement that the stride matches width 
> * 4 since width is now too large.

Not sure what you mean. The code you quoted does not touch width. It 
just bumps up the linesize, to align the start of every line on the 
desired align value.

> Initially I put together a patch that adds an extra parameter to 
> ff_frame_pool_video_init that tells it just to use the width directly 
> and not try to align it. This seemed a bit awkward though, especially 

That parameter already exists. It's called the align.

> when I tried to figure our why the code is even trying to make the width 
> a multiple of the align value (which is effectively what it's doing) to 
> start with. I can't make sense of why it's calling FFALIGN there at all 
> and checking the commit history didn't provide any hints. If anything I 
> would have thought that you would want to increase the width so that the 
> final line size would be aligned, EG: FFALIGN(pool->width*4, align)/4, 
> rather than the width itself.
> 
> In any case I have patches that work both ways, either an extra flag to 
> not do the aligned width or the FFALIGN(pool->width*4, align)/4 fix. The 
> former I'm confident of but am not so sure about the latter since I 
> don't understand why it's there to start with, nor whether it may break 

You mean you don't understand the concept of aligning lines?
The whole point is that a lot of hardware instructions require the data 
they work on to be aligned properly. Hence the linesize needs to be 
larger than width, to make sure every line starts well aligned.

> other filters. The fate tests all pass with it though on my system. It 
> will certainly work with frei0r though since it requires width to be a 
> multiple of 8 which will always give a line size that's aligned to 16.
> 
> Any advice on which one I should submit would be great.

If frei0r tells it to align it to 16 bytes, but then fails because it 
expects it to be packed without gaps, it's just passing the wrong 
alignment, and should pass 1.