[x265] [PATCH 4 of 4] cli: add cli options analysis-out and analysis-in
Sagar Kotecha
sagar at multicorewareinc.com
Tue Sep 9 16:35:23 CEST 2014
On Tue, Sep 9, 2014 at 4:54 PM, Steve Borho <steve at borho.org> wrote:
> On 09/09, sagar at multicorewareinc.com wrote:
> > # HG changeset patch
> > # User Sagar Kotecha <sagar at multicorewareinc.com>
> > # Date 1410252991 -19800
> > # Tue Sep 09 14:26:31 2014 +0530
> > # Node ID e546a7da6d486e1bc6c6da09ce7188a5796dff34
> > # Parent 3c02887100033f399135f2a774d8253eecb971e3
> > cli: add cli options analysis-out and analysis-in
> >
> > analysis-out - dump the intra/inter information into file
> > analysis-in - read the intra/inter information from file
> >
> > diff -r 3c0288710003 -r e546a7da6d48 source/common/param.cpp
> > --- a/source/common/param.cpp Tue Sep 09 14:24:11 2014 +0530
> > +++ b/source/common/param.cpp Tue Sep 09 14:26:31 2014 +0530
> > @@ -724,6 +724,8 @@
> > p->rc.bStatRead = pass & 2;
> > }
> > OPT("stats") p->rc.statFileName = strdup(value);
> > + OPT("analysis-out") p->bAnalysisDataOut = atobool(value);
> > + OPT("analysis-in") p->bAnalysisDataIn = atobool(value);
> > else
> > return X265_PARAM_BAD_NAME;
> > #undef OPT
> > @@ -1023,6 +1025,8 @@
> > "Constant rate-factor is incompatible with 2pass");
> > CHECK(param->rc.rateControlMode == X265_RC_CQP &&
> param->rc.bStatRead,
> > "Constant QP is incompatible with 2pass");
> > + CHECK((param->bAnalysisDataIn == 1 && param->bAnalysisDataOut == 1),
> >
> our coding style is more like:
> CHECK((param->bAnalysisDataIn && param->bAnalysisDataOut),
>
> > + "analysis-out and analysis-in options can not go together\n");
>
> white-space, and also: why not?
>
> > return check_failed;
> > }
> >
> > diff -r 3c0288710003 -r e546a7da6d48 source/x265.cpp
> > --- a/source/x265.cpp Tue Sep 09 14:24:11 2014 +0530
> > +++ b/source/x265.cpp Tue Sep 09 14:26:31 2014 +0530
> > @@ -202,6 +202,8 @@
> > { "pass", required_argument, NULL, 0 },
> > { "slow-firstpass", no_argument, NULL, 0 },
> > { "no-slow-firstpass", no_argument, NULL, 0 },
> > + { "analysis-out", required_argument, NULL, 0 },
> > + { "analysis-in", required_argument, NULL, 0 },
>
> required_argument?
>
> > { 0, 0, 0, 0 }
> > };
> >
> > @@ -229,6 +231,7 @@
> > int64_t prevUpdateTime;
> > float frameRate;
> > FILE* qpfile;
> > + FILE* analysisFile;
> >
> > /* in microseconds */
> > static const int UPDATE_INTERVAL = 250000;
> > @@ -245,6 +248,7 @@
> > prevUpdateTime = 0;
> > bDither = false;
> > qpfile = NULL;
> > + analysisFile = NULL;
> > }
> >
> > void destroy();
> > @@ -254,6 +258,8 @@
> > void showHelp(x265_param *param);
> > bool parse(int argc, char **argv, x265_param* param);
> > bool parseQPFile(x265_picture &pic_org);
> > + void readAnalysisFile(x265_picture* pic, x265_param*);
> > + void writeAnalysisFile(x265_picture* pic, x265_param*);
> > };
> >
> > void CLIOptions::destroy()
> > @@ -267,6 +273,9 @@
> > if (qpfile)
> > fclose(qpfile);
> > qpfile = NULL;
> > + if (analysisFile)
> > + fclose(analysisFile);
> > + analysisFile = NULL;
> > }
> >
> > void CLIOptions::writeNALs(const x265_nal* nal, uint32_t nalcount)
> > @@ -468,6 +477,8 @@
> > H0("\nReconstructed video options (debugging):\n");
> > H0("-r/--recon <filename> Reconstructed raw image YUV or
> Y4M output file name\n");
> > H0(" --recon-depth <integer> Bit-depth of reconstructed raw
> image file. Defaults to input bit depth, or 8 if Y4M\n");
> > + H0(" --analysis-out Frames intra/inter analysis
> info dumped into file. (each frame data is dumped into its own file, each
> file is named with its #poc \n");
> > + H0(" --analysis-in Intra-Inter analysis info is
> read from file. (each frame data is read from its own file, expected each
> file named with its #poc \n");
>
> these two options don't belong under "Reconstructed video options".
> They belong with the two-pass options or under their own heading
>
> Also, we need a patch in this series which bumps X265_BUILD and adds
> documentation for these two new options.
>
> > #undef OPT
> > #undef H0
> > printf("\n\nFull documentation may be found at
> http://x265.readthedocs.org/en/default/cli.html\n");
> > @@ -728,6 +739,68 @@
> > return false;
> > }
> >
> > +void CLIOptions::readAnalysisFile(x265_picture* pic, x265_param* p)
> > +{
> > + char fname[10];
> > + sprintf(fname, "%d", pic->poc);
> > + this->analysisFile = fopen(fname, "rb");
>
> a file per frame is kind of brutal, we'll need some mechanism for using
> a single analysis file. either by making each record a fixed size, or
> scanning ahead to find the record you need.
>
> > + if (this->analysisFile)
> > + {
> > + int poc, width, height;
> > + fscanf(this->analysisFile, "%dx%d %d %d %d %d\n", &width,
> &height, &poc, &pic->sliceType, &pic->intraInterData.numCUsInFrame,
> &pic->intraInterData.numPartitions);
> > + if (poc != pic->poc || width != p->sourceWidth || height !=
> p->sourceHeight)
> > + {
> > + x265_log(NULL, X265_LOG_WARNING, "Error in reading
> intra-inter data.\n");
> > + x265_free_inter_intra_data(pic);
> > + return;
> > + }
> > + if (pic->sliceType == X265_TYPE_I || pic->sliceType ==
> X265_TYPE_IDR)
> > + {
> > + fread(pic->intraInterData.intraData->depth,
> sizeof(uint8_t), pic->intraInterData.numPartitions *
> pic->intraInterData.numPartitions, this->analysisFile);
> > + fread(pic->intraInterData.intraData->modes,
> sizeof(uint8_t), pic->intraInterData.numPartitions *
> pic->intraInterData.numPartitions, this->analysisFile);
> > + fread(pic->intraInterData.intraData->partSizes,
> sizeof(char), pic->intraInterData.numPartitions *
> pic->intraInterData.numPartitions, this->analysisFile);
> > + fread(pic->intraInterData.intraData->poc, sizeof(int),
> pic->intraInterData.numCUsInFrame, this->analysisFile);
> > + fread(pic->intraInterData.intraData->cuAddr,
> sizeof(uint32_t), pic->intraInterData.numCUsInFrame, this->analysisFile);
> > + }
> > + else
> > + fread(pic->intraInterData.interData,
> sizeof(x265_inter_data), pic->intraInterData.numCUsInFrame * 85,
> this->analysisFile);
> > + }
> > + else
> > + {
> > + x265_log(NULL, X265_LOG_WARNING, "Error in opening file to read
> analysis data.\n");
> > + x265_free_inter_intra_data(pic);
> > + }
> > +}
> > +
> > +void CLIOptions::writeAnalysisFile(x265_picture* pic, x265_param *p)
> > +{
> > + char fname[10];
> > + sprintf(fname, "%d", pic->poc);
> > + this->analysisFile = fopen(fname, "wb");
> > + if (this->analysisFile)
> > + {
> > + fprintf(this->analysisFile, "%dx%d %d %d %d %d\n",
> p->sourceWidth, p->sourceHeight, pic->poc, pic->sliceType,
> pic->intraInterData.numCUsInFrame, pic->intraInterData.numPartitions);
> > + if (pic->sliceType == X265_TYPE_I || pic->sliceType ==
> X265_TYPE_IDR)
> > + {
> > + fwrite(pic->intraInterData.intraData->depth,
> > + sizeof(uint8_t), pic->intraInterData.numPartitions *
> pic->intraInterData.numCUsInFrame, this->analysisFile);
> > + fwrite(pic->intraInterData.intraData->modes,
> > + sizeof(uint8_t), pic->intraInterData.numPartitions *
> pic->intraInterData.numCUsInFrame, this->analysisFile);
> > + fwrite(pic->intraInterData.intraData->partSizes,
> > + sizeof(char), pic->intraInterData.numPartitions *
> pic->intraInterData.numCUsInFrame, this->analysisFile);
> > + fwrite(pic->intraInterData.intraData->poc, sizeof(int),
> pic->intraInterData.numCUsInFrame, this->analysisFile);
> > + fwrite(pic->intraInterData.intraData->cuAddr,
> sizeof(uint32_t), pic->intraInterData.numCUsInFrame, this->analysisFile);
> > + }
> > + else
> > + fwrite(pic->intraInterData.interData,
> sizeof(x265_inter_data), pic->intraInterData.numCUsInFrame * 85,
> this->analysisFile);
> > + }
> > + else
> > + {
> > + x265_log(NULL, X265_LOG_WARNING, "Error in opening file to
> write analysis data.\n");
> > + x265_free_inter_intra_data(pic);
> > + }
> > +}
> > +
> > bool CLIOptions::parseQPFile(x265_picture &pic_org)
> > {
> > int32_t num = -1, qp, ret;
> > @@ -820,6 +893,13 @@
> >
> > x265_picture_init(param, pic_in);
> >
> > + if ((param->bAnalysisDataOut || param->bAnalysisDataIn) &&
> !pic_recon)
> > + {
> > + x265_log(NULL, X265_LOG_WARNING, "Must specify recon with
> analysis-out or analysis-in options, Disabling --analysis-out,
> --analysis-in\n");
>
> hmm, that's unfortunate. it would be better if the encoder filled in
> the output picture struct if a recon picture is enabled or if
> bAnalysisDataOut is enabled. then this check would not be necessary.
>
> > + param->bAnalysisDataOut = false;
> > + param->bAnalysisDataIn = false;
>
> how does this affect the input side?
>
we delete the analysis buffer only through the pic_recon (once the encoding
is done the analysis buffers are given to pic_recon which latter used to
release the respective pic_in analysis buffers (We don't copy or maintain
separate copy of analysis buffer inside the encoder)).
> > + }
> > +
> > if (cliopt.bDither)
> > {
> > errorBuf = X265_MALLOC(int16_t, param->sourceWidth + 1);
> > @@ -856,6 +936,16 @@
> > pic_in->bitDepth = X265_DEPTH;
> > }
> >
> > + if ((param->bAnalysisDataIn || param->bAnalysisDataOut) &&
> pic_in)
>
> this looks like a bug, should it be checking bAnalysisDataOut here?
>
> We allocate the memory for the pic_in and point it out through recon for
dumping
> + {
> > + x265_alloc_inter_intra_data(pic_in, param);
> > +
> > + if (param->bAnalysisDataIn)
> > + cliopt.readAnalysisFile(pic_in, param);
> > + }
> > + else if (param->bAnalysisDataOut && pic_in)
> > + x265_alloc_inter_intra_data(pic_in, param);
> > +
> > int numEncoded = x265_encoder_encode(encoder, &p_nal, &nal,
> pic_in, pic_recon);
> > if (numEncoded < 0)
> > {
> > @@ -866,6 +956,12 @@
> > if (numEncoded && pic_recon)
> > {
> > cliopt.recon->writePicture(pic_out);
> > + if (param->bAnalysisDataIn || param->bAnalysisDataOut)
> > + {
> > + if (pic_recon->intraInterData.interData)
> > + cliopt.writeAnalysisFile(pic_recon, param);
> > + x265_free_inter_intra_data(pic_recon);
> > + }
> > }
> >
> > if (nal)
> > @@ -883,6 +979,12 @@
> > if (numEncoded && pic_recon)
> > {
> > cliopt.recon->writePicture(pic_out);
> > + if (param->bAnalysisDataIn || param->bAnalysisDataOut)
> > + {
> > + if (param->bAnalysisDataOut)
> > + cliopt.writeAnalysisFile(pic_recon, param);
> > + x265_free_inter_intra_data(pic_recon);
>
> this would be much simpler as just two if expressions
>
> > + }
> > }
> >
> > if (nal)
> > _______________________________________________
> > x265-devel mailing list
> > x265-devel at videolan.org
> > https://mailman.videolan.org/listinfo/x265-devel
>
> --
> Steve Borho
> _______________________________________________
> x265-devel mailing list
> x265-devel at videolan.org
> https://mailman.videolan.org/listinfo/x265-devel
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.videolan.org/pipermail/x265-devel/attachments/20140909/90c4e956/attachment-0001.html>
More information about the x265-devel
mailing list