Unconverted samples can happen due to a race condition: being notified
of new samples coming in is ignored when the thread is already executing.
If these notifications however were the last ones sent out because the
segment was completed right after, the currently ongoing conversion
will finish but not be restarted with the final sample range.
With this patch, this is prevented.
Passing segment instances fails because this creates a race condition.
When a long conversion is taking place, the SignalBase::samples_added
signal is called often but since it's in a separate thread, the calls
are queued and aren't executed immediately. Now if the conversion is
restarted - for example as a result of a changed conversion threshold -
then the segments holding the converted data are destroyed, rendering
the pointers submitted as parameters to samples_added invalid.
Once the signal queue is processed, those invalid pointers will be
accessed and PV segfaults.
Since the signal queue can neither be emptied nor flushed, this
leaves only two sensible choices:
1) Signal samples_added less often, thereby reducing the chance of
signals being queued
2) Supply the segment ID instead of the segment instance as that's
essentially the only thing we currently care about - in fact, the
only user of samples_added (ViewBase::on_samples_added) uses the
instance to query only this
As #1 is only a band-aid and not a waterproof solution, I chose
to go with #2.
This way, we can use the same mechanism for changing
min/max as well, preventing multiple successive starts
of the conversion algorithm.
Preventing this is necessary because it makes the UI
stop updating for a significant amount of time, which
we obviously don't want.
Before, the converted_data_ container was only created
once we had sample data coming in. This meant that it
wasn't possible to assign a converted signal to a decoder.
With this change, the data container is created even
when there is no data to fill it with, allowing logic_data()
to return a valid result and in turn allowing the user
to assign the signal to a decoder.
Multiple changes in one commit due to complexity:
1) data::decode::Decoder: Make use of the DecodeChannel struct for
channel assigments
2) DecodeSignal: Store DecodeChannel list in vector, not map
3) DecodeSignal: Remove unused get_data()
4) Remove boost::optional usage
5) Use DecodeSignal::segment_ as the container for muxed
logic data
6) Implement the DecodeSignal::logic_mux_proc thread and its
helper method mux_logic_samples()
7) Update DecodeSignal::decode_proc() to interface with the
logic muxer thread
8) Remove no longer needed DecodeSignal::wait_for_data()
Several changes make up this commit, which unfortunately
can't be separated:
1) Move decoder stack management from DecoderStack to
DecodeSignal, thereby making DecoderStack unnecessary
2) Change the decoder stack from std::list to an
std::vector for direct decoder access
3) Introduce logic_mux_thread which will take care
of muxing the individual SignalBases' logic data into
(cached) logic data that libsigrokdecode expects.
This is necessary as we can no longer do simple bit
mapping within a single logic data segment's logic
data as we now may feed from multiple logic data
segments at once
4) Refactored the creation of decode traces, making
it more streamlined and flexible while simplifying
the class interface
5) Refactored the auto-assignment of channels
6) Refactored is_decode_signal()
7) Reworked decode signal save/restore, allowing
proper handling and with the decoder stack now
being part of the signal, easier save/restore of
the stack and its settings
For starters, we equip it with some basic wrappers
around the decode stack, rework the annotation signal
a little and use the new DecodeSignal in favor of the
SignalBase class.
Use "using std::foo" to make the actual code itself a lot more readable.
There are some exceptions where we usually cannot do this, e.g. std::thread
often conflicts with "thread" from Qt or Boost.
Soeren Apel
Uwe Hermann
Gerhard Sittig