[x265] [PATCH 1 of 4] add analysis data structures and param options

[Steve Borho]

On 09/10, sagar at multicorewareinc.com wrote:
> # HG changeset patch
> # User Sagar Kotecha <sagar at multicorewareinc.com>
> # Date 1410350715 -19800
> #      Wed Sep 10 17:35:15 2014 +0530
> # Node ID 177916d7a3089e90c973dd5dc7428bf68df728d0
> # Parent  81c9f704ae38a75c070d1b9b3822cbf2a36959b2
> add analysis data structures and param options

ok, this series looks good. now it's down to nits
> 
> diff -r 81c9f704ae38 -r 177916d7a308 source/x265.h
> --- a/source/x265.h	Wed Sep 10 11:40:39 2014 +0200
> +++ b/source/x265.h	Wed Sep 10 17:35:15 2014 +0530
> @@ -88,6 +88,37 @@
>      uint8_t* payload;
>  } x265_nal;
>  
> +/* Used to dump intra and inter frame meta-data, Which can be used in
> + * subsequent calls for the diffrent bitrate of the same clip. */
> +struct x265_inter_data
> +{
> +    uint32_t zOrder;
> +    int      ref[2];
> +    int      costZero[2];
> +    int16_t  mvx[2];
> +    int16_t  mvy[2];
> +    uint8_t  depth;
> +    int      poc;
> +    uint32_t cuAddr;
> +};
> +
> +struct x265_intra_data
> +{
> +    uint8_t*  depth;
> +    uint8_t*  modes;
> +    char*     partSizes;
> +    int*      poc;
> +    uint32_t* cuAddr;
> +};
> +
> +struct x265_analysis_data
> +{
> +    x265_inter_data* interData;
> +    x265_intra_data* intraData;
> +    uint32_t         numCUsInFrame;
> +    uint32_t         numPartitions;
> +};
> +
>  /* Used to pass pictures into the encoder, and to get picture data back out of
>   * the encoder.  The input and output semantics are different */
>  typedef struct x265_picture
> @@ -134,6 +165,10 @@
>      /* force quantizer for != X265_QP_AUTO */
>      int     forceqp;
>  
> +    /* Holds analysis meta data when bAnalysisDataIn/bAnalysisDataOut is true
> +     * and analysisData pointers are valid */
> +    x265_analysis_data analysisData;

"meta" means nothing at all and the sentence has the same meaning
without it, so I recommend removing it. "is" should be "are" and the
slash "/" should probably be " or "

>      /* new data members to this structure must be added to the end so that
>       * users of x265_picture_alloc/free() can be assured of future safety */
>  } x265_picture;
> @@ -637,6 +672,14 @@
>       * effect in presets which use RDOQ (rd-levels 4 and 5). Default 0.0 */
>      double    psyRdoq;
>  
> +    /* Write analysis information into x265_picture buffer when analysis buffers
> +     * are valid */
> +    int       bAnalysisDataOut;

there should be at least a hint here for why a user might find this
useful. something along the lines of "this data can be passed to future
encoders to reduce the amount of work those encoders must perform"
>
> +    /* Read analysis information into x265_picture buffer when analysis buffers
> +     * are valid */
> +    int       bAnalysisDataIn;
> +
>      /*== Coding tools ==*/
>  
>      /* Enable the implicit signaling of the sign bit of the last coefficient of
> _______________________________________________
> x265-devel mailing list
> x265-devel at videolan.org
> https://mailman.videolan.org/listinfo/x265-devel

-- 
Steve Borho