[FFmpeg-devel] avfilter/vf_tile: increase max tile size

Submitted by Paul B Mahol on Oct. 10, 2017, 7:22 p.m.

Details

Message ID 20171010192223.9115-1-onemda@gmail.com
State New
Headers show

Commit Message

Paul B Mahol Oct. 10, 2017, 7:22 p.m.
Useful when producing tiles of 1xN or Nx1 size, 1024 is too small then.

Signed-off-by: Paul B Mahol <onemda@gmail.com>
---
 libavfilter/vf_tile.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Derek Buitenhuis Oct. 10, 2017, 7:32 p.m.
On 10/10/2017 8:22 PM, Paul B Mahol wrote:
> Useful when producing tiles of 1xN or Nx1 size, 1024 is too small then.
> 
> Signed-off-by: Paul B Mahol <onemda@gmail.com>
> ---
>  libavfilter/vf_tile.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Is there a reason this is hardcoded and not a user option?

- Derek
Paul B Mahol Oct. 10, 2017, 7:39 p.m.
On 10/10/17, Derek Buitenhuis <derek.buitenhuis@gmail.com> wrote:
> On 10/10/2017 8:22 PM, Paul B Mahol wrote:
>> Useful when producing tiles of 1xN or Nx1 size, 1024 is too small then.
>>
>> Signed-off-by: Paul B Mahol <onemda@gmail.com>
>> ---
>>  libavfilter/vf_tile.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> Is there a reason this is hardcoded and not a user option?

One can not set max/min for AV_OPT_TYPE_IMAGE_SIZE.
Derek Buitenhuis Oct. 10, 2017, 7:51 p.m.
On 10/10/2017 8:39 PM, Paul B Mahol wrote:
> One can not set max/min for AV_OPT_TYPE_IMAGE_SIZE.

So use a different type, or implement the option differently?

- Derek
Paul B Mahol Oct. 10, 2017, 7:59 p.m.
On 10/10/17, Derek Buitenhuis <derek.buitenhuis@gmail.com> wrote:
> On 10/10/2017 8:39 PM, Paul B Mahol wrote:
>> One can not set max/min for AV_OPT_TYPE_IMAGE_SIZE.
>
> So use a different type, or implement the option differently?

Not possible, there is option used from start.

Beside its small change. Why do you care at all about lavfi?
Derek Buitenhuis Oct. 10, 2017, 8:24 p.m.
On 10/10/2017 8:59 PM, Paul B Mahol wrote:
> Not possible, there is option used from start.

It's possible to deprecate and add a new one.

> Beside its small change. Why do you care at all about lavfi?

Cool it.

I get it's a simple change, and I'm not blocking it.

However, hardcoding limits is pretty silly/hacky. I suspect there
will be another email N months form now to bump it again, so why
not just fix it properly instead of hacking it?

- Derek
Paul B Mahol Oct. 11, 2017, 7:46 a.m.
On 10/10/17, Derek Buitenhuis <derek.buitenhuis@gmail.com> wrote:
> On 10/10/2017 8:59 PM, Paul B Mahol wrote:
>> Not possible, there is option used from start.
>
> It's possible to deprecate and add a new one.
>
>> Beside its small change. Why do you care at all about lavfi?
>
> Cool it.
>
> I get it's a simple change, and I'm not blocking it.
>
> However, hardcoding limits is pretty silly/hacky. I suspect there
> will be another email N months form now to bump it again, so why
> not just fix it properly instead of hacking it?

Check could be removed, and add check for overflows.
Derek Buitenhuis Oct. 11, 2017, 5:59 p.m.
On 10/11/2017 8:46 AM, Paul B Mahol wrote:
> Check could be removed, and add check for overflows.

Seems slightly more reasonable to me, I think...

- Derek
Nicolas George Oct. 11, 2017, 6:28 p.m.
Le decadi 20 vendémiaire, an CCXXVI, Derek Buitenhuis a écrit :
> Seems slightly more reasonable to me, I think...

I am fine with any solution that is safe but does not increase the
complexity of the code too much.

Regards,

Patch hide | download patch | download mbox

diff --git a/libavfilter/vf_tile.c b/libavfilter/vf_tile.c
index 87e0b940cf..d5fe4f255e 100644
--- a/libavfilter/vf_tile.c
+++ b/libavfilter/vf_tile.c
@@ -44,7 +44,7 @@  typedef struct TileContext {
     uint8_t rgba_color[4];
 } TileContext;
 
-#define REASONABLE_SIZE 1024
+#define REASONABLE_SIZE 4096
 
 #define OFFSET(x) offsetof(TileContext, x)
 #define FLAGS AV_OPT_FLAG_VIDEO_PARAM|AV_OPT_FLAG_FILTERING_PARAM