Missing entry in TOC file for first video stimulus in playlist mode.

[davidt0x]

This bug occurs when using video stimulus playlist entries without any timing information specified. The TOC file output after the experiment is usually missing the first video stimulus played during the experiment.

[davidt0x]

Ok, I think I have figured out why this TOC issue is happening with video playlists with no timing info. It happens because when the main video server thread runs it creates the VideoStimPlaylist object:

fly-vr/flyvr/video/video_server.py

Lines 1188 to 1190 in 5dac72a

	playlist_stim = VideoStimPlaylist(*stims, random=random,
	paused=paused if paused is not None else getattr(options, 'paused', False),
	play_item=getattr(options, 'play_item', None))

whose constructor calls self.play_item immediately if not paused:

fly-vr/flyvr/video/video_server.py

Lines 91 to 92 in 5dac72a

	elif not self._paused:
	self.play_item(next(self._playlist_iter))

This play_item method in turn updates the state of the playlist (advances the internal index) and then if and only if the fly_vr shared state variables are set it signals to append to the TOC file.:

fly-vr/flyvr/video/video_server.py

Lines 154 to 159 in 5dac72a

	if self._flyvr_shared_state is not None:
	self._flyvr_shared_state.signal_new_playlist_item(identifier, BACKEND_VIDEO,
	producer_instance_n=producer_instance_n,
	producer_playlist_n=self._child_playlist_n,
	# and a time for replay experiments
	time_ns=self._flyvr_shared_state.TIME_NS)

The problem is that the constructor of the VideoStimPlaylist sets this flyvr_shared_state variable to None by default. This gets fixed later when the next item in the playlist is invoked by the main server loop after picking up the playlist on the message queue:

fly-vr/flyvr/video/video_server.py

Line 1009 in 5dac72a

stim_or_cmd.initialize(self.mywin, self._fps, self.flyvr_shared_state)

It seems to me that the first thing todo would be remove the reference of the flyvr shared state from the VideoPlaylist entirely and put this signaling responsibility onto the server main loop. This seems like a separation of concerns issue. The code for managing the playlist sequencing shouldn't be in charge of signaling anything to the state of the rest of the program. This is how it is in the audio playlist case I believe. There could be something I am missing for why this is tricky in the video case though. Alternatively, we could just pass the flyvr_share_state through to the VideoPlaylistStim constructor. Though, I can't see any real reason this playlist code needs to know about the state of the whole system.

I will try to implement this fix locally and test on the setup.

[mwangq]

hey, I ran another quick test this morning using this config file: config_avd_producer_test.txt

I wanted to test two things:

1. whether the toc file looks the same as the time/do instructions from the config file (close but not quite... there seem to be some extra entries in daq, some lost entries in audio) 20220210_1122.toc.txt

2. whether the producer_playlist_n is being incremented for daq stimuli (it is not. I wanted to test this because yday I noticed the producer_playlist_n is not incremented for audio stimuli either) (also reflected in the sync info in daq.h5 and sound_server.h5)

Originally I wanted to use the toc for two purposes: to get the numerical ID (e.g. 1) for a given playlist item (e.g. v_gratingL) (as described in the flyVR sync doc ) and then also to use the toc to help me figure out which items were played when (because this is not hardcoded if I am randomly selecting and playing items, using an experiment.py).

I can almost figure out which items were played when, from a combo of producer_playlist_n and producer_instance_n in the sync data (assuming each instance_n made its way onto the toc file so I could figure out that producer_instance_n of 1 = v_gratingL). And producer_playlist_n can be roughly back-calculated by looking at the blocks of producer_instance_n. That is true if the stimulus is switching every time. It is not true, for instance, in this config file, where I have some stimuli (e.g. audio) play the same thing while I change another backend (e.g. daq).

Not sure if you prefer to open a separate issue for producer_playlist_n increment. I include the comment here because it is related to users being able to pull out when stimuli were played, which I thought the toc was also supposed to be for. and the "-1" values for producer_playlist_n show up in the daq/audio lines of the toc. I glanced at video_server.py and I see the producer_playlist_n increment there; I could probably implement it in sound_server.py but don't want to potentially break anything... (edit: I made some quick changes to sound_server.py and I think I have the producer_playlist_n incrementing now. nothing has broken yet...)

[nzjrs]

a small related fix is that for clarity only one experiment _start should be written to the TOC

fly-vr/flyvr/hwio/phidget.py

Line 165 in bf4718a

if CommonMessages.EXPERIMENT_START in msg:

[nzjrs]

hmm, for backends that output chunks (daq,audio) the produce_playlist_n should be incremented in the chunker/data_generator. A test is here that seems to be testing this

fly-vr/tests/audio/test_samplechunks.py

Line 218 in bf4718a

assert c3.producer_playlist_n == 3

[nzjrs]

(alternative hypothesis is that missing TOC lines are from lost IPC messages - best to increase logging and check each send per backend gets received by the TOC writer)

