Bug 1017 - incomplete(?) file import logic, early(?) fatal check for channels
Summary: incomplete(?) file import logic, early(?) fatal check for channels
Status: RESOLVED FIXED
Alias: None
Product: PulseView
Classification: Unclassified
Component: File handling (show other bugs)
Version: unreleased development snapshot
Hardware: All All
: Normal normal
Target Milestone: ---
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-08-20 22:18 CEST by Gerhard Sittig
Modified: 2018-05-21 16:29 CEST (History)
1 user (show)



Attachments
VCD, 2ch, 4Sa, excessive header (150.29 KB, text/plain)
2017-08-21 21:10 CEST, Gerhard Sittig
Details
VCD, 2ch, 4Sa, short content (119 bytes, application/vnd.gtkwave-vcd)
2017-08-21 21:12 CEST, Gerhard Sittig
Details
VCD, 2ch, 4sa, excessive header (syntax fixed) (150.29 KB, application/vnd.gtkwave-vcd)
2017-08-21 21:41 CEST, Gerhard Sittig
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Gerhard Sittig 2017-08-20 22:18:03 CEST
Current Pulseview logic opens the input file, calls read() once and passes 
the chunk of file content to the input module's receive() method, then checks 
for available channels when it "starts acquisition" from the input file.

An internal implementation detail of input modules might result in the 
necessity to run several receive() calls before the required information has 
become available to create the channels. An input module might even defer the 
channel creation until after the end() method was invoked, e.g. in case a 
short file was completely retrieved in a single receive() call, but that 
receive() routine had to return to common code before more data processing 
and sample data submission is allowed to run.

I've observed the issue with a local STF input module that's not yet available 
upstream, but will look into reproducing the issue with e.g. VCD or CSV formats.
Maybe an excessive comment at the start of the file will do.
Comment 1 Soeren Apel 2017-08-20 22:25:31 CEST
Note to self: make PV keep sending data to the input module, don't check for channels after the first receive() call. If something comes back on the session bus, dismiss it with a console warning if the number of channels is still 0. 

Also, emit a warning on the console (+ popup) if the number of channels is still 0 when end() is called.
Comment 2 Gerhard Sittig 2017-08-21 21:09:36 CEST
Have created this "somewhat insane" example file, which sigrok-cli can read 
successfully while pulseview doesn't. The file only has two logic channels 
with four samples each (while current mainline fails to present the very last 
sample, that's in independent omission in the VCD input module).  Sigrok-cli 
output:

  $ sigrok-cli -I vcd -i bug-1016-comment.vcd 
  libsigrok 0.6.0-git-851c80ef368c
  Acquisition with 2/2 channels at 100 MHz
  AUDCK:110
  nAUDSYNC:111

Pulseview reads the file, presents _no_ error (could not get it to popup the 
missing channels dialog), but neither presents traces nor got the samplerate 
(top ruler has sample numbers only).

Reproduction of the issue might be platform depent, varying with how many 
bytes a single read() call will pass to the input module's receive() call?

Input modules which defer sdi_ready signalling and channel creation:
- STF (not in mainline yet)
- WAV (only short header, few bytes)
- VCD (until $enddefinitions and its associated $end were seen)
- CSV (only a single line, until line terminator was seen)

Chronoview immediately raises sdi_ready but is said to see header info "late".
Comment 3 Gerhard Sittig 2017-08-21 21:10:56 CEST
Created attachment 319 [details]
VCD, 2ch, 4Sa, excessive header
Comment 4 Gerhard Sittig 2017-08-21 21:12:05 CEST
Created attachment 320 [details]
VCD, 2ch, 4Sa, short content
Comment 5 Gerhard Sittig 2017-08-21 21:41:52 CEST
Created attachment 321 [details]
VCD, 2ch, 4sa, excessive header (syntax fixed)

vim text fold markers were outside of the comment section in attachment 319 [details]
Comment 6 Gerhard Sittig 2018-05-12 17:26:41 CEST
For the record:  The recent chunk size bump to 4MiB obsoleted this report. 
Only the most insane combinations of file formats with extra large headers 
and input modules that cannot check early magic strings or essential keywords 
would run into the issue described above.  I'd consider the report a non-issue 
with recent mainline.
Comment 7 Gerhard Sittig 2018-05-21 16:29:36 CEST
Closing the report, as real world data should safely get processed, and only 
artificial examples may suffer from the initially reported issue.