roundware / roundware-server

Location-based contributory audio AR platform for art, education and documentary
http://roundware.org
Other
33 stars 20 forks source link

modify_stream causes stream to be unresponsive after playlist empties #238

Closed hburgund closed 9 years ago

hburgund commented 9 years ago

As I was doing some extensive testing on a fresh local vagrant build on the current develop branch, I discovered the following significant issue. It is 100% reproducible for me and is related to the modify_stream api/1 call. It does not happen when only move_listener calls are made to change the stream and cause assets to be added to the playlist and played; only modify_stream.

Essentially, if modify_stream is sent, all the proper assets play back, but as soon as they are done playing and the playlist is empty, an error occurs and then all subsequent move_listener and modify_stream calls are ignored. Interestingly, it seems like lots of code stops running as the "TickTock" log and other logs stop printing out as well as soon as another stream call is made after the error occurs.

Here are the most elemental steps to reproduce in the vagrant build with the default database and test project:

Traceback (most recent call last):
  File "/vagrant/roundwared/audiotrack.py", line 59, in asset_start_timer
    self.add_file()
  File "/vagrant/roundwared/audiotrack.py", line 109, in add_file
    self.current_recording = self.rc.get_recording()
  File "/vagrant/roundwared/recording_collection.py", line 96, in get_recording
    recording = self._get_recording()
  File "/vagrant/roundwared/recording_collection.py", line 127, in _get_recording
    p = Project.objects.get(id=int(self.request['project_id']))
TypeError: int() argument must be a string or a number, not 'list'

I've tried some other sets of api calls and each time, it seems to be the modify_stream call that causes the problem.

I have tested this on the FAM production server and I believe that I am getting the same behavior, though the logs are different and don't seem to include the same statements, so I can't say for sure.

I have no idea when this happened or how it went unnoticed. I will continue to test more and will update this issue as needed. If it really is just a type error, that should be easy to fix, but I don't know how the project_id would be a list? More verification is important too.

13rac1 commented 9 years ago

This seems to stop the error, but I am too tired right now to test thoroughly. Continuous might not be working. Either way, we need to test for this specific use case.

hburgund commented 9 years ago

I've done extensive testing and you found the problem. It seems that simply getting rid of this line:

p = Project.objects.get(id=int(self.request['project_id']))

and places that refer to it, did the trick. That line does look quite suspicious anyway; I have no idea why you would ever try to get to project data in such a circuitous fashion when it's directly accessible from the model. I'm assuming this was at one point a while back a more sensible approach.

But as you also point out, your quick fix is a bit of a band-aid only because there is a deeper issue with continuous mode. It does not work and when it did work at some point in the past, it wasn't working properly because it wasn't applying tag filters to the playlist when it re-filled after empty, per #96.

I have investigated and believe both continuous mode happening at all as well as it happening with the proper tag filters are fixed. Can you please have a look and if it seems good to you, I'll issue a pull request:

https://github.com/hburgund/roundware-server/commit/eb73fd79276488e354473a49bf481d9322d4f653

I'm not sure if there is a use anymore for the is_continuous function; we should remove if it's not referenced anywhere else...I haven't yet checked.