[x265] [PATCH 1 of 2] param2string: increase buffer size, do not store file names

Mon Jan 15 21:58:36 CET 2018

W dniu 15.01.2018 o 20:14, Ashok Kumar Mishra pisze:
> 
> On Sat, Jan 13, 2018 at 5:23 AM, Ma0 <mateuszb at poczta.onet.pl <mailto:mateuszb at poczta.onet.pl>> wrote:
> 
>     # HG changeset patch
>     # User Ma0 <mateuszb at poczta.onet.pl <mailto:mateuszb at poczta.onet.pl>>
>     # Date 1515799544 -3600
>     #      Sat Jan 13 00:25:44 2018 +0100
>     # Node ID 6f4c949761c446334245d72204a3cd7345b36a9f
>     # Parent  2f3c4158cf3553030920708271bc43cdc79932a3
>     param2string: increase buffer size, do not store file names
> 
>     diff -r 2f3c4158cf35 -r 6f4c949761c4 source/common/param.cpp
>     --- a/source/common/param.cpp   Thu Jan 04 12:37:01 2018 +0530
>     +++ b/source/common/param.cpp   Sat Jan 13 00:25:44 2018 +0100
>     @@ -1530,8 +1530,14 @@
>      char *x265_param2string(x265_param* p, int padx, int pady)
>      {
>          char *buf, *s;
>     +    int bufSize = 4000 + p->rc.zoneCount * 64;
> 
> 
> bufSize can be replaced with macro MAXPARAMSIZE. 
> 
> 
>     -    buf = s = X265_MALLOC(char, MAXPARAMSIZE);
>     +    if (p->numaPools)
>     +        bufSize += strlen(p->numaPools);
>     +    if (p->masteringDisplayColorVolume)
>     +        bufSize += strlen(p->masteringDisplayColorVolume);
> 
> 
> We have other string parameters like csv file name, analysis save and load etc., why adding string length of only two parameters in buffer size?

In this patch I've removed all file names -- csv, analysis-save and analysis-load 
(for privacy reason). There are only 'csv', 'analysis-save' or 'analysis-load' entries
if these options are active (without file names).

There are left two %s in sprintf -- for p->numaPools and p->masteringDisplayColorVolume.
For example if you execute:
x265 720p50_parkrun_ter.y4m w.hevc --pools 15,-------------------------------------------------------------

the file header will be
x265 (build 151) - 2.6+27-2f3c4158cf35:[Windows][GCC 7.2.1][64 bit] 8bit+10bit+12bit - H.265/HEVC codec - Copyright 2013-2017 (c) Multicoreware, Inc - http://x265.org - options: cpuid=1173503 frame-threads=3 numa-pools=15,------------------------------------------------------------- wpp [...]

There is no problem to overfill any constant size buffer.

There is also loop for zones -- another unpredictable text size.

So I assume: 4000 bytes for normal options + 16 bytes per zone + length of all remaining %s.

The problem with too small buffer is real -- example command from doom9 forum:
ffmpeg -i ../tearsofsteel-4k.y4m -v warning -strict -1 -pix_fmt yuv420p10 -f yuv4mpegpipe - | x265 --y4m --pool "15,-" --preset slower --tune ssim --profile main10 --level-idc 5.1 --crf 19 --cbqpoffs -2 --crqpoffs -2 --rc-lookahead 60 --me 3 --subme 5 --merange 25 --b-adapt 2 --bframes 9 --ref 5 --aq-mode 3 --aq-strength 0.9 --qcomp 0.70 --colorprim bt2020 --transfer smpte2084 --colormatrix bt2020nc --limit-modes --limit-refs 2 --output-depth 10 --rd 4 --rdoq-level 2 --psy-rd 2.0 --psy-rdoq 8 --ssim-rd --deblock -1 --no-sao --no-open-gop --no-rect --no-amp --no-strong-intra-smoothing --no-rskip --tu-intra-depth 2 --tu-inter-depth 2   --ctu 32 --limit-tu 3 --max-tu-size 32 --qg-size 32 --b-intra --weightb --rdpenalty 1 --keyint 360 --min-keyint 1 --vbv-bufsize 100000 --vbv-maxrate 100000 --hdr-opt --aud --hrd --chromaloc 2 --max-cll "552,190" --master-display "G(13250,34500)B(7500,3000)R(34000,16000)WP(15635,16450)L(40000000,50)" --log-level 2 --csv "D:\x265-workspace\output\harrypotter2001\00001.1080.csv" --output "D:\x265-workspace\output\harrypotter2001\00001.1080.hevc" -

the file header (if x265 doesn't hang) is:
x265 (build 151) - 2.6+27-2f3c4158cf35:[Windows][GCC 7.2.1][64 bit] 10bit - H.265/HEVC codec - Copyright 2013-2017 (c) Multicoreware, Inc - http://x265.org - options: cpuid=1173503 frame-threads=3 numa-pools=15,- wpp no-pmode no-pme no-psnr no-ssim log-level=2 csvfn=D:\x265-workspace\output\harrypotter2001\00001.1080.csv csv-log-level=0 bitdepth=10 input-csp=1 fps=24/1 input-res=4096x1714 interlace=0 total-frames=0 level-idc=51 high-tier=1 uhd-bd=0 ref=5 no-allow-non-conformance repeat-headers annexb aud hrd info hash=0 no-temporal-layers no-open-gop min-keyint=1 keyint=360 gop-lookahead=0 bframes=9 b-adapt=2 b-pyramid bframe-bias=0 rc-lookahead=60 lookahead-slices=4 scenecut=40 radl=0 no-intra-refresh ctu=32 min-cu-size=8 no-rect no-amp max-tu-size=32 tu-inter-depth=2 tu-intra-depth=2 limit-tu=3 rdoq-level=2 dynamic-rd=0.00 ssim-rd signhide no-tskip nr-intra=0 nr-inter=0 no-constrained-intra no-strong-intra-smoothing max-merge=3 limit-refs=2 limit-modes me=3 subme=5 merange=25 temporal-mvp weightp weightb no-analyze-src-pics deblock=-1:-1 no-sao no-sao-non-deblock rd=4 no-early-skip no-rskip no-fast-intra no-tskip-fast no-cu-lossless b-intra no-splitrd-skip rdpenalty=1 psy-rd=0.00 psy-rdoq=8.00 no-rd-refine no-lossless cbqpoffs=-2 crqpoffs=-2 rc=crf crf=19.0 qcomp=0.70 qpstep=4 stats-write=0 stats-read=0 vbv-maxrate=100000 vbv-bufsize=100000 vbv-init=0.9 crf-max=0.0 crf-min=0.0 ipratio=1.40 pbratio=1.30 aq-mode=3 aq-strength=0.90 cutree zone-count=0 no-strict-cbr qg-size=32 no-rc-grain qpmax=69 qpmin=0 no-const-vbv sar=0 overscan=0 videoformat=5 range=0 colorprim=9 transfer=16 colormatrix=9 chromaloc=1 chromaloc-top=2 chromaloc-bottom=2 display-window=0 master-display=G(13250,34500)B(7500,3000)R(34000,16000)WP(15635,16450)L(40000000,50) max-cll=552,190 min-luma=0 max-luma=1023 log2-max-poc-lsb=8 vui-timing-info vui-hrd-info slices=1 no-opt-qp-pps no-opt-ref-list-length-pps no-multi-pass-opt-rps scenecut-bias=0.05 no-opt-cu-delta-qp no-aq-motion hdr hdr-opt no-dhdr10-opt analysis-save=(null) analysis-load=(null) analysis-reuse-level=5 scale-factor=0 refine-intra=0 refine-inter=0 refine-mv=0 no-limit-sao ctu-info=0 no-lowpass-dct refine-mv-type=0 copy-pic=1

2194 bytes, in x265_param2string function part 2028 bytes (MAXPARAMSIZE was 2000).

With this patch the file header is only 2094 bytes (1928 bytes in x265_param2string):
x265 (build 151) - 2.6+27-2f3c4158cf35:[Windows][MSVC 1900][64 bit] 10bit - H.265/HEVC codec - Copyright 2013-2018 (c) Multicoreware, Inc - http://x265.org - options: cpuid=1173503 frame-threads=3 numa-pools=15,- wpp no-pmode no-pme no-psnr no-ssim log-level=2 csv csv-log-level=0 bitdepth=10 input-csp=1 fps=24/1 input-res=4096x1714 interlace=0 total-frames=0 level-idc=51 high-tier=1 uhd-bd=0 ref=5 no-allow-non-conformance repeat-headers annexb aud hrd info hash=0 no-temporal-layers no-open-gop min-keyint=1 keyint=360 gop-lookahead=0 bframes=9 b-adapt=2 b-pyramid bframe-bias=0 rc-lookahead=60 lookahead-slices=4 scenecut=40 radl=0 no-intra-refresh ctu=32 min-cu-size=8 no-rect no-amp max-tu-size=32 tu-inter-depth=2 tu-intra-depth=2 limit-tu=3 rdoq-level=2 dynamic-rd=0.00 ssim-rd signhide no-tskip nr-intra=0 nr-inter=0 no-constrained-intra no-strong-intra-smoothing max-merge=3 limit-refs=2 limit-modes me=3 subme=5 merange=25 temporal-mvp weightp weightb no-analyze-src-pics deblock=-1:-1 no-sao no-sao-non-deblock rd=4 no-early-skip no-rskip no-fast-intra no-tskip-fast no-cu-lossless b-intra no-splitrd-skip rdpenalty=1 psy-rd=0.00 psy-rdoq=8.00 no-rd-refine no-lossless cbqpoffs=-2 crqpoffs=-2 rc=crf crf=19.0 qcomp=0.70 qpstep=4 stats-write=0 stats-read=0 vbv-maxrate=100000 vbv-bufsize=100000 vbv-init=0.9 crf-max=0.0 crf-min=0.0 ipratio=1.40 pbratio=1.30 aq-mode=3 aq-strength=0.90 cutree zone-count=0 no-strict-cbr qg-size=32 no-rc-grain qpmax=69 qpmin=0 no-const-vbv sar=0 overscan=0 videoformat=5 range=0 colorprim=9 transfer=16 colormatrix=9 chromaloc=1 chromaloc-top=2 chromaloc-bottom=2 display-window=0 master-display=G(13250,34500)B(7500,3000)R(34000,16000)WP(15635,16450)L(40000000,50) max-cll=552,190 min-luma=0 max-luma=1023 log2-max-poc-lsb=8 vui-timing-info vui-hrd-info slices=1 no-opt-qp-pps no-opt-ref-list-length-pps no-multi-pass-opt-rps scenecut-bias=0.05 no-opt-cu-delta-qp no-aq-motion hdr hdr-opt no-dhdr10-opt analysis-reuse-level=5 scale-factor=0 refine-intra=0 refine-inter=0 refine-mv=0 no-limit-sao ctu-info=0 no-lowpass-dct refine-mv-type=0 copy-pic=1

The macro MAXPARAMSIZE is used only in one place:
    buf = s = X265_MALLOC(char, MAXPARAMSIZE);
and it is defined in another file (nobody bother to check the value in another file).

We could leave the macro MAXPARAMSIZE in param.h (with bigger value than 2000)
but I think that the formula:
bufSize = MAXPARAMSIZE + 16 * zones + length_of_all_remaining_%s
should stay.

Mateusz