GNOME Bugzilla – Bug 754575
trick modes in gst-rtsp-server
Last modified: 2018-11-03 15:39:12 UTC
I am creating this ticket to initiate a discussion about supporting trick modes in the server. The use case I am trying to address is an Onvif client specifying the Frames header field in PLAY: Frames: [intra|all] which is used for limiting transfer bandwidth when playing .mkv recorded files. Don't know if any of you has already thought about it, but what do you think about exposing methods for setting/getting trick modes in GstRTSPMedia like: void gst_rtsp_media_set_trick_modes (GstRTSPMedia * media, GstRTSPTrickModeFlags flags); GstRTSPMediaTrickModeFlags gst_rtsp_media_get_trick_modes (GstRTSPMEdia * media); And then setting GstSeekFlags appropriately when doing the seek from gst_rtsp_media_seek ()?
A slightly different approach would be to modify gst_rtsp_media_seek() to take two more arguments, the seek flags and the rate, which allows for play modes other than the currently available. I addition, if we change GstRTSPClient to include one more virtual function, setup_play_mode(), we allow the implementations to setup the seek in a non-default way as it sees fit.
Created attachment 320604 [details] [review] Makes it possible to tell the rtsp-media how to do the seek
Created attachment 320605 [details] [review] Let implementations decide how to do the seek
Review of attachment 320604 [details] [review]: ::: gst/rtsp-server/rtsp-media.h @@ +257,3 @@ + GstRTSPTimeRange *range, + GstSeekFlags flags, + gdouble rate); API Break. Don't do that. Create a new one instead.
You are right, I didn't think about that. Uploading a new version of the patches. Added a new function gst_rtsp_media_seek_full() which does all the work, and is wrapped by gst_rtsp_media_seek().
Created attachment 320670 [details] [review] Makes it possible to tell the rtsp-media how to do the seek
Created attachment 320671 [details] [review] Let implementations decide how to do the seek
Found a bug in the media patch. Will upload a new patch in a short while.
Created attachment 320889 [details] [review] Makes it possible to tell the rtsp-media how to do the seek
Created attachment 320890 [details] [review] Let implementations decide how to do the seek
Created attachment 322752 [details] [review] Makes it possible to tell the rtsp-media how to do the seek
Sorry for spamming like this, but I realized that I somehow managed to screw up the previous version of this patch. But now it should be OK.
Created attachment 338323 [details] [review] Allow trick mode playback by
Created attachment 338324 [details] [review] Add support for Scale and Speed headers
Created attachment 338325 [details] [review] Allow sub classes to adjust the playback mode
Created attachment 339331 [details] [review] Allow sub classes to adjust the playback mode
Review of attachment 339331 [details] [review]: ::: gst/rtsp-server/rtsp-client.c @@ +1181,3 @@ static GstRTSPStatusCode +default_adjust_play_mode (GstRTSPClient * client, GstRTSPContext * ctx, + GstRTSPTimeRange * range, gdouble * scale, gdouble * speed, gdouble * rate, How about passing range by pointer to pointer? This would allow us to "fix" range if it is missing. VLC likes to send PLAY request with Scale header but without Range header. @@ +1184,3 @@ + GstSeekFlags * flags) +{ + /* FIXME: How to handle the Scale header? According to standard we have to transcode but this usually is not an option at least for me due to performance issues. For now I send all frames as they are and switch to key frames only mode when the scale is high enough. VLC seems to be satisfied with this approach. @@ +1202,3 @@ + + /* as for the speed header, just map it to rate */ + * For formats with delta units, like h264, we could perhaps drop But RTP timestamps are calculated directly from running time (if this hasn't changed since 1.4.5). This means RTP stream will look like it has scale applied to it instead of speed except its bitrate will be multiplied by rate.
(In reply to Nikita Bobkov from comment #17) > Review of attachment 339331 [details] [review] [review]: > > ::: gst/rtsp-server/rtsp-client.c > @@ +1181,3 @@ > static GstRTSPStatusCode > +default_adjust_play_mode (GstRTSPClient * client, GstRTSPContext * ctx, > + GstRTSPTimeRange * range, gdouble * scale, gdouble * speed, gdouble * > rate, > > How about passing range by pointer to pointer? This would allow us to "fix" > range if it is missing. VLC likes to send PLAY request with Scale header but > without Range header. That's a good idea I guess. > @@ +1184,3 @@ > + GstSeekFlags * flags) > +{ > + /* FIXME: How to handle the Scale header? > > According to standard we have to transcode but this usually is not an option > at least for me due to performance issues. For now I send all frames as they > are and switch to key frames only mode when the scale is high enough. VLC > seems to be satisfied with this approach. Well, the Scale header is supposed to increase the viewing rate (assuming it's > 1) but maintain the data rate. By just sending the all data we're not really Scaling anything. Moreover, how to decide what's a high enough Scale value to trigger the key-frames-only approach? By making the function virtual an implementation can decide what's appropriate in it's context. > @@ +1202,3 @@ > + > + /* as for the speed header, just map it to rate */ > + * For formats with delta units, like h264, we could perhaps drop > > But RTP timestamps are calculated directly from running time (if this hasn't > changed since 1.4.5). This means RTP stream will look like it has scale > applied to it instead of speed except its bitrate will be multiplied by rate. I don't quite follow here. As far as I understand the whole purpose of the Speed header is so the client can decide the viewing rate. The following is from RFC2326: "Use of this field changes the bandwidth used for data delivery. It is meant for use in specific circumstances where preview of the presentation at a higher or lower rate is necessary." Isn't that what the patch does? Affects the viewing rate?
(In reply to Branko Subasic from comment #18) > (In reply to Nikita Bobkov from comment #17) > > Review of attachment 339331 [details] [review] [review] [review]: > > > > ::: gst/rtsp-server/rtsp-client.c > > @@ +1181,3 @@ > > static GstRTSPStatusCode > > +default_adjust_play_mode (GstRTSPClient * client, GstRTSPContext * ctx, > > + GstRTSPTimeRange * range, gdouble * scale, gdouble * speed, gdouble * > > rate, > > > > How about passing range by pointer to pointer? This would allow us to "fix" > > range if it is missing. VLC likes to send PLAY request with Scale header but > > without Range header. > > That's a good idea I guess. > > > @@ +1184,3 @@ > > + GstSeekFlags * flags) > > +{ > > + /* FIXME: How to handle the Scale header? > > > > According to standard we have to transcode but this usually is not an option > > at least for me due to performance issues. For now I send all frames as they > > are and switch to key frames only mode when the scale is high enough. VLC > > seems to be satisfied with this approach. > > Well, the Scale header is supposed to increase the viewing rate (assuming > it's > 1) but maintain the data rate. By just sending the all data we're not > really Scaling anything. Well... we are scaling timestamps :-) But yes, you are right. > Moreover, how to decide what's a high enough Scale value to trigger the > key-frames-only approach? By making the function virtual an implementation > can decide what's appropriate in it's context. > Yes. I like your virtual function approach. I used your code and I am doing the work I need by overriding it. I've just shared what I had to do to make VLC work. I understand that it is not what RFC say I should do. Sadly. > > @@ +1202,3 @@ > > + > > + /* as for the speed header, just map it to rate */ > > + * For formats with delta units, like h264, we could perhaps drop > > > > But RTP timestamps are calculated directly from running time (if this hasn't > > changed since 1.4.5). This means RTP stream will look like it has scale > > applied to it instead of speed except its bitrate will be multiplied by rate. > > I don't quite follow here. As far as I understand the whole purpose of the > Speed header is so the client can decide the viewing rate. > > The following is from RFC2326: > > "Use of this field changes the bandwidth used for data delivery. It is > meant for use in specific circumstances where preview of the > presentation at a higher or lower rate is necessary." > > Isn't that what the patch does? Affects the viewing rate? It also affects RTP timestamps which are supposed to mean presentation time. But AFAIK speed should not affect presentation at all. Server just sends same data but possibly in advance. In case of speed timestamps should be incremented by speed * clock_rate per second of wall clock. In our case they are incremented by clock_rate. This is due to running time of pipeline is not affected by rate of segment. So we actually "scale" timestamps. Here is an interesting link: http://ietf.10.n7.nabble.com/RTSP-Speed-and-Scale-overview-text-td191370.html I guess they might be talking about RTSP 2.0. Even if so i don't think that effect of Speed header has changed.
Created attachment 341487 [details] [review] Reverse playback support With this patch VLC can even play formats without predicted frames (like MJPEG) in reverse from our server. For other formats (like H264) it shows only key frames. RTSP server sends frames of GOP in forward order which seems to be conforming to ONVIF streaming specification. Does anyone know RTSP/ONVIF client which can play such streams?
Review of attachment 341487 [details] [review]: ::: gst/rtsp-server/rtsp-stream.c @@ +2758,3 @@ if (format != GST_FORMAT_TIME) *stop = -1; + else There is an indentation issue here.
Created attachment 341496 [details] [review] Reverse playback support
From my understanding the difference between Scale and Speed here is the same as applied_rate and rate in GStreamer. In one case, you have some rate already applied to the stream itself (that is, timestamps are going in real-time but e.g. a difference in timestamps of 1s represents 2s difference in the media timeline), in the other case you're just sending the data faster/slower.
Review of attachment 338323 [details] [review]: Generally looks good to me ::: gst/rtsp-server/rtsp-media.c @@ +2191,1 @@ + if (range != NULL) { Maybe mention that we now allow range!=NULL for seeks to the same position that just change e.g. the rate. In the commit message and a comment here @@ +2214,3 @@ stop_type = GST_SEEK_TYPE_SET; + trick_mode = flags != 0 || rate != 1.0; Why is it trick_mode if flags != 0? GST_SEEK_FLAG_ACCURATE or GST_SEEK_FLAG_FLUSH don't seem like trick modes You probably also want to ensure that unsupported flags return in an error here.
Review of attachment 338324 [details] [review]: Scale should probably be applied_rate. That is: it needs special support by the media, and generally in our seek events we don't distinguish between the two. Which in conclusion means: if the client asks for scale=2 and rate=12, we might as well provide applied_rate=1/24 and rate=1.0. Which should be considered in the response (i.e. we should give what is actually happening!), and can be parsed from the segment event. For example, scaletempo converts rate=X to applied_rate=1/X. It is the decision of the elements inside the pipeline, how to get the effective rate from the seek event. ::: gst/rtsp-server/rtsp-client.c @@ +1197,3 @@ + } + + /* parse the scale and speed headers, if present, and remember wheteher they Typo: whether @@ +1199,3 @@ + /* parse the scale and speed headers, if present, and remember wheteher they + * were present in the request. by only adding scale and/or speed headers to + * our respons if the request contained scale and/or speed headers we lower respons*e* @@ +1226,3 @@ + */ + GST_FIXME ("how to handle the Scale-header? For now just use 1.0"); + scale = 1.0; Shouldn't we cause rate=scale, scale=1.0 here then? That's giving the same media timeline to the client at least @@ +1255,3 @@ + *resp_scale_hdr = g_strdup_printf ("%1.1f", scale); + if (add_speed_hdr) + *resp_speed_hdr = g_strdup_printf ("%1.1f", speed); Some more digits would be nice, maybe 1.3f?
Review of attachment 339331 [details] [review]: ::: gst/rtsp-server/rtsp-client.c @@ +1202,3 @@ + + /* as for the speed header, just map it to rate */ + *rate = *speed; RTP timestamps are running_time, yes. Meaning, e.g.: if rate=2, applied_rate=1, then timestamps are as before but you get the data twice as fast as (original) real-time. If rate=1, applied_rate=2, then timestamps are all divided by 2 but you get them in real-time. That's exactly how the distinction between Speed and Scale could be achieved here. Just that it's not the decision of the client in the end, but of the server. Unless we extend the seek events by an applied_rate field. ::: gst/rtsp-server/rtsp-client.h @@ +89,3 @@ * be sent for a tunneled connection. The response can be modified. Since 1.4 + * @adjust_play_mode: called when a PLAY request is received to allow + * implementations to modify scale, speed, or seek flags for the seek. Since 1.12 Document which of the arguments are in, which are out, and which are both ("ref" in C# speak)
Review of attachment 339331 [details] [review]: ::: gst/rtsp-server/rtsp-client.c @@ +1184,3 @@ + GstSeekFlags * flags) +{ + /* FIXME: How to handle the Scale header? That, and going with KEYUNIT trick mode depending on the situation definitely makes sense.
Review of attachment 341496 [details] [review]: Looks good, but please provide a clearer commit message. Also it doesn't confuse clients if we now send things gop-by-gop backwards, and each gop forwards?
Created attachment 341572 [details] [review] Reverse playback support I hope it is more clear now...
(In reply to Sebastian Dröge (slomo) from comment #28) > Review of attachment 341496 [details] [review] [review]: > > Looks good, but please provide a clearer commit message. > > Also it doesn't confuse clients if we now send things gop-by-gop backwards, > and each gop forwards? The only RTSP client I found to be able to initiate reverse playback is VLC. As I said it does not really like this kind of stream: (In reply to Nikita Bobkov from comment #20) > Created attachment 341487 [details] [review] [review] > Reverse playback support > > With this patch VLC can even play formats without predicted frames (like > MJPEG) in reverse from our server. For other formats (like H264) it shows > only key frames. RTSP server sends frames of GOP in forward order which > seems to be conforming to ONVIF streaming specification. Does anyone know > RTSP/ONVIF client which can play such streams? But I am not sure that VLC is good reference here considering how it splits PLAY request into two parts: one with Range header and other with Scale.
Created attachment 373993 [details] [review] Allow specifying rate when seeking
Created attachment 373994 [details] [review] Allow sub classes to adjust the seek
Created attachment 373995 [details] [review] Add support for Scale and Speed headers
I have now redone the patches according to comments above. I have also restructured the patches slightly. The first adds the possibility to specify a rate when seeking the media, the second patch makes it possible for sub-classes to tweak the seek, and the third adds support for the Scale and Speed headers.
Review of attachment 373995 [details] [review]: As far as I understand, these patches will not lead to a compliant behaviour with respect to the Scale header, and I don't think the seeking mechanism in GStreamer will let us express a combination of ABS(Scale) != 1.0 and Speed != 1.0 appropriately. ABS(Scale) != 1.0 is expected to lead to the same data rate as ABS(scale) == 1.0 , which roughly translates to seeking with the TRICK_MODE flag and rate == Scale . This means we cannot simply use a rate == Scale * Speed , and have to either extend the seeking mechanism, or more pragmatically enforce the constraint that when Speed != 1.0 , the only valid values for Scale are -1.0 or 1.0 .
(In reply to Mathieu Duponchelle from comment #35) > Review of attachment 373995 [details] [review] [review]: > > As far as I understand, these patches will not lead to a compliant behaviour > with respect to the Scale header, Well, I'm not sure what a compliant behaviour actually is. The RFC says that the implementation of Scale is implementation dependent, and that the response must contain the actual scale chosen. Similarly implementation of Speed is "contingent on the server's ability and desire to serve the media stream at the given speed". But I agree that it does not behave as the RFC intended. > and I don't think the seeking mechanism in > GStreamer will let us express a combination of ABS(Scale) != 1.0 and Speed > != 1.0 appropriately. ABS(Scale) != 1.0 is expected to lead to the same data > rate as ABS(scale) == 1.0 , which roughly translates to seeking with the > TRICK_MODE flag and rate == Scale . You are absolutely right, GStreamer's seeking mechanism does not support the intended behaviour of the Scale header. > This means we cannot simply use a rate == Scale * Speed , and have to either > extend the seeking mechanism, or more pragmatically enforce the constraint > that when Speed != 1.0 , the only valid values for Scale are -1.0 or 1.0 . A (long) while ago I discussed this on the irc with Sebastian Dröge. His point of view was that we let the media decide. This is the approach that we agreed on. I am of course willing to change it, but I'd like to hear Sebastian's opinion as well.
-- GitLab Migration Automatic Message -- This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity. You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/gstreamer/gst-rtsp-server/issues/14.