search

esa / CCSDS_MO_TRANS

REPOSITORY ARCHIVED - for the latest version please go to https://github.com/esa/mo-services-java
Other
3 stars 7 forks source link

How to use two decoder/encoder per message? #8

Closed gbonnefille closed 7 years ago

gbonnefille commented 7 years ago

In order to implement TCP/IP transport as drafted in Mission Operations - Message Abstraction Layer Binding to TCP/IP Transport and Split Binary Encoding Draft Recommended Standard, we have to use two distinct decoders per message:

One part of the solution seems to be the creation of a dedicated StreamFactory in order to control the decoding/encoding logic. The code seems quite ready for that because many constructors allow specific feature for sub-classes. But there is an underlaying problem: the implementation of these decoder/encoder are not able to share a single InputStream. The reason is both use an internal buffer. So, the first decoder read as many bytes as possible, do is job and when the second try to do is job, it reclaims bytes from the InputStream, but it is empty.

Is there any recommended approach to do this?

In our side, we imagine that the logic of the buffered input should be extracted from encoder/decoder and ported by the InputStream directly. By this way, we will be able to share the same buffered input between FixedBinaryDecoder and SplitBinaryDecoder.

SamCooper commented 7 years ago

The SPP encoding need to also do this, take a look here to see how it does it:

https://github.com/esa/CCSDS_MO_TRANS/blob/master/CCSDS_MAL_TRANSPORT_SPP/src/main/java/esa/mo/mal/transport/spp/SPPMessageHeader.java#L274

It may not be perfect in which case you may need to make modifications but if you do this I should be able to bring those changes back into the main tree as long as they are done in a portable way.

gbonnefille commented 7 years ago

In our case, we don't want to duplicate an existing decoder but reuse directly the existing ones. To do this, we just want to implement the MALElementInputStream and redirect to the correct decoder.

IMHO, the part of the design which should be revised is to remove the responsibility of the cache management from the decoders, to let the given InputStream manage this (for example by wrapping it with a BufferedInputStream. Doing this, we can simply share this InputStream between two decoders.

I can try to prepare a branch with such changes in order to let you appreciate this design.

SamCooper commented 7 years ago

I understand your point of view, but the java InputStreams are very inefficient so the cache management was moved into the decoders for performance reasons.

A better approach might be to move to nio but that would require a change to the MAL API.

gbonnefille commented 7 years ago

At the same time I realize that my comments are against an old version. Since this version, you introduced the InputReader which can be shared. So, this probably help to follow our approach.

We will have to do some significant effort to merge with new version. :-)