nrkno / sofie-core

Sofie Core: A Part of the Sofie TV Studio Automation System
https://github.com/nrkno/Sofie-TV-automation/
MIT License
124 stars 40 forks source link

Bug Report: Device Triggers temporarily go blank #1197

Open ianshade opened 1 month ago

ianshade commented 1 month ago

About me

This bug report is posted on behalf of TV 2 Norge.

Observed Behavior

Keys on a Stream Deck device connected to Core via Input Gateway go black for a few seconds (sometimes upwards of 10) as a result of many actions performed in a relatively short time. It is easily reproducible when performing many AdLib Actions that insert new Parts, and it seems more frequent when there are many Parts in the Segment. It can also be observed in the Test Tools > Device Triggers section in Sofie, where the list goes empty at the same time the issue occurs on the device, therefore I'm reporting this as a Core bug, likely something to do with the custom publications used by this feature.

https://github.com/nrkno/sofie-core/assets/9103996/7cb5a3ed-7933-4a32-91a7-1d4f61214d6e

Expected Behavior

Number of actions performed by the user in a short period (or the number of Parts in a Segment) should not cause the mounted triggers to go blank for multiple seconds.

Version

Sofie Core: release50 (aa69e5d, as well as some earlier ones), Input Gateway: develop (be957f04)

Severity / Impact

No response

Level of Involvement

No response

Julusian commented 1 month ago

I think I have identified this to be triggered by the use of takeAfterExecuteAction

This appears to be caused by a race condition in https://github.com/nrkno/sofie-core/blob/master/meteor/server/api/deviceTriggers/StudioObserver.ts#L92-L95 not finding a PartInstance upon its first run, resulting in it not finding a showStyleBase to use. I assume this is a side effect of how we are saving data to the database, with the RundownPlaylist collection being written before the PartInstances, causing this race condition

I think I have a simple fix, we don't need anything on the PartInstance other than the rundownId, which we are now also storing with the partInstanceId on the playlist isnide of the currentPartInfo and other properties, so we can skip loading the PartInstance entirely and rely on that id instead.

Julusian commented 1 month ago

Fix is pushed, please re-test to confirm this is resolved