> On 13 Jul 2016, at 12:21, Max <[email protected]> wrote:
> 

Hi all,

> After looking at the code, I don't think reusing table implementation
> would be easy - the approaches are too different as well as underlying
> data structures.

okay. then we will need to use the tree based version and from my point of view 
we should do it the following way:

* Remove osmo_t4_decode (and related routines) from libosmocore (last step)
* Make the tree based decoder ready in terms of the surrounding style
* Adopt/Clone the osmo_t4_decode tests and move them to the PCU (as part of the 
commit that adds the decoder to the PCU)

@Me: After the infrastructure is in the PCU I will remove osmo_t4_decode (and 
tests) from libosmocore

@Radisys:

I am afraid the tree based decoding is not ready yet and as it impacts the 
upcoming encoder (and other RLE code as far as I understand) let me put it here.


In osmo-pcu (and all other projects) we use libtalloc for memory allocations. 
This means the tree should use talloc with proper parent/child allocations. 
This way the destructor of the decoder will also free all nodes of the tree. 
There should be no call to malloc/free in the PCU.


Please take the tests from libosmocore and make sure they work with the new 
decoder. Tests are the lifeline and we need more and not less of them.


The decoder needs to be robust against invalid input. search_runlen can fail 
but the caller doesn't check for that. Please test with invalid/truncated or 
broken inputs to see we don't end in a never terminating while loop. Test using 
unit tests.
Similar the current  osmo_t4_decode can return an error that is propagated back 
to the compressed bitmap decoding and we should have the same.


Reduce code duplication. With the decoder the only difference between the 
if/else is if the data is filled with 0x00 or 0xFF and which table/root to use. 
Instead of having code clones please parameterize either by having local 
variables in the beginning or by calling different (inline) functions.



kind regards
        holger

Reply via email to