ratal / mdfreader

Read Measurement Data Format (MDF) versions 3.x and 4.x file formats in python
Other
169 stars 74 forks source link

bug: mdfreader fails on unpack #75

Closed jcfschulz closed 6 years ago

jcfschulz commented 7 years ago

Hi, I encountered a bug in the unpacking of the data. The signal, I am looking at is a 3 byte bitfield. So I'm not sure, if this is truly saved as 3 bytes or if the leading zeros are being truncated somewhere. However, in the end, mdfreader tries to read 4 bytes from a 3 byte string and fails. I attached a patch , that works for me, but since I'm only vaguely familiar with the mdf file format, I don't know, if this actually a good patch :) Unfortunately due to company restrictions, I cannot provide the failing mdf-input file.

I was using mdfreader 0.2.2 (so somewhat outdated) and python 2.7.2.

Greetings, Julius

ratal commented 7 years ago

Hi Julius, since version 0.2.2, several bugs were solved for this kind of issue, so I would advise to update to last version if possible via pip, git, or I can send you by mail. I will have a look at your patch proposal but this is not abnormal that mdfreader tries to read 4 bytes as it can use then numpy records functions that are efficient and then later extract the 3 bytes and eventually other channels out of the 4bytes. When there are strange data arrangements like not aligned bytes or hidden/not used bytes, mdfreader should switch to dataRead cython module that is more safe (still relatively quick) and as failsafe to pure python code (slow). To confirm this switch, you could identify this data arrangement using MDFValidator. Aymeric

ratal commented 7 years ago

Hi Julius, I noticed this code is normally in case of fail safe when dataRead is not working. Was it properly compiled with cython ? I studied your patch and it is solving issue when CFormat is not matching the data given. As it is probably giving a lot of calculation overhead, I would rather recomment to review why CFormat is wrongly defined for the given data (in channel class). Once again, please try with latest version. Aymeric

ratal commented 6 years ago

Since 2 months without feedback, I guess you update to newer version. Closed but if still an issue, reopen.