GNOME Bugzilla – Bug 774149
nlecomposition: De-duplicate seek events based on their sequence number
Last modified: 2016-11-12 08:43:07 UTC
See commit message
Created attachment 339400 [details] [review] nlecomposition: De-duplicate seek events based on their sequence number If there are e.g. multiple video sinks, we would get the same seek event multiple times. But we only want to handle it once.
Review of attachment 339400 [details] [review]: Looks good.
Attachment 339400 [details] pushed as ff6dc38 - nlecomposition: De-duplicate seek events based on their sequence number
Created attachment 339482 [details] [review] nlecomposition: Fix small remaining race in previous commit The seek action might currently be handled (in which case it is not in the actions list and the action lock is not locked), but not actually handled completely yet (the seqnum is not stored yet). To prevent this, we remember what the current action is that is being handled, and also compare to that.
Review of attachment 339482 [details] [review]: Would't it make sense to remove the action from the lisyt only when it is done instead of adding that code?
I've not done that to make it more explicit that this very action can't be stopped anymore. If you call remove_actions_to_type() for example and it would remove that action, that would result in a weird situation. Once the action is "current_action", you can observe it but not change it anymore.
(In reply to Sebastian Dröge (slomo) from comment #6) > I've not done that to make it more explicit that this very action can't be > stopped anymore. If you call remove_actions_to_type() for example and it > would remove that action, that would result in a weird situation. > > Once the action is "current_action", you can observe it but not change it > anymore. Well a field on the Action closure would be enough for that, I suspect special casing here is not necessary.
It seemed cleaner to me this way, but that also works. An additional field that would have to be handled by all functions that currently go over that list Should I change it accordingly?
Yeah, right I guess it is good like that too, go ahead.
Comment on attachment 339482 [details] [review] nlecomposition: Fix small remaining race in previous commit Attachment 339482 [details] pushed as acfa0e9 - nlecomposition: Fix small remaining race in previous commit