ultravideo/kvazaar

Tiles specification option format

jeanlf opened this issue · 5 comments

jeanlf commented

The syntax for options tiles-width-split and tiles-height-split for non-uniform tiling currently only allows comma-separated list. This breaks option parsing in ffmpeg integration. Would it be possible to allow for both comma and dot ?
eg change in parse_tiles_specification:

current_arg = strchr(current_arg, ',');

with something like

    const char* next_arg = strchr(current_arg, ',');
    if (!next_arg) next_arg = strchr(current_arg, '.');
    current_arg = next_arg;

thanks !

fador commented

Hi,
Yes that is a problem, adding the dot as an alternative sounds like a good idea!
I have to check that it doesn't mess up anything else, but I think I could push a fix tomorrow if nothing else urgent comes up 😁

jeanlf commented

Great ! Might be worth patching for parse_pu_depth_list and parse_slice_specification as well.

Jovasa commented

Does escaping the commas not work for the ffmpeg cmdline?

fador commented

@Jovasa is right, you can actually escape the parameters like this: -kvazaar-params "preset=ultrafast,pu-depth-inter=1-3\,3-3,qp=40,tiles-width-split=128\,256\,512,tiles-height-split=128\,256\,512" and the arguments seem to go to Kvazaar with comma separation.
So maybe we don't need to add extra separator but could be mentioned in documentation somewhere..?

jeanlf commented

Indeed this works, provided that the param value string is quoted (also working with gpac). I'll update my doc accordingly and let you guys decide if an alternate separator helps readability ;)

  • 👍1