[Bug 743346] When system time is increased the ongoing RTSP sessions will time out.

[GStreamer (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=743346

Sebastian Rasmussen <sebras at hotmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |sebras at hotmail.com

--- Comment #13 from Sebastian Rasmussen <sebras at hotmail.com> ---
To make the discussion permanent:

09:34 < sebras> slomo: https://bugzilla.gnome.org/show_bug.cgi?id=743346 would
you mind comitting this?
09:35 < sebras> slomo: we haven't seen any input from you for a long time and
are anxious to get it on
                master.
09:43 < slomo> sebras: patch looks good, but what does it help really? all
sessions created by the
               server will still use the realtime clock by default
09:51 < slomo> sebras: it's all ugly
09:51 < sebras> slomo: ?
09:51 < sebras> slomo: I agree that the patch as it stands now does not set the
clocktype.
09:52 < sebras> slomo: we aim to set the clocktype property of GstRTSPSession
from inside our derived 
                GstRTSPServer I believe (I'm in the process of checking this).
09:52 < sebras> slomo: are you saying that you want a similar fix for
gst-rtsp-server as default?
09:54 < slomo> no, we can't change that default unfortunately
09:54 < slomo> see my comment :)
09:54 < slomo> wtay: https://bugzilla.gnome.org/show_bug.cgi?id=743346 your
opinion please :)
09:56 < sebras> slomo: ok, but if we can't change the default then that means
that what this patch
                allows you to do is to set the timeout clocktype to monotonic
instead of the default 
                realtime clocktype for your own derived GstRTSPServer in
anticipation that GStreamer 
                2.0 is released where the default clocktype is changed
realtime->monotonic.
09:57 < sebras> slomo: does that make sense? if so then the patch ougth to be
appealing right now,
                no..?
09:57 < sebras> slomo: it is backwards compatible but you can enable the
correct clocktype and get 
                decent timeouts if you so desire.
09:57 < sebras> slomo: this is the intent at least.
09:59 < slomo> yes
09:59 < slomo> i just think it's still ugly that by default everything is
broken :)
09:59 < sebras> slomo: yeah I agree, but that's probably wtay's mistake back in
2012 or something. at 
                least this patch sets us on the right track.
10:00 < slomo> and also that every user of that API now has to check the clock
type
10:00 < slomo> it would be nicer if that could be isolated to rtsp-session.c
10:00 < wtay> why even check for realtime?
10:01 < wtay> the new api works with monotonic time and that's it
10:01 < wtay> and the old api with realtime
10:03 < slomo> wtay: how would that look like API wise?
10:03 < sebras> I'm not sure I get it now.
10:03 < sebras> rtsp-session is asked when the next timeout will occur, right?
10:04 < sebras> so it must know what clock to use to determine when that is.
10:04 < sebras> if rtsp-session is supposed to know this own its own, then you
must have some way to 
                tell rtsp-session this.
10:05 < wtay> if you use old api, it's the system clock
10:07 < slomo> what would the session pool do? what would happen if you mix old
and new API? e.g.
               session pool using old, application using new
10:08 < sebras> slomo: exactly, it only calls gst_rtsp_session_next_timeout()
currently.
10:08 < sebras> slomo: after the patch it may call old or new api.
10:11 < sebras> slomo: wtay: I guess the underlaying problem is that it is the
caller of
                gst_rtsp_session_next_timeout() that provides the "now"
timestamp, it is not collected 
                by rtsp-session on its own.
10:11 < wtay> in _touch() you save both current time and monotonic time, in
_next_timeout() you 
              calculate time against system clock, in _next_timeout_usec() you
calculate against 
              monotonic clock
10:11 < sebras> wtay: how would rtsp-session-pool know which api to call?
10:12 < wtay> it can call both as long as it gives either a system time or
monotonic time
10:12 < wtay> but I would change it to call the new method
10:13 < sebras> wtay: what is the "it" in "change it"?
10:13 < wtay> rtsp-session-pool
10:13 < wtay> make it use the new api so that it uses the monotonic clock
10:13 < sebras> wtay: ok, but then it wouldn't be backwards compatible,
right..?
10:14  * sebras is sure he's missing something vital...
10:14 < wtay> you think people expect weird timeouts when the clock is changed?
10:14 < sebras> wtay: well, that is the problem we are attempting to fix.
10:14 < wtay> I think it's a bug
10:14 < sebras> wtay: it is!
10:15 < wtay> so fix it!
10:15 < sebras> wtay: we tried. :)
10:15 < wtay> if you fix the bug, of course the old broken behaviour is no more
10:15 < sebras> wtay: ok, now you and slomo appear to disagree about whether to
keep the old 
                behaviour. why is that?
10:16 < slomo> because we're not the same person ;) ok, so what about this
then?
10:17 < sebras> slomo: yeah I know, but ideally you guys would agree. it makes
it much easier. :)
10:17 < slomo> the GTimeVal API continues with realtime, the new API uses
monotonic time. all calls to 
               the old API cause a g_warning() to be printed
10:17 < slomo> rtsp-session-pool uses the new API
10:17 < slomo> and otherwise like wtay just said
10:17 < slomo> and no clock selection API anymore :) which btw was wtay's
suggestions some weeks ago ;)
10:18 < sebras> slomo: oh, I though that suggestion came from you. my bad.
10:18 < wtay> I think it's ok to change behaviour here in this case, I can't
see how the previous
              behaviour was desirable
10:23 < slomo> sebras: can you update the bug with this new information then?
10:24 < sebras> slomo: I can, but I'm not 100% sure I grasp the full issue yet.
we're still discussing
                at our end and trying to understand what you and wtay actually
meant. :)
10:26 < slomo> sebras: next_timeout_usec() uses monotonic clock, the GTimeVal
variant uses the system
               clock. you store both clock values internally
10:26 < slomo> sebras: and the GTimeVal API should be wrapped in #ifndef
GST_DISABLE_DEPRECATED
               block... and probably also print a g_warning() to tell people
that this is a bad idea
10:33 < sebras> slomo: and rtsp-session-pool should be using monotonic clock
and calling
                next_timeout_usec()?
10:35 < slomo> yes

-- 
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.