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

2022-08-20 Thread Brendan Hack

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.


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

thanks

Brendan


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


[FFmpeg-devel] [PATCH] avfilter/vf_frei0r: set align to 1 for frei0r frames

2022-08-24 Thread Brendan Hack
The frei0r API requires linesize to be width * 4.
Since the align property of ff_default_get_video_buffer2
specifies line alignment, not buffer alignment, the
previous value of 16 breaks this requirement for frames
whose width is a multiple of 8, but not a multiple of 16
as it adds extra padding to ensure line aligment.

Fix Trac ticket #9873
---
 libavfilter/vf_frei0r.c | 9 ++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/libavfilter/vf_frei0r.c b/libavfilter/vf_frei0r.c
index 1e01114b76..58fa2245bf 100644
--- a/libavfilter/vf_frei0r.c
+++ b/libavfilter/vf_frei0r.c
@@ -359,14 +359,17 @@ static int filter_frame(AVFilterLink *inlink, AVFrame *in)
 {
 Frei0rContext *s = inlink->dst->priv;
 AVFilterLink *outlink = inlink->dst->outputs[0];
-AVFrame *out = ff_default_get_video_buffer2(outlink, outlink->w, 
outlink->h, 16);
+/* align parameter is the line alignment, not the buffer alignment.
+ * frei0r expects line size to be width*4 so we want an align of 1
+ * to ensure lines aren't padded out. */
+AVFrame *out = ff_default_get_video_buffer2(outlink, outlink->w, 
outlink->h, 1);
 if (!out)
 goto fail;
 
 av_frame_copy_props(out, in);
 
 if (in->linesize[0] != out->linesize[0]) {
-AVFrame *in2 = ff_default_get_video_buffer2(outlink, outlink->w, 
outlink->h, 16);
+AVFrame *in2 = ff_default_get_video_buffer2(outlink, outlink->w, 
outlink->h, 1);
 if (!in2)
 goto fail;
 av_frame_copy(in2, in);
@@ -481,7 +484,7 @@ static int source_config_props(AVFilterLink *outlink)
 static int source_request_frame(AVFilterLink *outlink)
 {
 Frei0rContext *s = outlink->src->priv;
-AVFrame *frame = ff_default_get_video_buffer2(outlink, outlink->w, 
outlink->h, 16);
+AVFrame *frame = ff_default_get_video_buffer2(outlink, outlink->w, 
outlink->h, 1);
 
 if (!frame)
 return AVERROR(ENOMEM);
-- 
2.25.1

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