Petri Savolainen(psavol) replied on github web page:

include/odp/api/spec/comp.h
line 322
@@ -0,0 +1,873 @@
+/* Copyright (c) 2017, Linaro Limited
+ * All rights reserved.
+ *
+ * SPDX-License-Identifier:    BSD-3-Clause
+ */
+
+/**
+ * @file
+ *
+ * ODP Compression
+ */
+
+#ifndef ODP_API_COMP_H_
+#define ODP_API_COMP_H_
+
+#include <odp/visibility_begin.h>
+#include <odp/api/support.h>
+#include <odp/api/packet.h>
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+/** @defgroup odp_compression ODP COMP
+ *  ODP Compression is an API set to do compression+hash or decompression+hash
+ *  operations on data. Hash is calculated on plaintext.
+ *
+ *  if opcode = ODP_COMP_COMPRESS, then it will apply hash and then compress,
+ *  if opcode = ODP_COMP_DECOMPRESS, then it will decompress and then apply
+ *  hash.
+ *  Independent hash-only operations are not supported. Implementation should
+ *  perform hash along with valid compression algo.
+ *  Macros, enums, types and operations to utilize compression interface.
+ *  @{
+ */
+
+/**
+ * @def ODP_COMP_SESSION_INVALID
+ * Invalid session handle
+ */
+
+/**
+ * @typedef odp_comp_session_t
+ * Compression/Decompression session handle
+ */
+
+/**
+ * Compression API operation mode
+ */
+typedef enum {
+       /** Synchronous, return results immediately */
+       ODP_COMP_SYNC,
+       /** Asynchronous, return results via event queue */
+       ODP_COMP_ASYNC
+} odp_comp_op_mode_t;
+
+/**
+ * Comp API operation type.
+ *
+ */
+typedef enum {
+       /** Compress */
+       ODP_COMP_OP_COMPRESS,
+       /** Decompress */
+       ODP_COMP_OP_DECOMPRESS
+} odp_comp_op_t;
+
+/**
+ * Comp API hash algorithm
+ *
+ */
+typedef enum {
+       /** ODP_COMP_HASH_ALG_NONE - No hash algorithm selected. */
+       ODP_COMP_HASH_ALG_NONE,
+       /** ODP_COMP_HASH_ALG_SHA1 - SHA-1 hash algorithm. */
+       ODP_COMP_HASH_ALG_SHA1,
+       /** ODP_COMP_HASH_ALG_SHA256 - SHA-2 hash algorithm
+        * 256-bit digest length.
+        */
+       ODP_COMP_HASH_ALG_SHA256
+} odp_comp_hash_alg_t;
+
+/**
+ * Comp API compression algorithm
+ *
+ */
+typedef enum {
+       /** No algorithm specified.
+        * Means no compression, output == input.
+        * if provided, no operation (compression/decompression or hash)
+        * applied on input. Added for testing purpose.
+        */
+       ODP_COMP_ALG_NULL,
+       /** DEFLATE - RFC1951 */
+       ODP_COMP_ALG_DEFLATE,
+       /** ZLIB - RFC1950 */
+       ODP_COMP_ALG_ZLIB,
+       /** LZS */
+       ODP_COMP_ALG_LZS
+} odp_comp_alg_t;
+
+/**
+ * Comp API session creation return code
+ *
+ */
+typedef enum {
+       /** Session created */
+       ODP_COMP_SES_CREATE_ERR_NONE,
+       /** Creation failed, no resources */
+       ODP_COMP_SES_CREATE_ERR_ENOMEM,
+       /** Creation failed, bad compression params */
+       ODP_COMP_SES_CREATE_ERR_INV_COMP,
+       /** Creation failed, bad hash params */
+       ODP_COMP_SES_CREATE_ERR_INV_HASH,
+       /** Creation failed,requested configuration not supported*/
+       ODP_COMP_SES_CREATE_ERR_NOT_SUPPORTED
+} odp_comp_ses_create_err_t;
+
+/**
+ * Comp API operation return codes
+ *
+ */
+typedef enum {
+       /** Operation completed successfully*/
+       ODP_COMP_ERR_NONE,
+       /** Operation paused due to insufficient output buffer.
+        *
+        * This is not an error condition. On seeing this situation,
+        * Implementation should maintain context of in-progress operation and
+        * application should call packet processing API again with valid
+        * output buffer but no other alteration to operation params
+        * (odp_comp_op_param_t).
+        *
+        * if using async mode, application should either make sure to
+        * provide sufficient output buffer size OR maintain relevant
+        * context (or ordering) information with respect to each input packet
+        * en-queued for processing.
+        *
+        */
+       ODP_COMP_ERR_OUT_OF_SPACE,
+       /** Invalid user data pointers */
+       ODP_COMP_ERR_DATA_PTR,
+       /** Invalid input data size */
+       ODP_COMP_ERR_DATA_SIZE,
+       /**  Compression Algo failure */
+       ODP_COMP_ERR_ALGO_FAIL,
+       /** Error if operation has been requested in an invalid state */
+       ODP_COMP_ERR_INV_STATE,
+       /** Error if API does not support any of the operational parameter. */
+       ODP_COMP_ERR_NOT_SUPPORTED,
+       /** Error if session is invalid. */
+       ODP_COMP_ERR_INV_SESS
+} odp_comp_err_t;
+
+/**
+ * Comp API enumeration for preferred compression level/speed. Applicable
+ * only for compression operation not decompression.
+ * Value provided defines a trade-off between speed and compression ratio.
+ *
+ * If compression level == ODP_COMP_LEVEL_MIN, output will be produced at
+ * fastest possible rate,
+ *
+ * If compression level == ODP_COMP_LEVEL_MAX, output will be highest possible
+ * compression,
+ *
+ * compression level == ODP_COMP_LEVEL_DEFAULT means implementation will use
+ * its default choice of compression level.
+ *
+ */
+typedef enum {
+       /* Use implementation default */
+       ODP_COMP_LEVEL_DEFAULT = 0,
+       /** Minimum compression (fastest in speed) */
+       ODP_COMP_LEVEL_MIN,
+       /** Maximum compression (slowest in speed) */
+       ODP_COMP_LEVEL_MAX,
+} odp_comp_level_t;
+
+/**
+ * Comp API enumeration for huffman encoding. Valid for compression operation.
+ *
+ */
+typedef enum {
+       /** use implementation default to choose between compression codes  */
+       ODP_COMP_HUFFMAN_CODE_DEFAULT = 0,
+       /** use fixed huffman codes */
+       ODP_COMP_HUFFMAN_CODE_FIXED,
+       /** use dynamic huffman coding */
+       ODP_COMP_HUFFMAN_CODE_DYNAMIC,
+} odp_comp_huffman_code_t;
+
+/**
+ * Hash algorithms in a bit field structure
+ *
+ */
+typedef union odp_comp_hash_algos_t {
+       /** hash algorithms */
+       struct {
+               /** ODP_COMP_HASH_ALG_SHA1 */
+               uint32_t sha1  : 1;
+
+               /** ODP_COMP_HASH_ALG_SHA256 */
+               uint32_t sha256 : 1;
+
+       } bit;
+
+       /** All bits of the bit field structure
+        *
+        * This field can be used to set/clear all flags, or bitwise
+        * operations over the entire structure.
+        */
+       uint32_t all_bits;
+} odp_comp_hash_algos_t;
+
+/**
+ * Comp algorithms in a bit field structure
+ *
+ */
+typedef union odp_comp_algos_t {
+       /** Compression algorithms */
+       struct {
+               /** ODP_COMP_ALG_NULL */
+               uint32_t null       : 1;
+
+               /** ODP_COMP_ALG_DEFLATE */
+               uint32_t deflate    : 1;
+
+               /** ODP_COMP_ALG_ZLIB */
+               uint32_t zlib       : 1;
+
+               /** ODP_COMP_ALG_LZS */
+               uint32_t lzs        :1;
+       } bit;
+
+       /** All bits of the bit field structure
+        * This field can be used to set/clear all flags, or bitwise
+        * operations over the entire structure.
+        */
+       uint32_t all_bits;
+} odp_comp_algos_t;
+
+/**
+ * Compression Interface Capabilities
+ *
+ */
+typedef struct odp_comp_capability_t {
+       /** Maximum number of  sessions */
+       uint32_t max_sessions;
+
+       /** Supported compression algorithms */
+       odp_comp_algos_t comp_algos;
+
+       /** Supported hash algorithms. */
+       odp_comp_hash_algos_t hash_algos;
+
+       /** Support type for synchronous operation mode (ODP_COMP_SYNC).
+        *  User should set odp_comp_session_param_t:mode based on
+        *  support level as indicated by this param.
+        */
+       odp_support_t sync;
+
+       /** Support type for asynchronous operation mode (ODP_COMP_ASYNC).
+        *  User should set odp_comp_session_param_t:mode param based on
+        *  support level as indicated by this param.
+        */
+       odp_support_t async;
+} odp_comp_capability_t;
+
+/**
+ * Hash algorithm capabilities
+ *
+ */
+typedef struct odp_comp_hash_alg_capability_t {
+       /** Digest length in bytes */
+       uint32_t digest_len;
+} odp_comp_hash_alg_capability_t;
+
+/**
+ * Compression algorithm capabilities structure for each algorithm.
+ *
+ */
+typedef struct odp_comp_alg_capability_t {
+       /** Enumeration indicating algorithm support for dictionary load */
+       odp_support_t support_dict;
+
+       /** Optional Maximum length of dictionary supported
+        *  by implementation of an algorithm.
+        *
+        *  Invalid if support_dict == ODP_SUPPORT_NO.
+        *
+        *  Implementation use dictionary of length less than or equal to value
+        *  indicated by dict_len. if set to 0 and if support_dict ==
+        *  ODP_SUPPORT_YES, then implementation will use dictionary length
+        *  less than or equal to user input length in odp_comp_set_dict()
+        *  and update used dictionary length at output of the call.
+        *
+        */
+       uint32_t dict_len;
+
+       /** Maximum compression level supported by implementation of this algo.
+        * Indicates number of compression levels supported by implementation,
+        *
+        * where,
+        *
+        * 1 means fastest compression i.e. output produced at
+        * best possible speed at the expense of compression quality, and
+        *
+        * max_level means best compression i.e.output produced is best possible
+        * compressed content at the expense of speed.
+        *
+        * Example, if max_level = 4 , it means algorithm supports four levels
+        * of compression from value 1 up to 4. User can set this value from
+        * 1 (fastest compression) to 4 (best compression).
+        * See RFC1950 for an example explanation to level.
+        *
+        * Value 0 mean implementation use its default value.
+        *
+        */
+       uint32_t max_level;
+
+       /* Supported hash algorithms */
+       odp_comp_hash_algos_t hash_algo;


Comment:
Field name should match the type == "hash_algos" (many algorithms in a bit 
field). Underneath "hash_algo" is used with enum odp_comp_hash_alg_t (one algo).

> Petri Savolainen(psavol) wrote:
> I did comment the same a month back:
> 
> Is the data input only? If it is, then this should be "const uint8_t *buf"
> 
> What is expected about the format of the data? Null terminated strings? If 
> it's strings, then e.g. "const char *str" would be better.
> 
> Also the type documentation should be updated:
> * This is not dictionary type, but pointer into dictionary data - right?
> * "Consists of pointer to byte buffer. length of dictionary indicated by 
> length parameter. ", this needs clarification.  What is expected about the 
> data behind the pointer?
>> Petri Savolainen(psavol) wrote:
>> @psavol Hmm. I thought that the user should use this value. And 
>> `odp_comp_level_t` should be dropped from the spec.
>>> Petri Savolainen(psavol) wrote:
>>> How this relates to odp_comp_level_t enumeration ? Shouldn't user use those 
>>> levels and not a number from 0 to this one ?
>>>> Petri Savolainen(psavol) wrote:
>>>> This is description of how de/-compression operation work. This should be 
>>>> specified at operation documentation, and optimally only there. Otherwise, 
>>>> we are in danger of this spec getting deprecated (== in conflict with 
>>>> operation documentation) without noticing. 
>>>>> Petri Savolainen(psavol) wrote:
>>>>> True, all types need to be odp_ prefixed. Also this type would be better 
>>>>> to typedef and pull out (from inside of algo_param_t) since it's used in 
>>>>> two places already.
>>>>>> Petri Savolainen(psavol) wrote:
>>>>>> As a side note: it might be logical, we might want to add 
>>>>>> `odp_support_t` field to notify user if implementation supports stateful 
>>>>>> compression. I can easily assume that there might be implementations 
>>>>>> implementing only stateless compression. 
>>>>>>> Petri Savolainen(psavol) wrote:
>>>>>>> odp_comp_alg_deflate_param, please.
>>>>>>>> Petri Savolainen(psavol) wrote:
>>>>>>>> Please check the see list. It mentions non-existing functions.
>>>>>>>>> Petri Savolainen(psavol) wrote:
>>>>>>>>> no result here
>>>>>>>>>> Petri Savolainen(psavol) wrote:
>>>>>>>>>> s/Else/Otherwise/
>>>>>>>>>>> Petri Savolainen(psavol) wrote:
>>>>>>>>>>> ... an error....
>>>>>>>>>>>> Petri Savolainen(psavol) wrote:
>>>>>>>>>>>> ... with valid  pkt_out and pkt_in being ODP_PACKET_INVALID...
>>>>>>>>>>>>> Petri Savolainen(psavol) wrote:
>>>>>>>>>>>>> It is an error to call this api when session was created in 
>>>>>>>>>>>>> ODP_COMP_ASYNC mode.
>>>>>>>>>>>>>> Petri Savolainen(psavol) wrote:
>>>>>>>>>>>>>> As said, single and multi versions are possible, but not as 
>>>>>>>>>>>>>> proposed in this patch. Both versions should have the same param 
>>>>>>>>>>>>>> structure and specification. Only difference would be pkt_in[] 
>>>>>>>>>>>>>> vs pkt_in, pkt_out[] vs pkt_out, param[] vs. param ...
>>>>>>>>>>>>>>> Petri Savolainen(psavol) wrote:
>>>>>>>>>>>>>>> Then I propose to add a new API like
>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>> odp_comp_compress_multi(const odp_packet_t pkt_in[], 
>>>>>>>>>>>>>>> odp_packet_t pkt_out[], const odp_comp_compress_param_t 
>>>>>>>>>>>>>>> param[], int num_pkt);
>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>> which takes of handling multiple packets and retain current 
>>>>>>>>>>>>>>> version for single packet handling.
>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>> Thanks
>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>> Shally
>>>>>>>>>>>>>>> @bala-manoharan 
>>>>>>>>>>>>>>>> Petri Savolainen(psavol) wrote:
>>>>>>>>>>>>>>>> Results for both sync and async through result function:
>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>> int odp_comp_result(odp_comp_packet_result_t *result, 
>>>>>>>>>>>>>>>> odp_packet_t packet);
>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>> OR have the results as output parameter. Results function is 
>>>>>>>>>>>>>>>> needed any way for the async operation.
>>>>>>>>>>>>>>>>> Petri Savolainen(psavol) wrote:
>>>>>>>>>>>>>>>>> int odp_comp_compress(const odp_packet_t pkt_in[], 
>>>>>>>>>>>>>>>>> odp_packet_t pkt_out[], const odp_comp_compress_param_t 
>>>>>>>>>>>>>>>>> param[], int num_pkt);
>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>> Single packet version may be also added, but is not necessary 
>>>>>>>>>>>>>>>>> since couple of additional if-clauses does not significant 
>>>>>>>>>>>>>>>>> overhead when compare CPU cycles of the entire comp operation.
>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>> int odp_comp_compress_single(odp_packet_t pkt_in, 
>>>>>>>>>>>>>>>>> odp_packet_t pkt_out, const odp_comp_compress_param_t param);
>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>> The two prototypes would be the same except [] and num_pkt.
>>>>>>>>>>>>>>>>> 
https://github.com/Linaro/odp/pull/156#discussion_r137511518
updated_at 2017-09-07 11:17:42

Reply via email to