Created attachment 100118 [details] [review]
upport for assigning realtime priority

This patch adds support for assigning realtime priority to pipeline (pad)
threads considered critical, like audio pipelines.

This should be added to GstTask (as streaming threads do not have to be related to a GstPad). There is also the GST_MESSAGE_STREAM_STATUS message that should notify the app when and where a stream starts so that the app can adjust the thread priority. This probably would also need to include some sort of identification of the stream so that all "audio" or "network capture" streams can have their priority boosted. I would like to make this more configurable instead of making it hardcoded. Also running arbitrary streaming threads with realtime priority might not be a good idea.

Created attachment 133165 [details] [review]
first implementation of STREAM_STATUS

First patch to implement STREAM_STATUS messages. These messages will notify the app about streaming threads and allows it to interact and control these threads.
Has design docs, unit-tests, examples, etc..

Created attachment 133252 [details] [review]
updated patch

- Removes thread create/destroy from the task into a new GstTaskPool object that is easily subclassable into a custom threadpool.

Created attachment 133255 [details] [review]
task: Add function to set task priority

I'm proposing a different approach to RT threads. Instead of having a separate thread pool, we can instead just set the priority of the thread when a Task is executed and reset it afterwards, that then allows us to change the priority of a task while its running. That means we can set the priority of a whole series of interconnected tasks using events. In my case, I want the tasks that will be used to receive audio to be RT, but not video (for example), so I can just send the event up the pipeline when I add the audio sink, without having to go fish into rtpbin or decodebin.

Created attachment 133256 [details] [review]
pad: Add real-time event and set task according to it

Comment #5 and Comment #6: A slightly different use case here but certainly something that we also should support.

The patch in Comment #4 also allows for setting the thread priority but at a different moment, when the thread is first entered (_ENTER) or when the thread is created (_CREATED). See also the example stream-status.c. It's slighly harder to use because you have to catch the messages and then figure out if the thread is part of the pipeline you want to change the priority of. The advantage is that you can raise the priority before dataflow happens and while autoplugging is busy constructing stuff. Likewise you can lower the priority again when the thread is DESTROYED.

The messages also have the added benefit that you can also handle internal threads created by an element (the threads to write to an audio device come to mind)

The GstTaskPool idea is a bit more involved. If you want to do serious realtime threading you also likely want to have a watchdog thread to kill any stray threads in order to not lock up your system. This is something that a realtime threadpool can do in the background. The taskpool is also usefull if you need threads with specific stacksizes or other features that are not configurable after the thread is created and running.

Configuring the priorities with an event seems to be the easiest but least flexible way to do things. You would likely send the event after the pipeline is autoplugged up to the sink, in which case dataflow already happened at the normal priority and you missed your chance to raise the priority earlier.

Also an event requires you to run the entire src -> sink path with the same priority. For RTP, for example, it could make sense to only run the receiver parts with a high priority up to the jitterbuffers in order to not lose packets, but after that regular scheduling might work just fine (or do this just for the video part and run the complete audio part at higher priority).

This is just to give you a bit more background info for what this patch wanted to support. I'll add your event based patch too, likely by making a new core EVENT_PRIORITY type. It also needs to be more configurable as it currently just sets the priority to one of the realtime variants.

One thing missing from your implementation is being able to change the priority of an already running thread may also be useful. For example, we may not know if a file contains audio or video before it has been demuxed. Or to implement event-based handling, the event will probably not be produced before the threads are started if there is autoplugging. I'm not really sure how to implement that in a flexible way.

As for the event based stuff, maybe we could have a generic event that triggers a callback for each pad traversed (or each pad that has a task), that said, this wont work for non-pad tasks.

For changing the priorities of running tasks, you could keep a ref to all the stream-status messages and work from there. It would theoretically allow you to put a caps notify on those pads referenced in the message (for tasks started by pads) and then conditionally raise priorities based on the media type as the pads become negotiated. A bit clumsy but doable. Or the other way around, look for deep-notify::caps and then do something with the related tasks from the pad that fired it. 

I was also considering a getter for the task of a pad. The only problem is that you don't really know when a pad has a task unless you look at the messages. And if you need to keep the messages for that, you already have the task.

Even for the Tasks that don't have pads, the element could intercept the priority events on certain pads and raise related internal threads, if needed.

What could be interesting is some sort of probe event that would be insterted in the dataflow and would trigger something on each pad it traveled through, not sure what the use case here is or how to link that to the tasks.

Or another idea: have a query function you can do on a pad that returns the task that does the streaming on this pad, it'll go upstream until the task is found. You probably need this if you want to use the deep-notify on pads without tasks.

In practice it's not really a problem, you can add pad probes, pad block or notifies on pads and since they are all done from the streaming threads, you have a handle to the thread right away. We probably need a method to get the task of the current thread for that.

The part about get/set priority in your patch doesn't seem useful, since it seems that g_thread_set_priority doesn't change the policy (and priority is ignored in SCHED_OTHER on Linux at least).. Since GLib doesn't abstract it and its so system specific, leaving it to custom callbacks is probably better..

I'm also not convinced of the utility of the GstTaskPool, it would be easier to just be able to control callback functions in the GstTask (like GstPad), so we could have callbacks for start thread and stop thread, etc.. and the default implementation would call g_thread_pool_push/join, but then I could have custom implementations that just do pthread_create/join (and then I would have the pthread_t so I can change its priority later). I guess having a custom GstTaskPool subclass does the same (but is kind of more complicated since I have to subclass a GObject).. 

And anyway, since GstBaseAudioSink/Src uses g_thread_create/join directly, playing around with GstTask isn't enough to have proper realtime scheduling.

That said, I think I can do all I want by just listening for the ENTER message and do a pthread_self() and then I get the thread for this pad/task. And using the LEAVE message to restore the thread to its original state. I guess internal threads should also emit it (I'm thinking of baseaudio src/sink here).

And then when I discover that I need RT, I can iterate over the pads/elements following their links (using gst_pad_iterate_internal_links & gst_pad_get_peer) and for each one check if I have a message/task for this one and set the properties I want on the thread. Getting the task from the current thread doesn't seem to useful then, since I would already have a list of pad/elem->tasks/threads associations (from the messages). Also, since a task is not always linked to a GThread, getting the Task is not so useful.

Also, looking at the caps is not enough.. for example if I have udpsrc->jb->sink then only the caps of the tasks after the jitterbuffer have caps that can tell me its audio, not the caps before.. So using notify::caps isn't so useful.

I guess we can also forget the probe event for now. I'd prefer getting the STREAM_STATUS messages asap so we can try playing around with them.

I am closing this as we have the stream status stuff.

GLib discussion takes place in bug #687223 now for those interested.

Also setting priorities or anything else on GThread is not possible anymore, thus the stream-status messages lost most of its usefulness

Setting priorities on GThread hasn't actually worked for a long time anyway, it's still useful if you create your own tasks as in the example, no?

If you have sync handler to catch the stream-status enter and leave messages, you can use that to modify the threads priority.