GNOME Bugzilla – Bug 687991
videodecoder: add getter for QoS proportion
Last modified: 2012-11-20 14:09:03 UTC
Add a getter for the QoS proportion, to help subclasses do better estimations based on the proportion.
Created attachment 228572 [details] [review] videodecoder: add getter for QoS proportion Add a getter for the QoS proportion, to help subclasses do better estimations based on the proportion.
Even though it's new API, it would be nice if it can be considered a bug and be included in the 1.0.x series, as otherwise sub classes would need to have a different logic depending on whether it's 1.0.x or >= 1.2.x
*** Bug 687990 has been marked as a duplicate of this bug. ***
Review of attachment 228572 [details] [review]: IMHO can be added to 1.0, that shouldn't be a problem ::: gst-libs/gst/video/gstvideodecoder.c @@ +3170,3 @@ + g_return_val_if_fail (GST_IS_VIDEO_DECODER (decoder), 1); + + return decoder->priv->proportion; Needs to use the object lock here ::: gst-libs/gst/video/gstvideodecoder.h @@ +354,3 @@ GstVideoCodecFrame *frame); +gdouble gst_video_decoder_get_qos_proportion (GstVideoDecoder *decoder); I would do something like gst_video_decoder_get_qos_whatever (decoder, &proportion, &earliest_time, other_fields)
Created attachment 228591 [details] [review] videodecoder: add getter for QoS proportion Add a getter for the QoS proportion, to help subclasses do better estimations based on the proportion. https://bugzilla.gnome.org/show_bug.cgi?id=687991 https://bugzilla.gnome.org/show_bug.cgi?id=687990
Created attachment 228592 [details] [review] videodecoder: add getter for QoS proportion Add a getter for the QoS proportion, to help subclasses do better estimations based on the proportion.
Created attachment 228593 [details] [review] videodecoder: add getter for QoS proportion Add a getter for the QoS proportion, to help subclasses do better estimations based on the proportion.
Sorry for the mess :)
You could also get and store this info in the subclass from the event via the src_event handler, I think. It's fine to pick this into 1.0.x IMHO.
Does the function name sound fine to you, Tim?
Other options might be: * get_qos_state * get_qos_stats
I would prefer _get_qos_info() or _get_qos_stats() over _get_qos_state(). Both sound fine to me.
Ok, so what to do with this? Is there any value in the dropped/processed_frames count (for the subclass) ?
Let's use get_qos_info() then. Don't think the dropped/processed frames are useful for the subclass, also they're not part of the QOS events.
To me there is only one value needed, which is the proportion, as for earliest_time we can use the get_max_decoding_time API. That's why I proposed in my first patch a single getter for this value. It's also more future-proof as if something new is added, it is easier adding a new getter than a get_qos_info_full.
Created attachment 229031 [details] [review] videodecoder: add getter for QoS proportion and earliest_time Add a getter for the QoS proportion and earliest_time to help subclasses do better estimations based on the proportion.
Add 'Since' markers, a 'Returns:' tag, and added the function to the .def file with the exported symbols and docs/libs/*sections.txt. commit 5f55ea1ef31ab598751d2b98b5a66360f647ea1b Author: Andoni Morales Alastruey <ylatuya@gmail.com> Date: Fri Nov 9 15:37:57 2012 +0100 videodecoder: add getter for QoS proportion and earliest_time Add a getter for the QoS proportion and earliest_time to help subclasses do better estimations based on the proportion. API: gst_video_decoder_get_qos_info() https://bugzilla.gnome.org/show_bug.cgi?id=687991
I thought we decided on only adding get_qos_proportion() in the end because of extensibility concerns
I did not have any such concerns (we don't plan any changes there in the near future, so why worry). My impression was that no one felt like making a decision, so I pushed the version that's closest to the event API. If you want to change it to _get_propertion(), that works for me too.
commit 67455643b902a181cc64cb1fecc6bc5587255afc Author: Sebastian Dröge <sebastian.droege@collabora.co.uk> Date: Tue Nov 20 12:21:08 2012 +0100 videodecoder: Return the proportion directly commit a6aba8a97927d5e5c1bf0dd9344d51ca9fd0fdb4 Author: Sebastian Dröge <sebastian.droege@collabora.co.uk> Date: Tue Nov 20 12:08:26 2012 +0100 videodecoder: Rename from get_qos_info() to get_qos_proportion() And only return the proportion. The earliest time already can be retrieved from get_max_decode_time() and by renaming we allow this to be more extensible in the future.