[davidt0x]

It seems to me the missing entries in TOC are only ones where the stimulus is repeating? Am I missing something? If that is the case, it is probably this line of code that is the culprit:

fly-vr/flyvr/audio/sound_server.py

Lines 398 to 413 in 5dac72a

	if chunk_producers_differ(self._last_chunk, chunk):
	self._log.debug('chunk from new producer: %r' % chunk)
	self.flyvr_shared_state.signal_new_playlist_item(chunk.producer_identifier, BACKEND_AUDIO,
	chunk_producer_instance_n=chunk.producer_instance_n,
	chunk_n=chunk.chunk_n,
	chunk_producer_playlist_n=chunk.producer_playlist_n,
	chunk_mixed_producer=chunk.mixed_producer,
	chunk_mixed_start_offset=chunk.mixed_start_offset,
	# ensure identical values to the h5 row
	fictrac_frame_num=row[0],
	daq_output_num_samples_written=row[1],
	daq_input_num_samples_read=row[2],
	sound_output_num_samples_written=row[3],
	video_output_num_frames=row[4],
	# and a time for replay experiments
	time_ns=row[5])

[nzjrs]

It seems to me the missing entries in TOC are only ones where the stimulus is repeating? Am I missing something? If that is the case, it is probably this line of code that is the culprit:

fly-vr/flyvr/audio/sound_server.py

Lines 398 to 413 in 5dac72a

	if chunk_producers_differ(self._last_chunk, chunk):
	self._log.debug('chunk from new producer: %r' % chunk)
	self.flyvr_shared_state.signal_new_playlist_item(chunk.producer_identifier, BACKEND_AUDIO,
	chunk_producer_instance_n=chunk.producer_instance_n,
	chunk_n=chunk.chunk_n,
	chunk_producer_playlist_n=chunk.producer_playlist_n,
	chunk_mixed_producer=chunk.mixed_producer,
	chunk_mixed_start_offset=chunk.mixed_start_offset,
	# ensure identical values to the h5 row
	fictrac_frame_num=row[0],
	daq_output_num_samples_written=row[1],
	daq_input_num_samples_read=row[2],
	sound_output_num_samples_written=row[3],
	video_output_num_frames=row[4],
	# and a time for replay experiments
	time_ns=row[5])

The simple sequential case seems to be well enough tested here https://github.com/murthylab/fly-vr/blob/master/tests/audio/test_samplechunks.py

but I think this is the right track. I believe I know what is going on.

the time based experiment plays items, which hits this

fly-vr/flyvr/audio/sound_server.py

Line 203 in 5dac72a

self.data_generator = self._stim_playlist.play_item(stim)

which creates a new data_generator

but I think this check

fly-vr/flyvr/audio/signal_producer.py

Line 64 in 5dac72a

return this.mixed_producer or (prev.producer_identifier,

does not catch what is a new chunk from a new data generator. I would suggest to confirm that producer_instance_n is incremented correctly resulting from the and also check producer_instance_n equality in chunk_producers_differ. however, I'm AFK from my windows dev PC - so it could be in fact that this was not the design of producer_instance_n and so chunk_producers_differ should also check the hash() of the creating data generator to handle this case

(I can test next week FWIW)

[nzjrs]

(I can test next week FWIW)

for the above, a hello world test would be to make a test that essentially calls play_item on a playlist for a non sequential sequence of items in the playlist, and checks the subsequent first chunks from the returned generatorS are detected as chunk_producers_differ() == True

[mwangq]

hmm, for backends that output chunks (daq,audio) the produce_playlist_n should be incremented in the chunker/data_generator.

strange news. I found some .toc from Dec and Jan where the (audio) chunk_producer_playlist_n incremented as expected. all of these experiments were audio backend only, using a variety of stimuli (custom matlab made stimuli and "sin" item types). I wonder if the .toc issue I showed above (where the chunk_producer_playlist_n are all -1, for both daq and audio) might have something to do with multiple backends...

[nzjrs]

hmm, for backends that output chunks (daq,audio) the produce_playlist_n should be incremented in the chunker/data_generator.

strange news. I found some .toc from Dec and Jan where the (audio) chunk_producer_playlist_n incremented as expected. all of these experiments were audio backend only, using a variety of stimuli (custom matlab made stimuli and "sin" item types). I wonder if the .toc issue I showed above (where the chunk_producer_playlist_n are all -1, for both daq and audio) might have something to do with multiple backends...

I would expect this to have worked if the experiment was not dynamic - i.e. didn't use the experiment time stanza. If it was simply a predefined sequential playlist that played in order, then all the chunk_producer_playlist_n and friends will all be incremented correctly

that is, as I said #16 (comment) it's probbably a function of tearing down and recreating the data_generator which confuses the chunker change detection

[nzjrs]

I believe all issues here should be fixed by these 2 commits

• 650bb60
• 5722d59

I could only test this on Linux so the commits should be cherry picked appropriately and the branch not merged wholesale.