Bug 1629 - Protocol decoding only done for first frame
Summary: Protocol decoding only done for first frame
Alias: None
Product: PulseView
Classification: Unclassified
Component: Protocol decoding (show other bugs)
Version: unreleased development snapshot
Hardware: All All
: Normal normal
Target Milestone: ---
Assignee: Nobody
Depends on:
Reported: 2020-10-25 07:51 CET by Philipp Marek
Modified: 2020-12-06 21:27 CET (History)
2 users (show)


Note You need to log in before you can comment on or make changes to this bug.
Description Philipp Marek 2020-10-25 07:51:05 CET
When using a device that captures multiple frames (I'm using a Hantek DSO-2250), the protocol decoders are used only for the first frame.
Comment 1 Soeren Apel 2020-11-23 23:53:31 CET
Thanks for the report, this should now be fixed using 	9431e2d3d256f3602c3637847a8ec3ad3fdcd590. Please verify that it's now working for you.
Comment 2 Philipp Marek 2020-11-30 19:46:12 CET
Well, behaviour _has_ changed ... but not for the better.

Still only frame 1 decoded; and CPU use is now at maximum (4 CPUs + HT):

    PID PR  NI    VIRT    RES    SHR S  %CPU  %MEM     TIME+ COMMAND                                                                                         
 133298 20   0 1780004 128948  61948 S 794,0   0,8   1:10.75 pulseview
Comment 3 Ralf 2020-12-06 19:45:20 CET
I have the same behavior with MSO5000 and investigated a bit.
Same as Philipp I think patch 9431e2d3d256f3602c3637847a8ec3ad3fdcd590 is going the wrong way. It causes 2 threads to run continuously for each analog channel converted to logic.

The error has differences when using logic channels from LA or analog channels converted to logic channels (one more conversion level).
It also depends on the number of packets (SR_DF_ANALOG/SR_DF_LOGIC) received for a frame.

You can find my current working version on github: https://github.com/jr-oss/pulseview/tree/bug_1629
I need to do some cleanup before starting a PR, but would also welcome your feedback for my draft version.

It also fixes a minor annoyance showing too many "Error cleared" messages
I could start a separate PR for this.
Comment 4 Soeren Apel 2020-12-06 20:46:37 CET
While I generally appreciate when people want to contribute to PV and/or sigrok in general, I really don't understand those who decide to make considerate code changes without consulting the maintainer first to check if they have a chance of being approved and merged.

You wasted your time because you didn't, sad to say.

I worked on this bug yesterday, I'll push my fix after testing it more thoroughly.
Comment 5 Ralf 2020-12-06 21:27:27 CET
I don't think it was a waste of time, because I learned from it and I am looking forward to your solution.
Since I was not sure that I could actually contribute as I am not familiar with Qt, I did not consider asking beforehand. It is more of a workaround than a solution, anyway.

Lessons learned: Do you think, that this unrelated commit has a chance of being accepted? Shall I create a new bug report for it?