<div dir="ltr">We planned to remove the --cu-stats option which have the data based on slicetype and now this will be made per frame and printed on the csv based on the log-level. I will send the patch as per the new design. We are also planning to do this in API.</div><div class="gmail_extra"><br><div class="gmail_quote">On Tue, May 26, 2015 at 6:15 PM, Steve Borho <span dir="ltr"><<a href="mailto:steve@borho.org" target="_blank">steve@borho.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">On 05/25, Divya Manivannan wrote:<br>
> # HG changeset patch<br>
> # User Divya Manivannan <<a href="mailto:divya@multicorewareinc.com">divya@multicorewareinc.com</a>><br>
> # Date 1432535853 -19800<br>
> # Mon May 25 12:07:33 2015 +0530<br>
> # Node ID 05579078c8faf0f4c3dbe10805d1b83048db1e96<br>
> # Parent a7bf7a150a705489cb63d0454c59ec599bad8c93<br>
> stats: report counts of each CU partition size/mode per frame<br>
><br>
> diff -r a7bf7a150a70 -r 05579078c8fa source/common/param.cpp<br>
> --- a/source/common/param.cpp Fri May 22 14:29:35 2015 +0530<br>
> +++ b/source/common/param.cpp Mon May 25 12:07:33 2015 +0530<br>
> @@ -104,6 +104,7 @@<br>
> param->csvfn = NULL;<br>
> param->rc.lambdaFileName = NULL;<br>
> param->bLogCuStats = 0;<br>
> + param->bLogFrameStats = 0;<br>
> param->decodedPictureHashSEI = 0;<br>
><br>
> /* Quality Measurement Metrics */<br>
> @@ -582,6 +583,7 @@<br>
> }<br>
> }<br>
> OPT("cu-stats") p->bLogCuStats = atobool(value);<br>
> + OPT("frame-stats") p->bLogFrameStats = atobool(value);<br>
> OPT("annexb") p->bAnnexB = atobool(value);<br>
> OPT("repeat-headers") p->bRepeatHeaders = atobool(value);<br>
> OPT("wpp") p->bEnableWavefront = atobool(value);<br>
> diff -r a7bf7a150a70 -r 05579078c8fa source/encoder/encoder.cpp<br>
> --- a/source/encoder/encoder.cpp Fri May 22 14:29:35 2015 +0530<br>
> +++ b/source/encoder/encoder.cpp Mon May 25 12:07:33 2015 +0530<br>
> @@ -73,6 +73,7 @@<br>
> m_numDelayedPic = 0;<br>
> m_outputCount = 0;<br>
> m_csvfpt = NULL;<br>
> + m_csvfptFrame = NULL;<br>
> m_param = NULL;<br>
> m_latestParam = NULL;<br>
> m_cuOffsetY = NULL;<br>
> @@ -254,6 +255,20 @@<br>
> }<br>
> }<br>
><br>
> + if (m_param->bLogFrameStats)<br>
> + {<br>
> + m_csvfptFrame = fopen("FrameLog.csv", "wb");<br>
> + if (m_csvfptFrame)<br>
> + {<br>
> + fprintf(m_csvfptFrame, "POC, SLICE TYPE, 64x64 CU%% , 32x32 CU%%, 16x16 CU%%, 8x8 CU%%, 4x4 prediction%%, Intra CU%%, Inter CU%%, Skip CU%%\n");<br>
<br>
</div></div>w/s<br>
<div><div class="h5"><br>
> + }<br>
> + else<br>
> + {<br>
> + x265_log(m_param, X265_LOG_ERROR, "Unable to open CSV log file FrameLog.csv, aborting\n");<br>
> + m_aborted = true;<br>
> + }<br>
> + }<br>
> +<br>
> int numRows = (m_param->sourceHeight + g_maxCUSize - 1) / g_maxCUSize;<br>
> int numCols = (m_param->sourceWidth + g_maxCUSize - 1) / g_maxCUSize;<br>
> for (int i = 0; i < m_param->frameNumThreads; i++)<br>
> @@ -367,6 +382,8 @@<br>
> fclose(m_analysisFile);<br>
> if (m_csvfpt)<br>
> fclose(m_csvfpt);<br>
> + if (m_csvfptFrame)<br>
> + fclose(m_csvfptFrame);<br>
><br>
> if (m_param)<br>
> {<br>
> @@ -1465,6 +1482,16 @@<br>
> fprintf(m_csvfpt, "\n");<br>
> fflush(stderr);<br>
> }<br>
> +<br>
> + if (m_param->bLogFrameStats && m_csvfptFrame)<br>
> + {<br>
> + fprintf(m_csvfptFrame, "%d, %c-SLICE,", poc, c);<br>
</div></div>> + fprintf(m_csvfptFrame, " %.2lf, %.2lf, %.2lf,", curEncoder->m_frameStats..percentCuDepth[0], curEncoder->m_frameStats.cuStat.percentCuDepth[1], curEncoder->m_frameStats.cuStat.percentCuDepth[2]);<br>
<div><div class="h5">> + fprintf(m_csvfptFrame, " %.2lf, %.2lf, %.2lf,", curEncoder->m_frameStats.cuStat.percentCuDepth[3], curEncoder->m_frameStats.cuStat.percentIntraNxN, curEncoder->m_frameStats.cuStat.percentIntraCu);<br>
> + fprintf(m_csvfptFrame, " %.2lf, %.2lf", curEncoder->m_frameStats.cuStat.percentInterCu, curEncoder->m_frameStats.cuStat.percentSkipCu);<br>
> + fprintf(m_csvfptFrame, "\n");<br>
> + fflush(stderr);<br>
> + }<br>
> }<br>
><br>
> #if defined(_MSC_VER)<br>
> diff -r a7bf7a150a70 -r 05579078c8fa source/encoder/encoder.h<br>
> --- a/source/encoder/encoder.h Fri May 22 14:29:35 2015 +0530<br>
> +++ b/source/encoder/encoder.h Mon May 25 12:07:33 2015 +0530<br>
> @@ -106,6 +106,7 @@<br>
> EncStats m_analyzeP;<br>
> EncStats m_analyzeB;<br>
> FILE* m_csvfpt;<br>
> + FILE* m_csvfptFrame;<br>
> int64_t m_encodeStartTime;<br>
><br>
> // weighted prediction<br>
> diff -r a7bf7a150a70 -r 05579078c8fa source/encoder/frameencoder.cpp<br>
> --- a/source/encoder/frameencoder.cpp Fri May 22 14:29:35 2015 +0530<br>
> +++ b/source/encoder/frameencoder.cpp Mon May 25 12:07:33 2015 +0530<br>
> @@ -552,7 +552,7 @@<br>
> }<br>
> }<br>
><br>
> - if (m_param->rc.bStatWrite)<br>
> + if (m_param->rc.bStatWrite || m_param->bLogFrameStats)<br>
> {<br>
> int totalI = 0, totalP = 0, totalSkip = 0;<br>
><br>
> @@ -562,14 +562,30 @@<br>
> m_frameStats.mvBits += m_rows[i].rowStats.mvBits;<br>
> m_frameStats.coeffBits += m_rows[i].rowStats.coeffBits;<br>
> m_frameStats.miscBits += m_rows[i].rowStats.miscBits;<br>
> + m_frameStats.cuStat.cntInterCu += m_rows[i].rowStats.cuStat.cntInterCu;<br>
> + m_frameStats.cuStat.cntIntraCu += m_rows[i].rowStats.cuStat.cntIntraCu;<br>
> + m_frameStats.cuStat.cntIntraNxN += m_rows[i].rowStats.cuStat.cntIntraNxN;<br>
> + m_frameStats.cuStat.cntSkipCu += m_rows[i].rowStats.cuStat.cntSkipCu;<br>
> + m_frameStats.cuStat.totalCu += m_rows[i].rowStats.cuStat.totalCu;<br>
> totalI += m_rows[i].rowStats.iCuCnt;<br>
> totalP += m_rows[i].rowStats.pCuCnt;<br>
> totalSkip += m_rows[i].rowStats.skipCuCnt;<br>
> +<br>
> + for (uint32_t depth = 0; depth <= g_maxCUDepth; depth++)<br>
> + {<br>
> + m_frameStats.cuStat.cntCuDepth[depth] += m_rows[i].rowStats.cuStat.cntCuDepth[depth];<br>
> + }<br>
<br>
</div></div>we don't use braces around single line expressions, generally<br>
<div><div class="h5"><br>
> }<br>
> int totalCuCount = totalI + totalP + totalSkip;<br>
> m_frameStats.percentIntra = (double)totalI / totalCuCount;<br>
> m_frameStats.percentInter = (double)totalP / totalCuCount;<br>
> m_frameStats.percentSkip = (double)totalSkip / totalCuCount;<br>
> + m_frameStats.cuStat.percentIntraNxN = (double)(m_frameStats.cuStat.cntIntraNxN * 100) / m_frameStats.cuStat.totalCu;<br>
> + m_frameStats.cuStat.percentIntraCu = (double)(m_frameStats.cuStat.cntIntraCu * 100) / m_frameStats.cuStat.totalCu;<br>
> + m_frameStats.cuStat.percentInterCu = (double)(m_frameStats.cuStat.cntInterCu * 100) / m_frameStats.cuStat.totalCu;<br>
> + m_frameStats.cuStat.percentSkipCu = (double)(m_frameStats.cuStat.cntSkipCu * 100) / m_frameStats.cuStat.totalCu;<br>
> + for (uint32_t depth = 0; depth <= g_maxCUDepth; depth++)<br>
> + m_frameStats.cuStat.percentCuDepth[depth] = (double)(m_frameStats.cuStat.cntCuDepth[depth] * 100) / m_frameStats.cuStat.totalCu;<br>
> }<br>
><br>
> m_bs.resetBits();<br>
> @@ -904,17 +920,22 @@<br>
> // Completed CU processing<br>
> curRow.completed++;<br>
><br>
> - if (m_param->bLogCuStats || m_param->rc.bStatWrite)<br>
> + if (m_param->bLogCuStats || m_param->rc.bStatWrite || m_param->bLogFrameStats)<br>
> curEncData.m_rowStat[row].sumQpAq += collectCTUStatistics(*ctu, qTreeInterCnt, qTreeIntraCnt, qTreeSkipCnt);<br>
> else if (m_param->rc.aqMode)<br>
> curEncData.m_rowStat[row].sumQpAq += calcCTUQP(*ctu);<br>
><br>
> // copy no. of intra, inter Cu cnt per row into frame stats for 2 pass<br>
> - if (m_param->rc.bStatWrite)<br>
> + if (m_param->rc.bStatWrite || m_param->bLogFrameStats)<br>
> {<br>
> curRow.rowStats.mvBits += best.mvBits;<br>
> curRow.rowStats.coeffBits += best.coeffBits;<br>
> curRow.rowStats.miscBits += best.totalBits - (best.mvBits + best.coeffBits);<br>
> + curRow.rowStats.cuStat.cntInterCu += m_cuStatistic.cntInterCu;<br>
> + curRow.rowStats.cuStat.cntIntraCu += m_cuStatistic.cntIntraCu;<br>
> + curRow.rowStats.cuStat.cntIntraNxN += m_cuStatistic.cntIntraNxN;<br>
> + curRow.rowStats.cuStat.cntSkipCu += m_cuStatistic.cntSkipCu;<br>
> + curRow.rowStats.cuStat.totalCu += m_cuStatistic.totalCu;<br>
><br>
> for (uint32_t depth = 0; depth <= g_maxCUDepth; depth++)<br>
> {<br>
> @@ -923,10 +944,12 @@<br>
> curRow.rowStats.iCuCnt += qTreeIntraCnt[depth] << shift;<br>
> curRow.rowStats.pCuCnt += qTreeInterCnt[depth] << shift;<br>
> curRow.rowStats.skipCuCnt += qTreeSkipCnt[depth] << shift;<br>
> + curRow.rowStats.cuStat.cntCuDepth[depth] += m_cuStatistic.cntCuDepth[depth];<br>
><br>
> // clear the row cu data from thread local object<br>
> qTreeIntraCnt[depth] = qTreeInterCnt[depth] = qTreeSkipCnt[depth] = 0;<br>
> }<br>
> + memset(&m_cuStatistic, 0, sizeof(CuStatistic));<br>
> }<br>
><br>
> curEncData.m_cuStat[cuAddr].totalBits = best.totalBits;<br>
> @@ -1106,6 +1129,7 @@<br>
> int FrameEncoder::collectCTUStatistics(const CUData& ctu, uint32_t* qtreeInterCnt, uint32_t* qtreeIntraCnt, uint32_t* qtreeSkipCnt)<br>
> {<br>
> StatisticLog* log = &m_sliceTypeLog[ctu.m_slice->m_sliceType];<br>
> + CuStatistic* cuLog = &m_cuStatistic;<br>
> int totQP = 0;<br>
><br>
> if (ctu.m_slice->m_sliceType == I_SLICE)<br>
> @@ -1117,6 +1141,9 @@<br>
><br>
> log->totalCu++;<br>
> log->cntIntra[depth]++;<br>
> + cuLog->cntCuDepth[depth]++;<br>
> + cuLog->cntIntraCu++;<br>
> + cuLog->totalCu++;<br>
<br>
</div></div>This all looks terribly redundant, and this is introducing new data<br>
races. Perhaps this function can increment only m_cuStatistic, and at<br>
the end of the frame those stats are all added to<br>
m_sliceTypeLog[m_slice->m_sliceType];<br>
<div><div class="h5"><br>
> qtreeIntraCnt[depth]++;<br>
> totQP += ctu.m_qp[absPartIdx] * (ctu.m_numPartitions >> (depth * 2));<br>
><br>
> @@ -1124,6 +1151,9 @@<br>
> {<br>
> log->totalCu--;<br>
> log->cntIntra[depth]--;<br>
> + cuLog->cntCuDepth[depth]--;<br>
> + cuLog->cntIntraCu--;<br>
> + cuLog->totalCu--;<br>
> qtreeIntraCnt[depth]--;<br>
> }<br>
> else if (ctu.m_partSize[absPartIdx] != SIZE_2Nx2N)<br>
> @@ -1132,6 +1162,8 @@<br>
> X265_CHECK(ctu.m_log2CUSize[absPartIdx] == 3 && ctu.m_slice->m_sps->quadtreeTULog2MinSize < 3, "Intra NxN found at improbable depth\n");<br>
> log->cntIntraNxN++;<br>
> log->cntIntra[depth]--;<br>
> + cuLog->cntIntraNxN++;<br>
> + cuLog->cntCuDepth[depth]--;<br>
> }<br>
> else if (ctu.m_lumaIntraDir[absPartIdx] > 1)<br>
> log->cuIntraDistribution[depth][ANGULAR_MODE_ID]++;<br>
> @@ -1148,22 +1180,28 @@<br>
><br>
> log->totalCu++;<br>
> log->cntTotalCu[depth]++;<br>
> + cuLog->cntCuDepth[depth]++;<br>
> + cuLog->totalCu++;<br>
> totQP += ctu.m_qp[absPartIdx] * (ctu.m_numPartitions >> (depth * 2));<br>
><br>
> if (ctu.m_predMode[absPartIdx] == MODE_NONE)<br>
> {<br>
> log->totalCu--;<br>
> log->cntTotalCu[depth]--;<br>
> + cuLog->cntCuDepth[depth]--;<br>
> + cuLog->totalCu--;<br>
> }<br>
> else if (ctu.isSkipped(absPartIdx))<br>
> {<br>
> log->totalCu--;<br>
> log->cntSkipCu[depth]++;<br>
> + cuLog->cntSkipCu++;<br>
> qtreeSkipCnt[depth]++;<br>
> }<br>
> else if (ctu.isInter(absPartIdx))<br>
> {<br>
> log->cntInter[depth]++;<br>
> + cuLog->cntInterCu++;<br>
> qtreeInterCnt[depth]++;<br>
><br>
> if (ctu.m_partSize[absPartIdx] < AMP_ID)<br>
> @@ -1174,6 +1212,7 @@<br>
> else if (ctu.isIntra(absPartIdx))<br>
> {<br>
> log->cntIntra[depth]++;<br>
> + cuLog->cntIntraCu++;<br>
> qtreeIntraCnt[depth]++;<br>
><br>
> if (ctu.m_partSize[absPartIdx] != SIZE_2Nx2N)<br>
> @@ -1181,6 +1220,8 @@<br>
> X265_CHECK(ctu.m_log2CUSize[absPartIdx] == 3 && ctu.m_slice->m_sps->quadtreeTULog2MinSize < 3, "Intra NxN found at improbable depth\n");<br>
> log->cntIntraNxN++;<br>
> log->cntIntra[depth]--;<br>
> + cuLog->cntIntraNxN++;<br>
> + cuLog->cntCuDepth[depth]--;<br>
> /* TODO: log intra modes at absPartIdx +0 to +3 */<br>
> }<br>
> else if (ctu.m_lumaIntraDir[absPartIdx] > 1)<br>
> diff -r a7bf7a150a70 -r 05579078c8fa source/encoder/frameencoder.h<br>
> --- a/source/encoder/frameencoder.h Fri May 22 14:29:35 2015 +0530<br>
> +++ b/source/encoder/frameencoder.h Mon May 25 12:07:33 2015 +0530<br>
> @@ -157,6 +157,7 @@<br>
> uint32_t m_crc[3];<br>
> uint32_t m_checksum[3];<br>
> StatisticLog m_sliceTypeLog[3]; // per-slice type CU statistics<br>
> + CuStatistic m_cuStatistic; // stats of current CU<br>
> FrameStats m_frameStats; // stats of current frame for multi-pass encodes<br>
><br>
> volatile int m_activeWorkerCount; // count of workers currently encoding or filtering CTUs<br>
> diff -r a7bf7a150a70 -r 05579078c8fa source/encoder/ratecontrol.h<br>
> --- a/source/encoder/ratecontrol.h Fri May 22 14:29:35 2015 +0530<br>
> +++ b/source/encoder/ratecontrol.h Mon May 25 12:07:33 2015 +0530<br>
> @@ -46,6 +46,27 @@<br>
> #define MIN_AMORTIZE_FRACTION 0.2<br>
> #define CLIP_DURATION(f) x265_clip3(MIN_FRAME_DURATION, MAX_FRAME_DURATION, f)<br>
><br>
> +struct CuStatistic<br>
> +{<br>
> + uint64_t cntCuDepth[4];<br>
> + uint64_t cntIntraNxN;<br>
> + uint64_t cntSkipCu;<br>
> + uint64_t cntIntraCu;<br>
> + uint64_t cntInterCu;<br>
> + uint64_t totalCu;<br>
> +<br>
> + double percentCuDepth[4];<br>
> + double percentIntraNxN;<br>
> + double percentSkipCu;<br>
> + double percentIntraCu;<br>
> + double percentInterCu;<br>
> +<br>
> + CuStatistic()<br>
> + {<br>
> + memset(this, 0, sizeof(CuStatistic));<br>
> + }<br>
> +};<br>
> +<br>
> /* Current frame stats for 2 pass */<br>
> struct FrameStats<br>
> {<br>
> @@ -61,6 +82,8 @@<br>
> double percentIntra;<br>
> double percentInter;<br>
> double percentSkip;<br>
> +<br>
> + CuStatistic cuStat;<br>
> };<br>
<br>
</div></div>this data structure nesting doesn't make a lot of sense.<br>
<span class=""><br>
> struct Predictor<br>
> diff -r a7bf7a150a70 -r 05579078c8fa source/x265.h<br>
> --- a/source/x265.h Fri May 22 14:29:35 2015 +0530<br>
> +++ b/source/x265.h Mon May 25 12:07:33 2015 +0530<br>
> @@ -443,6 +443,9 @@<br>
> * modes during mode decision. Default disabled */<br>
> int bLogCuStats;<br>
><br>
> + /* Enable logging of frame level data. Default is disabled*/<br>
</span>w/s<br>
<span class="">> + int bLogFrameStats;<br>
> +<br>
> /* Enable the measurement and reporting of PSNR. Default is enabled */<br>
> int bEnablePsnr;<br>
<br>
</span>new params would require reST docs and an API version bump<br>
<span class=""><br>
> diff -r a7bf7a150a70 -r 05579078c8fa source/x265cli.h<br>
> --- a/source/x265cli.h Fri May 22 14:29:35 2015 +0530<br>
> +++ b/source/x265cli.h Mon May 25 12:07:33 2015 +0530<br>
> @@ -56,6 +56,8 @@<br>
> { "csv", required_argument, NULL, 0 },<br>
> { "no-cu-stats", no_argument, NULL, 0 },<br>
> { "cu-stats", no_argument, NULL, 0 },<br>
> + { "no-frame-stats", no_argument, NULL, 0 },<br>
> + { "frame-stats", no_argument, NULL, 0 },<br>
> { "y4m", no_argument, NULL, 0 },<br>
> { "no-progress", no_argument, NULL, 0 },<br>
> { "output", required_argument, NULL, 'o' },<br>
> @@ -248,6 +250,7 @@<br>
> H0(" --log-level <string> Logging level: none error warning info debug full. Default %s\n", x265::logLevelNames[param->logLevel + 1]);<br>
> H0(" --no-progress Disable CLI progress reports\n");<br>
> H0(" --[no-]cu-stats Enable logging stats about distribution of cu across all modes. Default %s\n",OPT(param->bLogCuStats));<br>
> + H0(" --[no-]frame-stats Enable logging stats about frames. Default %s\n", OPT(param->bLogFrameStats));<br>
<br>
</span>--cu-stats toggles logging of stats to x265_log(), --frame-stats toggles<br>
the writing of a new CSV file (with a hard coded filename). This is<br>
non-intuitive. This really behaves like it should be named --frame-csv<br>
and its param should be a const char*, which begs a number of other questions.<br>
<br>
I don't much like how the libx265 code is writing so many files, when<br>
much of this data could be quite useful for any code that monitors the<br>
progress of an encode. I can understand exceptions being made for the<br>
2-pass stats files and the analysis re-use files being read and written<br>
directly by the encoder (since they must work via ffmpeg and similar<br>
apps), but I believe the CSV files really should be getting written by<br>
our CLI.<br>
<br>
Everything that is getting written into our CSV files today should be<br>
discoverable from the C API, and the CLI should be using those<br>
interfaces to write the CSV files itself.<br>
<br>
We should define a new x265_frame_stats struct in x265.h and add a<br>
'x265_frame_stats* frame_stats' to the end of x265_picture, and have<br>
x265_encoder_encode() fill it in if pic_out->frame_stats is not NULL.<br>
And this struct should contain all of the data that is written into<br>
per-frame CSV files today, plus these new per-frame coding stats.<br>
<br>
Then all of the CSV writing can be hoisted out of the libx265 library<br>
and into x265.cpp<br>
<span class="HOEnZb"><font color="#888888"><br>
--<br>
Steve Borho<br>
_______________________________________________<br>
x265-devel mailing list<br>
<a href="mailto:x265-devel@videolan.org">x265-devel@videolan.org</a><br>
<a href="https://mailman.videolan.org/listinfo/x265-devel" target="_blank">https://mailman.videolan.org/listinfo/x265-devel</a><br>
</font></span></blockquote></div><br></div>