Discussion:
[Bug 771525] New: gst-rtsp-server: poor performance while streaming RTP over RTSP
"GStreamer" (GNOME Bugzilla)
2016-09-16 10:21:05 UTC
Permalink
https://bugzilla.gnome.org/show_bug.cgi?id=771525

Bug ID: 771525
Summary: gst-rtsp-server: poor performance while streaming RTP
over RTSP
Classification: Platform
Product: GStreamer
Version: git master
OS: Linux
Status: NEW
Severity: normal
Priority: Normal
Component: gst-rtsp-server
Assignee: gstreamer-***@lists.freedesktop.org
Reporter: ***@axis.com
QA Contact: gstreamer-***@lists.freedesktop.org
GNOME version: ---

Currently, there is no support for buffer lists in gst-rtsp-server in the tcp
case.
The overhead of pushing single buffers individually through the pipeline gets
very high. I'll provide a patch suggesting a possible solution to this problem.

I've already created another bug addressing missing buffer list functionality
in appsink:
https://bugzilla.gnome.org/show_bug.cgi?id=752363.
--
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.
"GStreamer" (GNOME Bugzilla)
2016-09-16 13:16:34 UTC
Permalink
https://bugzilla.gnome.org/show_bug.cgi?id=771525

--- Comment #1 from Patricia Muscalu <***@axis.com> ---
Created attachment 335699
--> https://bugzilla.gnome.org/attachment.cgi?id=335699&action=edit
Use buffer list functionality
--
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.
"GStreamer" (GNOME Bugzilla)
2016-09-16 13:18:33 UTC
Permalink
https://bugzilla.gnome.org/show_bug.cgi?id=771525

--- Comment #2 from Patricia Muscalu <***@axis.com> ---
In the proposed solution the buffer list support is used in order to improve
performance in the RTP/RTSP case.
--
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.
"GStreamer" (GNOME Bugzilla)
2016-09-16 13:27:06 UTC
Permalink
https://bugzilla.gnome.org/show_bug.cgi?id=771525

--- Comment #3 from Patricia Muscalu <***@axis.com> ---
When sending single buffers in do_send_data function, the whole buffer is
mapped which is not optimal in case when buffers consist of several memory
blocks.
This part of the code is not changed in the proposed patch.
It requires some changes to GstRTSPMessage to support iovec:s ...
--
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.
"GStreamer" (GNOME Bugzilla)
2016-12-12 17:41:38 UTC
Permalink
https://bugzilla.gnome.org/show_bug.cgi?id=771525

Tim-Philipp Müller <***@zen.co.uk> changed:

What |Removed |Added
----------------------------------------------------------------------------
Attachment #335699|none |needs-work
status| |

--- Comment #4 from Tim-Philipp Müller <***@zen.co.uk> ---
Comment on attachment 335699
--> https://bugzilla.gnome.org/attachment.cgi?id=335699
Use buffer list functionality

Hi,

Thanks for the patch!
diff --git a/gst/rtsp-server/rtsp-stream-transport.h b/gst/rtsp-server/rtsp-stream-transport.h
index 8e2d366..2a83de5 100644
--- a/gst/rtsp-server/rtsp-stream-transport.h
+++ b/gst/rtsp-server/rtsp-stream-transport.h
@@ -53,7 +53,7 @@ typedef struct _GstRTSPStreamTransportPrivate GstRTSPStreamTransportPrivate;
*
* Returns: %TRUE on success
*/
-typedef gboolean (*GstRTSPSendFunc) (GstBuffer *buffer, guint8 channel, gpointer user_data);
+typedef gboolean (*GstRTSPSendFunc) (GstMiniObject *obj, guint8 channel, gpointer user_data);
I am not sure if this is something we can do. Not because of the type change in
the header file, but is it possible that someone subclassed this externally and
overrode this vfunc? And now they might suddenly get passed a buffer list
instead of a buffer. Also, it makes things difficult for bindings, which don't
know that GstBuffer and GstBufferList are mini objects. Don't know if that
would ever make sense in practice, but it is a public header if I'm not
mistaken.

I think the best would be to add a new SendListFunc + vfunc, and make
gst_rtsp_stream_transport_send_rtp_list() iterate using ->send() if the
->send_list() vfunc is NULL.

Does that make sense?
--
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.
"GStreamer" (GNOME Bugzilla)
2016-12-15 10:54:39 UTC
Permalink
https://bugzilla.gnome.org/show_bug.cgi?id=771525

--- Comment #5 from Patricia Muscalu <***@axis.com> ---
Hi,

Thank you very much for your response! Yes, it does make sense. We are working
on a new patch.
--
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.
"GStreamer" (GNOME Bugzilla)
2017-01-13 07:56:48 UTC
Permalink
https://bugzilla.gnome.org/show_bug.cgi?id=771525

--- Comment #6 from Patricia Muscalu <***@axis.com> ---
Created attachment 343409
--> https://bugzilla.gnome.org/attachment.cgi?id=343409&action=edit
Add support for buffer lists
--
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.
"GStreamer" (GNOME Bugzilla)
2017-01-13 07:57:36 UTC
Permalink
https://bugzilla.gnome.org/show_bug.cgi?id=771525

--- Comment #7 from Patricia Muscalu <***@axis.com> ---
Please have a look at the suggested patch. Thank you!
--
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.
"GStreamer" (GNOME Bugzilla)
2017-01-18 11:17:50 UTC
Permalink
https://bugzilla.gnome.org/show_bug.cgi?id=771525

Sebastian Dröge (slomo) <***@coaxion.net> changed:

What |Removed |Added
----------------------------------------------------------------------------
CC| |***@coaxion.net
Attachment #335699|0 |1
is obsolete| |
--
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.
"GStreamer" (GNOME Bugzilla)
2017-07-20 12:58:53 UTC
Permalink
https://bugzilla.gnome.org/show_bug.cgi?id=771525

anila <***@axis.com> changed:

What |Removed |Added
----------------------------------------------------------------------------
CC| |***@axis.com

--- Comment #8 from anila <***@axis.com> ---
Created attachment 356040
--> https://bugzilla.gnome.org/attachment.cgi?id=356040&action=edit
add support for bufferlist while streaming RTSP/TCP without data copying

Modified current patch by Patricia to avoid data copying.
g_socket_send_message is used so that we send vectors to the appsink without
data copying.
--
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.
"GStreamer" (GNOME Bugzilla)
2017-07-20 12:59:59 UTC
Permalink
https://bugzilla.gnome.org/show_bug.cgi?id=771525

--- Comment #9 from anila <***@axis.com> ---
Please have a look at the patch. Thank you!
--
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.
"GStreamer" (GNOME Bugzilla)
2017-07-25 11:52:47 UTC
Permalink
https://bugzilla.gnome.org/show_bug.cgi?id=771525

Olivier Crête <***@ocrete.ca> changed:

What |Removed |Added
----------------------------------------------------------------------------
CC| |***@ocrete.ca

--- Comment #10 from Olivier Crête <***@ocrete.ca> ---
Can you squash both patches into one? The second one seems to largely remove
things introduced in the first.
--
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.
"GStreamer" (GNOME Bugzilla)
2017-08-01 12:33:28 UTC
Permalink
https://bugzilla.gnome.org/show_bug.cgi?id=771525

Tim-Philipp Müller <***@zen.co.uk> changed:

What |Removed |Added
----------------------------------------------------------------------------
Depends on| |785684
--
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.
"GStreamer" (GNOME Bugzilla)
2017-08-01 13:03:20 UTC
Permalink
https://bugzilla.gnome.org/show_bug.cgi?id=771525

--- Comment #14 from anila <***@axis.com> ---
(In reply to Olivier Crête from comment #12)
::: gst/rtsp-server/rtsp-client.c
@@ +44,3 @@
#include <stdio.h>
#include <string.h>
+#include <sys/socket.h>
Why do you include this? arent all socket operations abstracted through GIO ?
Thats true, I have removed.
@@ +1037,3 @@
+ data_header += 4;
+ }
+ GST_DEBUG (" mem:%d, pos: %d", mem, pos);
This should be a GST_LOG, it's on every buffer.
@@ +4085,3 @@
+ num_written =
+ g_socket_send_message (socket, NULL, &vecs[i], send_len, NULL, 0,
+ MSG_DONTWAIT, NULL, &err);
You shouldn't pull out the socket from here.. Instead you should add the
proper API to gst_rtsp_connection_..()
Added API in gst-plugin-base.
And you sholdn't use MSG_DONTWAIT, the parameter here is a GSocketMsgFlags
Even though the parameter here is GSocketMsgFlags we use MSG_DONTWAIT in
g_socket_send_message for making it non blocking.
g_socket_send_message in turn uses sendmsg () which support the use of this
flag.
@@ +4087,3 @@
+ MSG_DONTWAIT, NULL, &err);
+ if (err != NULL)
+ GST_ERROR ("error: %s", err->message);
You end up re-printing the error down there, so this one can be downgraded
or removed.
Removed.
@@ +4088,3 @@
+ if (err != NULL)
+ GST_ERROR ("error: %s", err->message);
+ } while (num_written == -1 && (errno == EINTR || errno == EAGAIN));
Instead of looking at errno, you want to look at the err with
g_error_matches(). And if you want to g_clear_error(&err) if you decide to
ignore the error.
I have used g_error_matches() in the gstrtspconnection api for EAGAIN,
After analyzing the code for g_socket_send_message, we found that the error
EINTR was never returned. So I have removed the check for it.
@@ +4091,3 @@
+
+ if (num_written == -1) {
+ GST_ERROR ("failed to send message: %s", err->message);
While we're at it, maybe you should pass the client object here so you can
do GST_ERROR_OBJECT()..
@@ +4133,3 @@
+ total_mems = gst_buffer_n_memory (buffer);
+ map_infos = g_new (GstMapInfo, total_mems);
+ vecs = g_new (GOutputVector, total_mems + num_buffers);
Why not g_newa() here?
@@ +4155,3 @@
+ map_infos = g_new (GstMapInfo, total_mems);
+ vecs = g_new (GOutputVector, total_mems + num_buffers);
+ data_header = g_malloc (4 * num_buffers);
Any reason not to use g_newa() here too ?
as you know the g_newa allocate to the stackframe and it was used for
allocating where small amount of memory was needed.. thats the reason we didnt
use g_newa for map_infos and vecs. I can change the g_newa to g_new if you
want to make the code look consistent
@@ +4183,3 @@
+ GST_ERROR_OBJECT (client, "waiting for backlog");
+ ret = gst_rtsp_watch_wait_backlog (priv->watch, &time);
+ GST_ERROR_OBJECT (client, "Resend due to backlog full");
Those are not really errors, should be at DEBUG level.
Changed to debug level.
--
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.
"GStreamer" (GNOME Bugzilla)
2017-08-01 12:55:30 UTC
Permalink
https://bugzilla.gnome.org/show_bug.cgi?id=771525

--- Comment #13 from anila <***@axis.com> ---
Created attachment 356724
--> https://bugzilla.gnome.org/attachment.cgi?id=356724&action=edit
add support for bufferlist while streaming RTSP/TCP without data copying
--
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.
"GStreamer" (GNOME Bugzilla)
2017-09-29 08:46:44 UTC
Permalink
https://bugzilla.gnome.org/show_bug.cgi?id=771525

anila <***@axis.com> changed:

What |Removed |Added
----------------------------------------------------------------------------
Attachment #356397|needs-work |none
status| |
Attachment #356397|0 |1
is obsolete| |
Attachment #356724|0 |1
is obsolete| |

--- Comment #15 from anila <***@axis.com> ---
Created attachment 360649
--> https://bugzilla.gnome.org/attachment.cgi?id=360649&action=edit
send messages to rtspconnection without blocking

stashed both the patches to one
--
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.
"GStreamer" (GNOME Bugzilla)
2018-06-27 10:24:00 UTC
Permalink
https://bugzilla.gnome.org/show_bug.cgi?id=771525

Sebastian Dröge (slomo) <***@coaxion.net> changed:

What |Removed |Added
----------------------------------------------------------------------------
Attachment #360649|none |needs-work
status| |

--- Comment #16 from Sebastian Dröge (slomo) <***@coaxion.net> ---
Comment on attachment 360649
--> https://bugzilla.gnome.org/attachment.cgi?id=360649
send messages to rtspconnection without blocking

This breaks API/ABI with the send functions as now sends both lists and buffers
to the send_rtp functions. I have a patch that fixes this, and that as a first
step does not depend on any GstRTSPMessage/Watch API additions.

Those can come as a next step. I'll attach the new patch in a bit for
discussion here
--
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.
"GStreamer" (GNOME Bugzilla)
2018-06-27 10:40:56 UTC
Permalink
https://bugzilla.gnome.org/show_bug.cgi?id=771525

--- Comment #17 from Sebastian Dröge (slomo) <***@coaxion.net> ---
There's also the problem in rtsp-client.c:do_send_data() that it assumes that
the buffer can be unmapped after gst_rtsp_watch_send_message(). That's not the
case, it can queue up the message and send it at a later time from another
thread, and there's nothing at all that ensures that the memory is actually
still available or valid at all (it could be unmapped, reused, ...).

I'm not sure why this didn't cause huge problems in the past yet.
--
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.
"GStreamer" (GNOME Bugzilla)
2018-06-27 13:50:24 UTC
Permalink
https://bugzilla.gnome.org/show_bug.cgi?id=771525

Sebastian Dröge (slomo) <***@coaxion.net> changed:

What |Removed |Added
----------------------------------------------------------------------------
Depends on| |796692
--
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.
"GStreamer" (GNOME Bugzilla)
2018-06-27 13:50:46 UTC
Permalink
https://bugzilla.gnome.org/show_bug.cgi?id=771525

--- Comment #18 from Sebastian Dröge (slomo) <***@coaxion.net> ---
Created attachment 372851
--> https://bugzilla.gnome.org/attachment.cgi?id=372851&action=edit
rtsp-server: Add support for buffer lists

This adds new functions for passing buffer lists through the different
layers without breaking API/ABI, and enables the appsink to actually
provide buffer lists.

This should already reduce CPU usage and potentially context switches a
bit by passing a whole buffer list from the appsink instead of
individual buffers. As a next step it would be necessary to
a) Add support for a vector of data for the GstRTSPMessage body
b) Add support for sending multiple messages at once to the
GstRTSPWatch and let it be handled internally
c) Adding API to GOutputStream that works like writev()
--
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.
"GStreamer" (GNOME Bugzilla)
2018-06-27 14:11:12 UTC
Permalink
https://bugzilla.gnome.org/show_bug.cgi?id=771525

Sebastian Dröge (slomo) <***@coaxion.net> changed:

What |Removed |Added
----------------------------------------------------------------------------
Attachment #360649|0 |1
is obsolete| |

--- Comment #19 from Sebastian Dröge (slomo) <***@coaxion.net> ---
Comment on attachment 360649
--> https://bugzilla.gnome.org/attachment.cgi?id=360649
send messages to rtspconnection without blocking

The creation of the actual message, etc should also be done inside
GstRTSPConnection, and the GstRTSPMessage should just get the multiple chunks
of the body attached to it somehow. I have a plan
--
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.
"GStreamer" (GNOME Bugzilla)
2018-06-27 14:31:31 UTC
Permalink
https://bugzilla.gnome.org/show_bug.cgi?id=771525

Sebastian Dröge (slomo) <***@coaxion.net> changed:

What |Removed |Added
----------------------------------------------------------------------------
Summary|gst-rtsp-server: poor |gst-rtsp-server: Poor
|performance while streaming |performance with
|RTP over RTSP |interleaved RTSP due to
| |missing buffer list support
| |and merging of all memories
| |in buffers
--
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.
"GStreamer" (GNOME Bugzilla)
2018-07-09 07:46:01 UTC
Permalink
https://bugzilla.gnome.org/show_bug.cgi?id=771525
Bug 771525 depends on bug 796692, which changed state.

Bug 796692 Summary: sample/bufferlist/buffer: Non-writable container miniobjects allow writable access to their contents, causing memory problems
https://bugzilla.gnome.org/show_bug.cgi?id=796692

What |Removed |Added
----------------------------------------------------------------------------
Status|NEW |RESOLVED
Resolution|--- |FIXED
--
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.
"GStreamer" (GNOME Bugzilla)
2018-08-17 06:31:49 UTC
Permalink
https://bugzilla.gnome.org/show_bug.cgi?id=771525

Sebastian Dröge (slomo) <***@coaxion.net> changed:

What |Removed |Added
----------------------------------------------------------------------------
Attachment #372851|0 |1
is obsolete| |

--- Comment #20 from Sebastian Dröge (slomo) <***@coaxion.net> ---
Created attachment 373366
--> https://bugzilla.gnome.org/attachment.cgi?id=373366&action=edit
rtsp-server: Add support for buffer lists

This adds new functions for passing buffer lists through the different
layers without breaking API/ABI, and enables the appsink to actually
provide buffer lists.

This should already reduce CPU usage and potentially context switches a
bit by passing a whole buffer list from the appsink instead of
individual buffers. As a next step it would be necessary to
a) Add support for a vector of data for the GstRTSPMessage body
b) Add support for sending multiple messages at once to the
GstRTSPWatch and let it be handled internally
c) Adding API to GOutputStream that works like writev()
--
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.
"GStreamer" (GNOME Bugzilla)
2018-08-17 08:06:53 UTC
Permalink
https://bugzilla.gnome.org/show_bug.cgi?id=771525

Sebastian Dröge (slomo) <***@coaxion.net> changed:

What |Removed |Added
----------------------------------------------------------------------------
Attachment #373366|0 |1
is obsolete| |

--- Comment #21 from Sebastian Dröge (slomo) <***@coaxion.net> ---
Created attachment 373368
--> https://bugzilla.gnome.org/attachment.cgi?id=373368&action=edit
rtsp-server: Add support for buffer lists

This adds new functions for passing buffer lists through the different
layers without breaking API/ABI, and enables the appsink to actually
provide buffer lists.

This should already reduce CPU usage and potentially context switches a
bit by passing a whole buffer list from the appsink instead of
individual buffers. As a next step it would be necessary to
a) Add support for a vector of data for the GstRTSPMessage body
b) Add support for sending multiple messages at once to the
GstRTSPWatch and let it be handled internally
c) Adding API to GOutputStream that works like writev()
--
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.
"GStreamer" (GNOME Bugzilla)
2018-08-17 11:23:54 UTC
Permalink
https://bugzilla.gnome.org/show_bug.cgi?id=771525

Sebastian Dröge (slomo) <***@coaxion.net> changed:

What |Removed |Added
----------------------------------------------------------------------------
Attachment #373368|0 |1
is obsolete| |

--- Comment #22 from Sebastian Dröge (slomo) <***@coaxion.net> ---
Created attachment 373374
--> https://bugzilla.gnome.org/attachment.cgi?id=373374&action=edit
rtsp-server: Add support for buffer lists

This adds new functions for passing buffer lists through the different
layers without breaking API/ABI, and enables the appsink to actually
provide buffer lists.

This should already reduce CPU usage and potentially context switches a
bit by passing a whole buffer list from the appsink instead of
individual buffers. As a next step it would be necessary to
a) Add support for a vector of data for the GstRTSPMessage body
b) Add support for sending multiple messages at once to the
GstRTSPWatch and let it be handled internally
c) Adding API to GOutputStream that works like writev()
--
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.
"GStreamer" (GNOME Bugzilla)
2018-08-17 13:10:58 UTC
Permalink
https://bugzilla.gnome.org/show_bug.cgi?id=771525

Nicolas Dufresne (ndufresne) <***@ndufresne.ca> changed:

What |Removed |Added
----------------------------------------------------------------------------
Attachment #373368|none |needs-work
status| |

--- Comment #23 from Nicolas Dufresne (ndufresne) <***@ndufresne.ca> ---
Review of attachment 373368:
--> (https://bugzilla.gnome.org/review?bug=771525&attachment=373368)

::: gst/rtsp-server/rtsp-stream-transport.c
@@ +219,3 @@
+ *
+ * Install callbacks that will be called when data for a stream should be sent
+ * to a client. This is usually used when sending RTP/RTCP over TCP.

This need a Since mark, same for other new symbols.

::: gst/rtsp-server/rtsp-stream.c
@@ +2424,3 @@
+ n_messages += 1;
+ if (buffer_list)
+ n_messages += gst_buffer_list_length (buffer_list);

Can you really have both buffer and buffer_list set ? I think using else if ()
if not would be better. If it's the case, how do we know we should send the
buffer first ?
--
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.
"GStreamer" (GNOME Bugzilla)
2018-08-17 13:11:38 UTC
Permalink
https://bugzilla.gnome.org/show_bug.cgi?id=771525

Nicolas Dufresne (ndufresne) <***@ndufresne.ca> changed:

What |Removed |Added
----------------------------------------------------------------------------
Attachment #373374|none |needs-work
status| |

--- Comment #24 from Nicolas Dufresne (ndufresne) <***@ndufresne.ca> ---
Review of attachment 373374:
--> (https://bugzilla.gnome.org/review?bug=771525&attachment=373374)

I've commented the previous patch, but comment still applies.
--
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.
"GStreamer" (GNOME Bugzilla)
2018-08-24 06:51:35 UTC
Permalink
https://bugzilla.gnome.org/show_bug.cgi?id=771525

David Svensson Fors <***@axis.com> changed:

What |Removed |Added
----------------------------------------------------------------------------
CC| |***@axis.com
--
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.
"GStreamer" (GNOME Bugzilla)
2018-08-27 06:07:21 UTC
Permalink
https://bugzilla.gnome.org/show_bug.cgi?id=771525

--- Comment #25 from Sebastian Dröge (slomo) <***@coaxion.net> ---
More work is needed on these patches to be useful, but thanks for the review :)

What's missing is specifically:

1) Using writev() in GstRTSPConnection
2) Passing through whole bufferlists from gst-rtsp-server
3) Handling a whole bufferlist as one unit (instead of each buffer), as we now
only allow one data message in-flight at a time per interleave id
--
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.
"GStreamer" (GNOME Bugzilla)
2018-09-18 11:21:30 UTC
Permalink
https://bugzilla.gnome.org/show_bug.cgi?id=771525

Sebastian Dröge (slomo) <***@coaxion.net> changed:

What |Removed |Added
----------------------------------------------------------------------------
Attachment #373374|needs-work |none
status| |
Attachment #373374|0 |1
is obsolete| |

--- Comment #26 from Sebastian Dröge (slomo) <***@coaxion.net> ---
Created attachment 373684
--> https://bugzilla.gnome.org/attachment.cgi?id=373684&action=edit
rtsp-server: Add support for buffer lists

This adds new functions for passing buffer lists through the different
layers without breaking API/ABI, and enables the appsink to actually
provide buffer lists.

This should already reduce CPU usage and potentially context switches a
bit by passing a whole buffer list from the appsink instead of
individual buffers. As a next step it would be necessary to
a) Add support for a vector of data for the GstRTSPMessage body
b) Add support for sending multiple messages at once to the
GstRTSPWatch and let it be handled internally
c) Adding API to GOutputStream that works like writev()
--
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.
"GStreamer" (GNOME Bugzilla)
2018-09-18 11:21:44 UTC
Permalink
https://bugzilla.gnome.org/show_bug.cgi?id=771525

--- Comment #27 from Sebastian Dröge (slomo) <***@coaxion.net> ---
Created attachment 373685
--> https://bugzilla.gnome.org/attachment.cgi?id=373685&action=edit
rtsp-client: Add support for sending buffer lists directly
--
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.
"GStreamer" (GNOME Bugzilla)
2018-10-11 14:09:44 UTC
Permalink
https://bugzilla.gnome.org/show_bug.cgi?id=771525

Tim-Philipp Müller <***@zen.co.uk> changed:

What |Removed |Added
----------------------------------------------------------------------------
CC| |***@zen.co.uk

--- Comment #28 from Tim-Philipp Müller <***@zen.co.uk> ---
The connection part is a bit hard to review really, proof will be in the
pudding :)

Maybe rename message_to_string() to message_serialize() so it's aligned with
the output struct?
--
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.
"GStreamer" (GNOME Bugzilla)
2018-10-11 15:17:23 UTC
Permalink
https://bugzilla.gnome.org/show_bug.cgi?id=771525

Tim-Philipp Müller <***@zen.co.uk> changed:

What |Removed |Added
----------------------------------------------------------------------------
Attachment #373684|none |reviewed
status| |

--- Comment #29 from Tim-Philipp Müller <***@zen.co.uk> ---
Comment on attachment 373684
--> https://bugzilla.gnome.org/attachment.cgi?id=373684
rtsp-server: Add support for buffer lists

Looks fine to me by and large.
--- a/gst/rtsp-server/rtsp-stream.c
+++ b/gst/rtsp-server/rtsp-stream.c
@@ -2394,6 +2394,8 @@ send_tcp_message (GstRTSPStream * stream, gint idx)
GList *walk;
GstSample *sample;
GstBuffer *buffer;
+ GstBufferList *buffer_list;
+ guint n_messages = 0;
gboolean is_rtp;
if (priv->n_outstanding > 0 || !priv->have_buffer[idx]) {
@@ -2414,6 +2416,14 @@ send_tcp_message (GstRTSPStream * stream, gint idx)
}
buffer = gst_sample_get_buffer (sample);
+ buffer_list = gst_sample_get_buffer_list (sample);
+
+ /* We will get one message-sent notification per message,
+ * i.e. per buffer that is actually sent out */
+ if (buffer)
+ n_messages += 1;
+ if (buffer_list)
+ n_messages += gst_buffer_list_length (buffer_list);
Why are we doing "+=" here and not "=" and should it be "else if" here for
clarity or can we get both a buffer and a buffer list? (don't think so?)

"else if" also a few times in the code below.
--- a/tests/check/gst/rtspserver.c
+++ b/tests/check/gst/rtspserver.c
@@ -2117,7 +2117,7 @@ GST_START_TEST (test_record_tcp)
mfactory =
start_record_server
- ("( rtppcmadepay name=depay0 ! appsink name=sink async=false )");
+ ("( rtppcmadepay name=depay0 ! appsink name=sink buffer-list=true async=false )");
g_signal_connect (mfactory, "media-constructed",
G_CALLBACK (media_constructed_cb), &server_sink);
Does this (buffer-list=true) here actually do anything? Is it needed?
--
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.
"GStreamer" (GNOME Bugzilla)
2018-10-11 15:27:12 UTC
Permalink
https://bugzilla.gnome.org/show_bug.cgi?id=771525

Tim-Philipp Müller <***@zen.co.uk> changed:

What |Removed |Added
----------------------------------------------------------------------------
Attachment #373685|none |reviewed
status| |

--- Comment #30 from Tim-Philipp Müller <***@zen.co.uk> ---
Comment on attachment 373685
--> https://bugzilla.gnome.org/attachment.cgi?id=373685
rtsp-client: Add support for sending buffer lists directly

Looks reasonable to the extent that it can be reviewed.
+static gboolean
+do_send_messages (GstRTSPClient * client, GstRTSPMessage * messages,
+ guint n_messages, gboolean close, gpointer user_data)
+{
+ ...
+
+ for (i = 0; i < n_messages; i++) {
+ if (gst_rtsp_message_get_type (&messages[i]) == GST_RTSP_MESSAGE_DATA) {
+ guint8 channel = 0;
+ GstRTSPResult r;
+
+ /* FIXME: We assume that all data messages in the list are for the
+ * same channel */
+ r = gst_rtsp_message_parse_data (&messages[i], &channel);
Is that something that needs fixing before merging? It seems like a reasonable
assumption to me in our context? Maybe it needs documenting somewhere?
--
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.
"GStreamer" (GNOME Bugzilla)
2018-10-11 16:40:40 UTC
Permalink
https://bugzilla.gnome.org/show_bug.cgi?id=771525

Mathieu Duponchelle <***@gmail.com> changed:

What |Removed |Added
----------------------------------------------------------------------------
Attachment #373684|reviewed |needs-work
status| |
Attachment #373684|reviewed |needs-work
status| |
Attachment #373684|reviewed reviewed |needs-work needs-work
status| |

--- Comment #31 from Mathieu Duponchelle <***@gmail.com> ---
Review of attachment 373684:
--> (https://bugzilla.gnome.org/review?bug=771525&attachment=373684)

This will require updating sections.txt, lgtm apart from a few comments

::: gst/rtsp-server/rtsp-client.c
@@ +1187,3 @@

+static gboolean
+do_send_data_list (GstBufferList * buffer_list, guint8 channel,

The implementation seems similar to what rtsp-stream-transport will do if not
callback was provided, is this mostly a placeholder for future extension?

::: gst/rtsp-server/rtsp-media.c
@@ +2187,3 @@
g_object_set (appsrc, "block", TRUE, "format", GST_FORMAT_TIME, "is-live",
+ TRUE, "emit-signals", FALSE, NULL);
+ g_object_set (appsink, "sync", FALSE, "async", FALSE, "emit-signals",

emit-signals is false by default for appsink

::: gst/rtsp-server/rtsp-stream-transport.c
@@ +219,3 @@
+ *
+ * Install callbacks that will be called when data for a stream should be sent
+ * to a client. This is usually used when sending RTP/RTCP over TCP.

Since:

@@ -531,0 +566,7 @@
+/**
+ * gst_rtsp_stream_transport_send_rtp_list:
+ * @trans: a #GstRTSPStreamTransport
... 4 more ...

Since:

@@ +614,3 @@
+ *
+ * Send @buffer_list to the installed RTCP callback for @trans.
+ *

Since:

::: gst/rtsp-server/rtsp-stream.c
@@ +2474,3 @@
+ send_ret = gst_rtsp_stream_transport_send_rtp (tr, buffer);
+ if (buffer_list)
+ send_ret = gst_rtsp_stream_transport_send_rtp_list (tr, buffer_list);

this will ignore any potential error if the sample contained both a buffer and
a buffer list, not sure if that is allowed to happen in practice

@@ +3332,3 @@
/* make appsink */
priv->appsink[i] = gst_element_factory_make ("appsink", NULL);
+ g_object_set (priv->appsink[i], "emit-signals", FALSE, "buffer-list",

emit-signals is FALSE by default in appsink

::: gst/rtsp-sink/gstrtspclientsink.c
@@ +3853,3 @@
+static gboolean
+do_send_data_list (GstBufferList * buffer_list, guint8 channel,
+ GstRTSPStreamContext * context)

Same comment as the do_send_list in rtsp-client

--- Comment #32 from Mathieu Duponchelle <***@gmail.com> ---
Review of attachment 373684:
--> (https://bugzilla.gnome.org/review?bug=771525&attachment=373684)

This will require updating sections.txt, lgtm apart from a few comments

::: gst/rtsp-server/rtsp-client.c
@@ +1187,3 @@

+static gboolean
+do_send_data_list (GstBufferList * buffer_list, guint8 channel,

The implementation seems similar to what rtsp-stream-transport will do if not
callback was provided, is this mostly a placeholder for future extension?

::: gst/rtsp-server/rtsp-media.c
@@ +2187,3 @@
g_object_set (appsrc, "block", TRUE, "format", GST_FORMAT_TIME, "is-live",
+ TRUE, "emit-signals", FALSE, NULL);
+ g_object_set (appsink, "sync", FALSE, "async", FALSE, "emit-signals",

emit-signals is false by default for appsink

::: gst/rtsp-server/rtsp-stream-transport.c
@@ +219,3 @@
+ *
+ * Install callbacks that will be called when data for a stream should be sent
+ * to a client. This is usually used when sending RTP/RTCP over TCP.

Since:

@@ -531,0 +566,7 @@
+/**
+ * gst_rtsp_stream_transport_send_rtp_list:
+ * @trans: a #GstRTSPStreamTransport
... 4 more ...

Since:

@@ +614,3 @@
+ *
+ * Send @buffer_list to the installed RTCP callback for @trans.
+ *

Since:

::: gst/rtsp-server/rtsp-stream.c
@@ +2474,3 @@
+ send_ret = gst_rtsp_stream_transport_send_rtp (tr, buffer);
+ if (buffer_list)
+ send_ret = gst_rtsp_stream_transport_send_rtp_list (tr, buffer_list);

this will ignore any potential error if the sample contained both a buffer and
a buffer list, not sure if that is allowed to happen in practice

@@ +3332,3 @@
/* make appsink */
priv->appsink[i] = gst_element_factory_make ("appsink", NULL);
+ g_object_set (priv->appsink[i], "emit-signals", FALSE, "buffer-list",

emit-signals is FALSE by default in appsink

::: gst/rtsp-sink/gstrtspclientsink.c
@@ +3853,3 @@
+static gboolean
+do_send_data_list (GstBufferList * buffer_list, guint8 channel,
+ GstRTSPStreamContext * context)

Same comment as the do_send_list in rtsp-client

--- Comment #33 from Mathieu Duponchelle <***@gmail.com> ---
Review of attachment 373684:
--> (https://bugzilla.gnome.org/review?bug=771525&attachment=373684)

This will require updating sections.txt, lgtm apart from a few comments

::: gst/rtsp-server/rtsp-client.c
@@ +1187,3 @@

+static gboolean
+do_send_data_list (GstBufferList * buffer_list, guint8 channel,

The implementation seems similar to what rtsp-stream-transport will do if not
callback was provided, is this mostly a placeholder for future extension?

::: gst/rtsp-server/rtsp-media.c
@@ +2187,3 @@
g_object_set (appsrc, "block", TRUE, "format", GST_FORMAT_TIME, "is-live",
+ TRUE, "emit-signals", FALSE, NULL);
+ g_object_set (appsink, "sync", FALSE, "async", FALSE, "emit-signals",

emit-signals is false by default for appsink

::: gst/rtsp-server/rtsp-stream-transport.c
@@ +219,3 @@
+ *
+ * Install callbacks that will be called when data for a stream should be sent
+ * to a client. This is usually used when sending RTP/RTCP over TCP.

Since:

@@ -531,0 +566,7 @@
+/**
+ * gst_rtsp_stream_transport_send_rtp_list:
+ * @trans: a #GstRTSPStreamTransport
... 4 more ...

Since:

@@ +614,3 @@
+ *
+ * Send @buffer_list to the installed RTCP callback for @trans.
+ *

Since:

::: gst/rtsp-server/rtsp-stream.c
@@ +2474,3 @@
+ send_ret = gst_rtsp_stream_transport_send_rtp (tr, buffer);
+ if (buffer_list)
+ send_ret = gst_rtsp_stream_transport_send_rtp_list (tr, buffer_list);

this will ignore any potential error if the sample contained both a buffer and
a buffer list, not sure if that is allowed to happen in practice

@@ +3332,3 @@
/* make appsink */
priv->appsink[i] = gst_element_factory_make ("appsink", NULL);
+ g_object_set (priv->appsink[i], "emit-signals", FALSE, "buffer-list",

emit-signals is FALSE by default in appsink

::: gst/rtsp-sink/gstrtspclientsink.c
@@ +3853,3 @@
+static gboolean
+do_send_data_list (GstBufferList * buffer_list, guint8 channel,
+ GstRTSPStreamContext * context)

Same comment as the do_send_list in rtsp-client

--- Comment #34 from Mathieu Duponchelle <***@gmail.com> ---
Review of attachment 373684:
--> (https://bugzilla.gnome.org/review?bug=771525&attachment=373684)

This will require updating sections.txt, lgtm apart from a few comments

::: gst/rtsp-server/rtsp-client.c
@@ +1187,3 @@

+static gboolean
+do_send_data_list (GstBufferList * buffer_list, guint8 channel,

The implementation seems similar to what rtsp-stream-transport will do if not
callback was provided, is this mostly a placeholder for future extension?

::: gst/rtsp-server/rtsp-media.c
@@ +2187,3 @@
g_object_set (appsrc, "block", TRUE, "format", GST_FORMAT_TIME, "is-live",
+ TRUE, "emit-signals", FALSE, NULL);
+ g_object_set (appsink, "sync", FALSE, "async", FALSE, "emit-signals",

emit-signals is false by default for appsink

::: gst/rtsp-server/rtsp-stream-transport.c
@@ +219,3 @@
+ *
+ * Install callbacks that will be called when data for a stream should be sent
+ * to a client. This is usually used when sending RTP/RTCP over TCP.

Since:

@@ -531,0 +566,7 @@
+/**
+ * gst_rtsp_stream_transport_send_rtp_list:
+ * @trans: a #GstRTSPStreamTransport
... 4 more ...

Since:

@@ +614,3 @@
+ *
+ * Send @buffer_list to the installed RTCP callback for @trans.
+ *

Since:

::: gst/rtsp-server/rtsp-stream.c
@@ +2474,3 @@
+ send_ret = gst_rtsp_stream_transport_send_rtp (tr, buffer);
+ if (buffer_list)
+ send_ret = gst_rtsp_stream_transport_send_rtp_list (tr, buffer_list);

this will ignore any potential error if the sample contained both a buffer and
a buffer list, not sure if that is allowed to happen in practice

@@ +3332,3 @@
/* make appsink */
priv->appsink[i] = gst_element_factory_make ("appsink", NULL);
+ g_object_set (priv->appsink[i], "emit-signals", FALSE, "buffer-list",

emit-signals is FALSE by default in appsink

::: gst/rtsp-sink/gstrtspclientsink.c
@@ +3853,3 @@
+static gboolean
+do_send_data_list (GstBufferList * buffer_list, guint8 channel,
+ GstRTSPStreamContext * context)

Same comment as the do_send_list in rtsp-client

--- Comment #35 from Mathieu Duponchelle <***@gmail.com> ---
Review of attachment 373684:
--> (https://bugzilla.gnome.org/review?bug=771525&attachment=373684)

This will require updating sections.txt, lgtm apart from a few comments

::: gst/rtsp-server/rtsp-client.c
@@ +1187,3 @@

+static gboolean
+do_send_data_list (GstBufferList * buffer_list, guint8 channel,

The implementation seems similar to what rtsp-stream-transport will do if not
callback was provided, is this mostly a placeholder for future extension?

::: gst/rtsp-server/rtsp-media.c
@@ +2187,3 @@
g_object_set (appsrc, "block", TRUE, "format", GST_FORMAT_TIME, "is-live",
+ TRUE, "emit-signals", FALSE, NULL);
+ g_object_set (appsink, "sync", FALSE, "async", FALSE, "emit-signals",

emit-signals is false by default for appsink

::: gst/rtsp-server/rtsp-stream-transport.c
@@ +219,3 @@
+ *
+ * Install callbacks that will be called when data for a stream should be sent
+ * to a client. This is usually used when sending RTP/RTCP over TCP.

Since:

@@ -531,0 +566,7 @@
+/**
+ * gst_rtsp_stream_transport_send_rtp_list:
+ * @trans: a #GstRTSPStreamTransport
... 4 more ...

Since:

@@ +614,3 @@
+ *
+ * Send @buffer_list to the installed RTCP callback for @trans.
+ *

Since:

::: gst/rtsp-server/rtsp-stream.c
@@ +2474,3 @@
+ send_ret = gst_rtsp_stream_transport_send_rtp (tr, buffer);
+ if (buffer_list)
+ send_ret = gst_rtsp_stream_transport_send_rtp_list (tr, buffer_list);

this will ignore any potential error if the sample contained both a buffer and
a buffer list, not sure if that is allowed to happen in practice

@@ +3332,3 @@
/* make appsink */
priv->appsink[i] = gst_element_factory_make ("appsink", NULL);
+ g_object_set (priv->appsink[i], "emit-signals", FALSE, "buffer-list",

emit-signals is FALSE by default in appsink

::: gst/rtsp-sink/gstrtspclientsink.c
@@ +3853,3 @@
+static gboolean
+do_send_data_list (GstBufferList * buffer_list, guint8 channel,
+ GstRTSPStreamContext * context)

Same comment as the do_send_list in rtsp-client

--- Comment #36 from Mathieu Duponchelle <***@gmail.com> ---
Review of attachment 373684:
--> (https://bugzilla.gnome.org/review?bug=771525&attachment=373684)

This will require updating sections.txt, lgtm apart from a few comments

::: gst/rtsp-server/rtsp-client.c
@@ +1187,3 @@

+static gboolean
+do_send_data_list (GstBufferList * buffer_list, guint8 channel,

The implementation seems similar to what rtsp-stream-transport will do if not
callback was provided, is this mostly a placeholder for future extension?

::: gst/rtsp-server/rtsp-media.c
@@ +2187,3 @@
g_object_set (appsrc, "block", TRUE, "format", GST_FORMAT_TIME, "is-live",
+ TRUE, "emit-signals", FALSE, NULL);
+ g_object_set (appsink, "sync", FALSE, "async", FALSE, "emit-signals",

emit-signals is false by default for appsink

::: gst/rtsp-server/rtsp-stream-transport.c
@@ +219,3 @@
+ *
+ * Install callbacks that will be called when data for a stream should be sent
+ * to a client. This is usually used when sending RTP/RTCP over TCP.

Since:

@@ -531,0 +566,7 @@
+/**
+ * gst_rtsp_stream_transport_send_rtp_list:
+ * @trans: a #GstRTSPStreamTransport
... 4 more ...

Since:

@@ +614,3 @@
+ *
+ * Send @buffer_list to the installed RTCP callback for @trans.
+ *

Since:

::: gst/rtsp-server/rtsp-stream.c
@@ +2474,3 @@
+ send_ret = gst_rtsp_stream_transport_send_rtp (tr, buffer);
+ if (buffer_list)
+ send_ret = gst_rtsp_stream_transport_send_rtp_list (tr, buffer_list);

this will ignore any potential error if the sample contained both a buffer and
a buffer list, not sure if that is allowed to happen in practice

@@ +3332,3 @@
/* make appsink */
priv->appsink[i] = gst_element_factory_make ("appsink", NULL);
+ g_object_set (priv->appsink[i], "emit-signals", FALSE, "buffer-list",

emit-signals is FALSE by default in appsink

::: gst/rtsp-sink/gstrtspclientsink.c
@@ +3853,3 @@
+static gboolean
+do_send_data_list (GstBufferList * buffer_list, guint8 channel,
+ GstRTSPStreamContext * context)

Same comment as the do_send_list in rtsp-client

--- Comment #37 from Mathieu Duponchelle <***@gmail.com> ---
Review of attachment 373684:
--> (https://bugzilla.gnome.org/review?bug=771525&attachment=373684)

This will require updating sections.txt, lgtm apart from a few comments

::: gst/rtsp-server/rtsp-client.c
@@ +1187,3 @@

+static gboolean
+do_send_data_list (GstBufferList * buffer_list, guint8 channel,

The implementation seems similar to what rtsp-stream-transport will do if not
callback was provided, is this mostly a placeholder for future extension?

::: gst/rtsp-server/rtsp-media.c
@@ +2187,3 @@
g_object_set (appsrc, "block", TRUE, "format", GST_FORMAT_TIME, "is-live",
+ TRUE, "emit-signals", FALSE, NULL);
+ g_object_set (appsink, "sync", FALSE, "async", FALSE, "emit-signals",

emit-signals is false by default for appsink

::: gst/rtsp-server/rtsp-stream-transport.c
@@ +219,3 @@
+ *
+ * Install callbacks that will be called when data for a stream should be sent
+ * to a client. This is usually used when sending RTP/RTCP over TCP.

Since:

@@ -531,0 +566,7 @@
+/**
+ * gst_rtsp_stream_transport_send_rtp_list:
+ * @trans: a #GstRTSPStreamTransport
... 4 more ...

Since:

@@ +614,3 @@
+ *
+ * Send @buffer_list to the installed RTCP callback for @trans.
+ *

Since:

::: gst/rtsp-server/rtsp-stream.c
@@ +2474,3 @@
+ send_ret = gst_rtsp_stream_transport_send_rtp (tr, buffer);
+ if (buffer_list)
+ send_ret = gst_rtsp_stream_transport_send_rtp_list (tr, buffer_list);

this will ignore any potential error if the sample contained both a buffer and
a buffer list, not sure if that is allowed to happen in practice

@@ +3332,3 @@
/* make appsink */
priv->appsink[i] = gst_element_factory_make ("appsink", NULL);
+ g_object_set (priv->appsink[i], "emit-signals", FALSE, "buffer-list",

emit-signals is FALSE by default in appsink

::: gst/rtsp-sink/gstrtspclientsink.c
@@ +3853,3 @@
+static gboolean
+do_send_data_list (GstBufferList * buffer_list, guint8 channel,
+ GstRTSPStreamContext * context)

Same comment as the do_send_list in rtsp-client
--
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.
"GStreamer" (GNOME Bugzilla)
2018-10-11 16:40:25 UTC
Permalink
https://bugzilla.gnome.org/show_bug.cgi?id=771525

Mathieu Duponchelle <***@gmail.com> changed:

What |Removed |Added
----------------------------------------------------------------------------
Attachment #373684|reviewed |needs-work
status| |
Attachment #373684|reviewed |needs-work
status| |

--- Comment #31 from Mathieu Duponchelle <***@gmail.com> ---
Review of attachment 373684:
--> (https://bugzilla.gnome.org/review?bug=771525&attachment=373684)

This will require updating sections.txt, lgtm apart from a few comments

::: gst/rtsp-server/rtsp-client.c
@@ +1187,3 @@

+static gboolean
+do_send_data_list (GstBufferList * buffer_list, guint8 channel,

The implementation seems similar to what rtsp-stream-transport will do if not
callback was provided, is this mostly a placeholder for future extension?

::: gst/rtsp-server/rtsp-media.c
@@ +2187,3 @@
g_object_set (appsrc, "block", TRUE, "format", GST_FORMAT_TIME, "is-live",
+ TRUE, "emit-signals", FALSE, NULL);
+ g_object_set (appsink, "sync", FALSE, "async", FALSE, "emit-signals",

emit-signals is false by default for appsink

::: gst/rtsp-server/rtsp-stream-transport.c
@@ +219,3 @@
+ *
+ * Install callbacks that will be called when data for a stream should be sent
+ * to a client. This is usually used when sending RTP/RTCP over TCP.

Since:

@@ -531,0 +566,7 @@
+/**
+ * gst_rtsp_stream_transport_send_rtp_list:
+ * @trans: a #GstRTSPStreamTransport
... 4 more ...

Since:

@@ +614,3 @@
+ *
+ * Send @buffer_list to the installed RTCP callback for @trans.
+ *

Since:

::: gst/rtsp-server/rtsp-stream.c
@@ +2474,3 @@
+ send_ret = gst_rtsp_stream_transport_send_rtp (tr, buffer);
+ if (buffer_list)
+ send_ret = gst_rtsp_stream_transport_send_rtp_list (tr, buffer_list);

this will ignore any potential error if the sample contained both a buffer and
a buffer list, not sure if that is allowed to happen in practice

@@ +3332,3 @@
/* make appsink */
priv->appsink[i] = gst_element_factory_make ("appsink", NULL);
+ g_object_set (priv->appsink[i], "emit-signals", FALSE, "buffer-list",

emit-signals is FALSE by default in appsink

::: gst/rtsp-sink/gstrtspclientsink.c
@@ +3853,3 @@
+static gboolean
+do_send_data_list (GstBufferList * buffer_list, guint8 channel,
+ GstRTSPStreamContext * context)

Same comment as the do_send_list in rtsp-client

--- Comment #32 from Mathieu Duponchelle <***@gmail.com> ---
Review of attachment 373684:
--> (https://bugzilla.gnome.org/review?bug=771525&attachment=373684)

This will require updating sections.txt, lgtm apart from a few comments

::: gst/rtsp-server/rtsp-client.c
@@ +1187,3 @@

+static gboolean
+do_send_data_list (GstBufferList * buffer_list, guint8 channel,

The implementation seems similar to what rtsp-stream-transport will do if not
callback was provided, is this mostly a placeholder for future extension?

::: gst/rtsp-server/rtsp-media.c
@@ +2187,3 @@
g_object_set (appsrc, "block", TRUE, "format", GST_FORMAT_TIME, "is-live",
+ TRUE, "emit-signals", FALSE, NULL);
+ g_object_set (appsink, "sync", FALSE, "async", FALSE, "emit-signals",

emit-signals is false by default for appsink

::: gst/rtsp-server/rtsp-stream-transport.c
@@ +219,3 @@
+ *
+ * Install callbacks that will be called when data for a stream should be sent
+ * to a client. This is usually used when sending RTP/RTCP over TCP.

Since:

@@ -531,0 +566,7 @@
+/**
+ * gst_rtsp_stream_transport_send_rtp_list:
+ * @trans: a #GstRTSPStreamTransport
... 4 more ...

Since:

@@ +614,3 @@
+ *
+ * Send @buffer_list to the installed RTCP callback for @trans.
+ *

Since:

::: gst/rtsp-server/rtsp-stream.c
@@ +2474,3 @@
+ send_ret = gst_rtsp_stream_transport_send_rtp (tr, buffer);
+ if (buffer_list)
+ send_ret = gst_rtsp_stream_transport_send_rtp_list (tr, buffer_list);

this will ignore any potential error if the sample contained both a buffer and
a buffer list, not sure if that is allowed to happen in practice

@@ +3332,3 @@
/* make appsink */
priv->appsink[i] = gst_element_factory_make ("appsink", NULL);
+ g_object_set (priv->appsink[i], "emit-signals", FALSE, "buffer-list",

emit-signals is FALSE by default in appsink

::: gst/rtsp-sink/gstrtspclientsink.c
@@ +3853,3 @@
+static gboolean
+do_send_data_list (GstBufferList * buffer_list, guint8 channel,
+ GstRTSPStreamContext * context)

Same comment as the do_send_list in rtsp-client

--- Comment #33 from Mathieu Duponchelle <***@gmail.com> ---
Review of attachment 373684:
--> (https://bugzilla.gnome.org/review?bug=771525&attachment=373684)

This will require updating sections.txt, lgtm apart from a few comments

::: gst/rtsp-server/rtsp-client.c
@@ +1187,3 @@

+static gboolean
+do_send_data_list (GstBufferList * buffer_list, guint8 channel,

The implementation seems similar to what rtsp-stream-transport will do if not
callback was provided, is this mostly a placeholder for future extension?

::: gst/rtsp-server/rtsp-media.c
@@ +2187,3 @@
g_object_set (appsrc, "block", TRUE, "format", GST_FORMAT_TIME, "is-live",
+ TRUE, "emit-signals", FALSE, NULL);
+ g_object_set (appsink, "sync", FALSE, "async", FALSE, "emit-signals",

emit-signals is false by default for appsink

::: gst/rtsp-server/rtsp-stream-transport.c
@@ +219,3 @@
+ *
+ * Install callbacks that will be called when data for a stream should be sent
+ * to a client. This is usually used when sending RTP/RTCP over TCP.

Since:

@@ -531,0 +566,7 @@
+/**
+ * gst_rtsp_stream_transport_send_rtp_list:
+ * @trans: a #GstRTSPStreamTransport
... 4 more ...

Since:

@@ +614,3 @@
+ *
+ * Send @buffer_list to the installed RTCP callback for @trans.
+ *

Since:

::: gst/rtsp-server/rtsp-stream.c
@@ +2474,3 @@
+ send_ret = gst_rtsp_stream_transport_send_rtp (tr, buffer);
+ if (buffer_list)
+ send_ret = gst_rtsp_stream_transport_send_rtp_list (tr, buffer_list);

this will ignore any potential error if the sample contained both a buffer and
a buffer list, not sure if that is allowed to happen in practice

@@ +3332,3 @@
/* make appsink */
priv->appsink[i] = gst_element_factory_make ("appsink", NULL);
+ g_object_set (priv->appsink[i], "emit-signals", FALSE, "buffer-list",

emit-signals is FALSE by default in appsink

::: gst/rtsp-sink/gstrtspclientsink.c
@@ +3853,3 @@
+static gboolean
+do_send_data_list (GstBufferList * buffer_list, guint8 channel,
+ GstRTSPStreamContext * context)

Same comment as the do_send_list in rtsp-client

--- Comment #34 from Mathieu Duponchelle <***@gmail.com> ---
Review of attachment 373684:
--> (https://bugzilla.gnome.org/review?bug=771525&attachment=373684)

This will require updating sections.txt, lgtm apart from a few comments

::: gst/rtsp-server/rtsp-client.c
@@ +1187,3 @@

+static gboolean
+do_send_data_list (GstBufferList * buffer_list, guint8 channel,

The implementation seems similar to what rtsp-stream-transport will do if not
callback was provided, is this mostly a placeholder for future extension?

::: gst/rtsp-server/rtsp-media.c
@@ +2187,3 @@
g_object_set (appsrc, "block", TRUE, "format", GST_FORMAT_TIME, "is-live",
+ TRUE, "emit-signals", FALSE, NULL);
+ g_object_set (appsink, "sync", FALSE, "async", FALSE, "emit-signals",

emit-signals is false by default for appsink

::: gst/rtsp-server/rtsp-stream-transport.c
@@ +219,3 @@
+ *
+ * Install callbacks that will be called when data for a stream should be sent
+ * to a client. This is usually used when sending RTP/RTCP over TCP.

Since:

@@ -531,0 +566,7 @@
+/**
+ * gst_rtsp_stream_transport_send_rtp_list:
+ * @trans: a #GstRTSPStreamTransport
... 4 more ...

Since:

@@ +614,3 @@
+ *
+ * Send @buffer_list to the installed RTCP callback for @trans.
+ *

Since:

::: gst/rtsp-server/rtsp-stream.c
@@ +2474,3 @@
+ send_ret = gst_rtsp_stream_transport_send_rtp (tr, buffer);
+ if (buffer_list)
+ send_ret = gst_rtsp_stream_transport_send_rtp_list (tr, buffer_list);

this will ignore any potential error if the sample contained both a buffer and
a buffer list, not sure if that is allowed to happen in practice

@@ +3332,3 @@
/* make appsink */
priv->appsink[i] = gst_element_factory_make ("appsink", NULL);
+ g_object_set (priv->appsink[i], "emit-signals", FALSE, "buffer-list",

emit-signals is FALSE by default in appsink

::: gst/rtsp-sink/gstrtspclientsink.c
@@ +3853,3 @@
+static gboolean
+do_send_data_list (GstBufferList * buffer_list, guint8 channel,
+ GstRTSPStreamContext * context)

Same comment as the do_send_list in rtsp-client

--- Comment #35 from Mathieu Duponchelle <***@gmail.com> ---
Review of attachment 373684:
--> (https://bugzilla.gnome.org/review?bug=771525&attachment=373684)

This will require updating sections.txt, lgtm apart from a few comments

::: gst/rtsp-server/rtsp-client.c
@@ +1187,3 @@

+static gboolean
+do_send_data_list (GstBufferList * buffer_list, guint8 channel,

The implementation seems similar to what rtsp-stream-transport will do if not
callback was provided, is this mostly a placeholder for future extension?

::: gst/rtsp-server/rtsp-media.c
@@ +2187,3 @@
g_object_set (appsrc, "block", TRUE, "format", GST_FORMAT_TIME, "is-live",
+ TRUE, "emit-signals", FALSE, NULL);
+ g_object_set (appsink, "sync", FALSE, "async", FALSE, "emit-signals",

emit-signals is false by default for appsink

::: gst/rtsp-server/rtsp-stream-transport.c
@@ +219,3 @@
+ *
+ * Install callbacks that will be called when data for a stream should be sent
+ * to a client. This is usually used when sending RTP/RTCP over TCP.

Since:

@@ -531,0 +566,7 @@
+/**
+ * gst_rtsp_stream_transport_send_rtp_list:
+ * @trans: a #GstRTSPStreamTransport
... 4 more ...

Since:

@@ +614,3 @@
+ *
+ * Send @buffer_list to the installed RTCP callback for @trans.
+ *

Since:

::: gst/rtsp-server/rtsp-stream.c
@@ +2474,3 @@
+ send_ret = gst_rtsp_stream_transport_send_rtp (tr, buffer);
+ if (buffer_list)
+ send_ret = gst_rtsp_stream_transport_send_rtp_list (tr, buffer_list);

this will ignore any potential error if the sample contained both a buffer and
a buffer list, not sure if that is allowed to happen in practice

@@ +3332,3 @@
/* make appsink */
priv->appsink[i] = gst_element_factory_make ("appsink", NULL);
+ g_object_set (priv->appsink[i], "emit-signals", FALSE, "buffer-list",

emit-signals is FALSE by default in appsink

::: gst/rtsp-sink/gstrtspclientsink.c
@@ +3853,3 @@
+static gboolean
+do_send_data_list (GstBufferList * buffer_list, guint8 channel,
+ GstRTSPStreamContext * context)

Same comment as the do_send_list in rtsp-client
--
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.
"GStreamer" (GNOME Bugzilla)
2018-10-11 16:40:01 UTC
Permalink
https://bugzilla.gnome.org/show_bug.cgi?id=771525

--- Comment #31 from Mathieu Duponchelle <***@gmail.com> ---
Review of attachment 373684:
--> (https://bugzilla.gnome.org/review?bug=771525&attachment=373684)

This will require updating sections.txt, lgtm apart from a few comments

::: gst/rtsp-server/rtsp-client.c
@@ +1187,3 @@

+static gboolean
+do_send_data_list (GstBufferList * buffer_list, guint8 channel,

The implementation seems similar to what rtsp-stream-transport will do if not
callback was provided, is this mostly a placeholder for future extension?

::: gst/rtsp-server/rtsp-media.c
@@ +2187,3 @@
g_object_set (appsrc, "block", TRUE, "format", GST_FORMAT_TIME, "is-live",
+ TRUE, "emit-signals", FALSE, NULL);
+ g_object_set (appsink, "sync", FALSE, "async", FALSE, "emit-signals",

emit-signals is false by default for appsink

::: gst/rtsp-server/rtsp-stream-transport.c
@@ +219,3 @@
+ *
+ * Install callbacks that will be called when data for a stream should be sent
+ * to a client. This is usually used when sending RTP/RTCP over TCP.

Since:

@@ -531,0 +566,7 @@
+/**
+ * gst_rtsp_stream_transport_send_rtp_list:
+ * @trans: a #GstRTSPStreamTransport
... 4 more ...

Since:

@@ +614,3 @@
+ *
+ * Send @buffer_list to the installed RTCP callback for @trans.
+ *

Since:

::: gst/rtsp-server/rtsp-stream.c
@@ +2474,3 @@
+ send_ret = gst_rtsp_stream_transport_send_rtp (tr, buffer);
+ if (buffer_list)
+ send_ret = gst_rtsp_stream_transport_send_rtp_list (tr, buffer_list);

this will ignore any potential error if the sample contained both a buffer and
a buffer list, not sure if that is allowed to happen in practice

@@ +3332,3 @@
/* make appsink */
priv->appsink[i] = gst_element_factory_make ("appsink", NULL);
+ g_object_set (priv->appsink[i], "emit-signals", FALSE, "buffer-list",

emit-signals is FALSE by default in appsink

::: gst/rtsp-sink/gstrtspclientsink.c
@@ +3853,3 @@
+static gboolean
+do_send_data_list (GstBufferList * buffer_list, guint8 channel,
+ GstRTSPStreamContext * context)

Same comment as the do_send_list in rtsp-client

--- Comment #32 from Mathieu Duponchelle <***@gmail.com> ---
Review of attachment 373684:
--> (https://bugzilla.gnome.org/review?bug=771525&attachment=373684)

This will require updating sections.txt, lgtm apart from a few comments

::: gst/rtsp-server/rtsp-client.c
@@ +1187,3 @@

+static gboolean
+do_send_data_list (GstBufferList * buffer_list, guint8 channel,

The implementation seems similar to what rtsp-stream-transport will do if not
callback was provided, is this mostly a placeholder for future extension?

::: gst/rtsp-server/rtsp-media.c
@@ +2187,3 @@
g_object_set (appsrc, "block", TRUE, "format", GST_FORMAT_TIME, "is-live",
+ TRUE, "emit-signals", FALSE, NULL);
+ g_object_set (appsink, "sync", FALSE, "async", FALSE, "emit-signals",

emit-signals is false by default for appsink

::: gst/rtsp-server/rtsp-stream-transport.c
@@ +219,3 @@
+ *
+ * Install callbacks that will be called when data for a stream should be sent
+ * to a client. This is usually used when sending RTP/RTCP over TCP.

Since:

@@ -531,0 +566,7 @@
+/**
+ * gst_rtsp_stream_transport_send_rtp_list:
+ * @trans: a #GstRTSPStreamTransport
... 4 more ...

Since:

@@ +614,3 @@
+ *
+ * Send @buffer_list to the installed RTCP callback for @trans.
+ *

Since:

::: gst/rtsp-server/rtsp-stream.c
@@ +2474,3 @@
+ send_ret = gst_rtsp_stream_transport_send_rtp (tr, buffer);
+ if (buffer_list)
+ send_ret = gst_rtsp_stream_transport_send_rtp_list (tr, buffer_list);

this will ignore any potential error if the sample contained both a buffer and
a buffer list, not sure if that is allowed to happen in practice

@@ +3332,3 @@
/* make appsink */
priv->appsink[i] = gst_element_factory_make ("appsink", NULL);
+ g_object_set (priv->appsink[i], "emit-signals", FALSE, "buffer-list",

emit-signals is FALSE by default in appsink

::: gst/rtsp-sink/gstrtspclientsink.c
@@ +3853,3 @@
+static gboolean
+do_send_data_list (GstBufferList * buffer_list, guint8 channel,
+ GstRTSPStreamContext * context)

Same comment as the do_send_list in rtsp-client
--
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.
"GStreamer" (GNOME Bugzilla)
2018-10-11 16:40:32 UTC
Permalink
https://bugzilla.gnome.org/show_bug.cgi?id=771525

Mathieu Duponchelle <***@gmail.com> changed:

What |Removed |Added
----------------------------------------------------------------------------
Attachment #373684|reviewed |needs-work
status| |
Attachment #373684|reviewed |needs-work
status| |
Attachment #373684|reviewed |needs-work
status| |

--- Comment #31 from Mathieu Duponchelle <***@gmail.com> ---
Review of attachment 373684:
--> (https://bugzilla.gnome.org/review?bug=771525&attachment=373684)

This will require updating sections.txt, lgtm apart from a few comments

::: gst/rtsp-server/rtsp-client.c
@@ +1187,3 @@

+static gboolean
+do_send_data_list (GstBufferList * buffer_list, guint8 channel,

The implementation seems similar to what rtsp-stream-transport will do if not
callback was provided, is this mostly a placeholder for future extension?

::: gst/rtsp-server/rtsp-media.c
@@ +2187,3 @@
g_object_set (appsrc, "block", TRUE, "format", GST_FORMAT_TIME, "is-live",
+ TRUE, "emit-signals", FALSE, NULL);
+ g_object_set (appsink, "sync", FALSE, "async", FALSE, "emit-signals",

emit-signals is false by default for appsink

::: gst/rtsp-server/rtsp-stream-transport.c
@@ +219,3 @@
+ *
+ * Install callbacks that will be called when data for a stream should be sent
+ * to a client. This is usually used when sending RTP/RTCP over TCP.

Since:

@@ -531,0 +566,7 @@
+/**
+ * gst_rtsp_stream_transport_send_rtp_list:
+ * @trans: a #GstRTSPStreamTransport
... 4 more ...

Since:

@@ +614,3 @@
+ *
+ * Send @buffer_list to the installed RTCP callback for @trans.
+ *

Since:

::: gst/rtsp-server/rtsp-stream.c
@@ +2474,3 @@
+ send_ret = gst_rtsp_stream_transport_send_rtp (tr, buffer);
+ if (buffer_list)
+ send_ret = gst_rtsp_stream_transport_send_rtp_list (tr, buffer_list);

this will ignore any potential error if the sample contained both a buffer and
a buffer list, not sure if that is allowed to happen in practice

@@ +3332,3 @@
/* make appsink */
priv->appsink[i] = gst_element_factory_make ("appsink", NULL);
+ g_object_set (priv->appsink[i], "emit-signals", FALSE, "buffer-list",

emit-signals is FALSE by default in appsink

::: gst/rtsp-sink/gstrtspclientsink.c
@@ +3853,3 @@
+static gboolean
+do_send_data_list (GstBufferList * buffer_list, guint8 channel,
+ GstRTSPStreamContext * context)

Same comment as the do_send_list in rtsp-client

--- Comment #32 from Mathieu Duponchelle <***@gmail.com> ---
Review of attachment 373684:
--> (https://bugzilla.gnome.org/review?bug=771525&attachment=373684)

This will require updating sections.txt, lgtm apart from a few comments

::: gst/rtsp-server/rtsp-client.c
@@ +1187,3 @@

+static gboolean
+do_send_data_list (GstBufferList * buffer_list, guint8 channel,

The implementation seems similar to what rtsp-stream-transport will do if not
callback was provided, is this mostly a placeholder for future extension?

::: gst/rtsp-server/rtsp-media.c
@@ +2187,3 @@
g_object_set (appsrc, "block", TRUE, "format", GST_FORMAT_TIME, "is-live",
+ TRUE, "emit-signals", FALSE, NULL);
+ g_object_set (appsink, "sync", FALSE, "async", FALSE, "emit-signals",

emit-signals is false by default for appsink

::: gst/rtsp-server/rtsp-stream-transport.c
@@ +219,3 @@
+ *
+ * Install callbacks that will be called when data for a stream should be sent
+ * to a client. This is usually used when sending RTP/RTCP over TCP.

Since:

@@ -531,0 +566,7 @@
+/**
+ * gst_rtsp_stream_transport_send_rtp_list:
+ * @trans: a #GstRTSPStreamTransport
... 4 more ...

Since:

@@ +614,3 @@
+ *
+ * Send @buffer_list to the installed RTCP callback for @trans.
+ *

Since:

::: gst/rtsp-server/rtsp-stream.c
@@ +2474,3 @@
+ send_ret = gst_rtsp_stream_transport_send_rtp (tr, buffer);
+ if (buffer_list)
+ send_ret = gst_rtsp_stream_transport_send_rtp_list (tr, buffer_list);

this will ignore any potential error if the sample contained both a buffer and
a buffer list, not sure if that is allowed to happen in practice

@@ +3332,3 @@
/* make appsink */
priv->appsink[i] = gst_element_factory_make ("appsink", NULL);
+ g_object_set (priv->appsink[i], "emit-signals", FALSE, "buffer-list",

emit-signals is FALSE by default in appsink

::: gst/rtsp-sink/gstrtspclientsink.c
@@ +3853,3 @@
+static gboolean
+do_send_data_list (GstBufferList * buffer_list, guint8 channel,
+ GstRTSPStreamContext * context)

Same comment as the do_send_list in rtsp-client

--- Comment #33 from Mathieu Duponchelle <***@gmail.com> ---
Review of attachment 373684:
--> (https://bugzilla.gnome.org/review?bug=771525&attachment=373684)

This will require updating sections.txt, lgtm apart from a few comments

::: gst/rtsp-server/rtsp-client.c
@@ +1187,3 @@

+static gboolean
+do_send_data_list (GstBufferList * buffer_list, guint8 channel,

The implementation seems similar to what rtsp-stream-transport will do if not
callback was provided, is this mostly a placeholder for future extension?

::: gst/rtsp-server/rtsp-media.c
@@ +2187,3 @@
g_object_set (appsrc, "block", TRUE, "format", GST_FORMAT_TIME, "is-live",
+ TRUE, "emit-signals", FALSE, NULL);
+ g_object_set (appsink, "sync", FALSE, "async", FALSE, "emit-signals",

emit-signals is false by default for appsink

::: gst/rtsp-server/rtsp-stream-transport.c
@@ +219,3 @@
+ *
+ * Install callbacks that will be called when data for a stream should be sent
+ * to a client. This is usually used when sending RTP/RTCP over TCP.

Since:

@@ -531,0 +566,7 @@
+/**
+ * gst_rtsp_stream_transport_send_rtp_list:
+ * @trans: a #GstRTSPStreamTransport
... 4 more ...

Since:

@@ +614,3 @@
+ *
+ * Send @buffer_list to the installed RTCP callback for @trans.
+ *

Since:

::: gst/rtsp-server/rtsp-stream.c
@@ +2474,3 @@
+ send_ret = gst_rtsp_stream_transport_send_rtp (tr, buffer);
+ if (buffer_list)
+ send_ret = gst_rtsp_stream_transport_send_rtp_list (tr, buffer_list);

this will ignore any potential error if the sample contained both a buffer and
a buffer list, not sure if that is allowed to happen in practice

@@ +3332,3 @@
/* make appsink */
priv->appsink[i] = gst_element_factory_make ("appsink", NULL);
+ g_object_set (priv->appsink[i], "emit-signals", FALSE, "buffer-list",

emit-signals is FALSE by default in appsink

::: gst/rtsp-sink/gstrtspclientsink.c
@@ +3853,3 @@
+static gboolean
+do_send_data_list (GstBufferList * buffer_list, guint8 channel,
+ GstRTSPStreamContext * context)

Same comment as the do_send_list in rtsp-client

--- Comment #34 from Mathieu Duponchelle <***@gmail.com> ---
Review of attachment 373684:
--> (https://bugzilla.gnome.org/review?bug=771525&attachment=373684)

This will require updating sections.txt, lgtm apart from a few comments

::: gst/rtsp-server/rtsp-client.c
@@ +1187,3 @@

+static gboolean
+do_send_data_list (GstBufferList * buffer_list, guint8 channel,

The implementation seems similar to what rtsp-stream-transport will do if not
callback was provided, is this mostly a placeholder for future extension?

::: gst/rtsp-server/rtsp-media.c
@@ +2187,3 @@
g_object_set (appsrc, "block", TRUE, "format", GST_FORMAT_TIME, "is-live",
+ TRUE, "emit-signals", FALSE, NULL);
+ g_object_set (appsink, "sync", FALSE, "async", FALSE, "emit-signals",

emit-signals is false by default for appsink

::: gst/rtsp-server/rtsp-stream-transport.c
@@ +219,3 @@
+ *
+ * Install callbacks that will be called when data for a stream should be sent
+ * to a client. This is usually used when sending RTP/RTCP over TCP.

Since:

@@ -531,0 +566,7 @@
+/**
+ * gst_rtsp_stream_transport_send_rtp_list:
+ * @trans: a #GstRTSPStreamTransport
... 4 more ...

Since:

@@ +614,3 @@
+ *
+ * Send @buffer_list to the installed RTCP callback for @trans.
+ *

Since:

::: gst/rtsp-server/rtsp-stream.c
@@ +2474,3 @@
+ send_ret = gst_rtsp_stream_transport_send_rtp (tr, buffer);
+ if (buffer_list)
+ send_ret = gst_rtsp_stream_transport_send_rtp_list (tr, buffer_list);

this will ignore any potential error if the sample contained both a buffer and
a buffer list, not sure if that is allowed to happen in practice

@@ +3332,3 @@
/* make appsink */
priv->appsink[i] = gst_element_factory_make ("appsink", NULL);
+ g_object_set (priv->appsink[i], "emit-signals", FALSE, "buffer-list",

emit-signals is FALSE by default in appsink

::: gst/rtsp-sink/gstrtspclientsink.c
@@ +3853,3 @@
+static gboolean
+do_send_data_list (GstBufferList * buffer_list, guint8 channel,
+ GstRTSPStreamContext * context)

Same comment as the do_send_list in rtsp-client

--- Comment #35 from Mathieu Duponchelle <***@gmail.com> ---
Review of attachment 373684:
--> (https://bugzilla.gnome.org/review?bug=771525&attachment=373684)

This will require updating sections.txt, lgtm apart from a few comments

::: gst/rtsp-server/rtsp-client.c
@@ +1187,3 @@

+static gboolean
+do_send_data_list (GstBufferList * buffer_list, guint8 channel,

The implementation seems similar to what rtsp-stream-transport will do if not
callback was provided, is this mostly a placeholder for future extension?

::: gst/rtsp-server/rtsp-media.c
@@ +2187,3 @@
g_object_set (appsrc, "block", TRUE, "format", GST_FORMAT_TIME, "is-live",
+ TRUE, "emit-signals", FALSE, NULL);
+ g_object_set (appsink, "sync", FALSE, "async", FALSE, "emit-signals",

emit-signals is false by default for appsink

::: gst/rtsp-server/rtsp-stream-transport.c
@@ +219,3 @@
+ *
+ * Install callbacks that will be called when data for a stream should be sent
+ * to a client. This is usually used when sending RTP/RTCP over TCP.

Since:

@@ -531,0 +566,7 @@
+/**
+ * gst_rtsp_stream_transport_send_rtp_list:
+ * @trans: a #GstRTSPStreamTransport
... 4 more ...

Since:

@@ +614,3 @@
+ *
+ * Send @buffer_list to the installed RTCP callback for @trans.
+ *

Since:

::: gst/rtsp-server/rtsp-stream.c
@@ +2474,3 @@
+ send_ret = gst_rtsp_stream_transport_send_rtp (tr, buffer);
+ if (buffer_list)
+ send_ret = gst_rtsp_stream_transport_send_rtp_list (tr, buffer_list);

this will ignore any potential error if the sample contained both a buffer and
a buffer list, not sure if that is allowed to happen in practice

@@ +3332,3 @@
/* make appsink */
priv->appsink[i] = gst_element_factory_make ("appsink", NULL);
+ g_object_set (priv->appsink[i], "emit-signals", FALSE, "buffer-list",

emit-signals is FALSE by default in appsink

::: gst/rtsp-sink/gstrtspclientsink.c
@@ +3853,3 @@
+static gboolean
+do_send_data_list (GstBufferList * buffer_list, guint8 channel,
+ GstRTSPStreamContext * context)

Same comment as the do_send_list in rtsp-client

--- Comment #36 from Mathieu Duponchelle <***@gmail.com> ---
Review of attachment 373684:
--> (https://bugzilla.gnome.org/review?bug=771525&attachment=373684)

This will require updating sections.txt, lgtm apart from a few comments

::: gst/rtsp-server/rtsp-client.c
@@ +1187,3 @@

+static gboolean
+do_send_data_list (GstBufferList * buffer_list, guint8 channel,

The implementation seems similar to what rtsp-stream-transport will do if not
callback was provided, is this mostly a placeholder for future extension?

::: gst/rtsp-server/rtsp-media.c
@@ +2187,3 @@
g_object_set (appsrc, "block", TRUE, "format", GST_FORMAT_TIME, "is-live",
+ TRUE, "emit-signals", FALSE, NULL);
+ g_object_set (appsink, "sync", FALSE, "async", FALSE, "emit-signals",

emit-signals is false by default for appsink

::: gst/rtsp-server/rtsp-stream-transport.c
@@ +219,3 @@
+ *
+ * Install callbacks that will be called when data for a stream should be sent
+ * to a client. This is usually used when sending RTP/RTCP over TCP.

Since:

@@ -531,0 +566,7 @@
+/**
+ * gst_rtsp_stream_transport_send_rtp_list:
+ * @trans: a #GstRTSPStreamTransport
... 4 more ...

Since:

@@ +614,3 @@
+ *
+ * Send @buffer_list to the installed RTCP callback for @trans.
+ *

Since:

::: gst/rtsp-server/rtsp-stream.c
@@ +2474,3 @@
+ send_ret = gst_rtsp_stream_transport_send_rtp (tr, buffer);
+ if (buffer_list)
+ send_ret = gst_rtsp_stream_transport_send_rtp_list (tr, buffer_list);

this will ignore any potential error if the sample contained both a buffer and
a buffer list, not sure if that is allowed to happen in practice

@@ +3332,3 @@
/* make appsink */
priv->appsink[i] = gst_element_factory_make ("appsink", NULL);
+ g_object_set (priv->appsink[i], "emit-signals", FALSE, "buffer-list",

emit-signals is FALSE by default in appsink

::: gst/rtsp-sink/gstrtspclientsink.c
@@ +3853,3 @@
+static gboolean
+do_send_data_list (GstBufferList * buffer_list, guint8 channel,
+ GstRTSPStreamContext * context)

Same comment as the do_send_list in rtsp-client
--
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.
"GStreamer" (GNOME Bugzilla)
2018-10-11 16:40:18 UTC
Permalink
https://bugzilla.gnome.org/show_bug.cgi?id=771525

Mathieu Duponchelle <***@gmail.com> changed:

What |Removed |Added
----------------------------------------------------------------------------
Attachment #373684|reviewed |needs-work
status| |

--- Comment #31 from Mathieu Duponchelle <***@gmail.com> ---
Review of attachment 373684:
--> (https://bugzilla.gnome.org/review?bug=771525&attachment=373684)

This will require updating sections.txt, lgtm apart from a few comments

::: gst/rtsp-server/rtsp-client.c
@@ +1187,3 @@

+static gboolean
+do_send_data_list (GstBufferList * buffer_list, guint8 channel,

The implementation seems similar to what rtsp-stream-transport will do if not
callback was provided, is this mostly a placeholder for future extension?

::: gst/rtsp-server/rtsp-media.c
@@ +2187,3 @@
g_object_set (appsrc, "block", TRUE, "format", GST_FORMAT_TIME, "is-live",
+ TRUE, "emit-signals", FALSE, NULL);
+ g_object_set (appsink, "sync", FALSE, "async", FALSE, "emit-signals",

emit-signals is false by default for appsink

::: gst/rtsp-server/rtsp-stream-transport.c
@@ +219,3 @@
+ *
+ * Install callbacks that will be called when data for a stream should be sent
+ * to a client. This is usually used when sending RTP/RTCP over TCP.

Since:

@@ -531,0 +566,7 @@
+/**
+ * gst_rtsp_stream_transport_send_rtp_list:
+ * @trans: a #GstRTSPStreamTransport
... 4 more ...

Since:

@@ +614,3 @@
+ *
+ * Send @buffer_list to the installed RTCP callback for @trans.
+ *

Since:

::: gst/rtsp-server/rtsp-stream.c
@@ +2474,3 @@
+ send_ret = gst_rtsp_stream_transport_send_rtp (tr, buffer);
+ if (buffer_list)
+ send_ret = gst_rtsp_stream_transport_send_rtp_list (tr, buffer_list);

this will ignore any potential error if the sample contained both a buffer and
a buffer list, not sure if that is allowed to happen in practice

@@ +3332,3 @@
/* make appsink */
priv->appsink[i] = gst_element_factory_make ("appsink", NULL);
+ g_object_set (priv->appsink[i], "emit-signals", FALSE, "buffer-list",

emit-signals is FALSE by default in appsink

::: gst/rtsp-sink/gstrtspclientsink.c
@@ +3853,3 @@
+static gboolean
+do_send_data_list (GstBufferList * buffer_list, guint8 channel,
+ GstRTSPStreamContext * context)

Same comment as the do_send_list in rtsp-client

--- Comment #32 from Mathieu Duponchelle <***@gmail.com> ---
Review of attachment 373684:
--> (https://bugzilla.gnome.org/review?bug=771525&attachment=373684)

This will require updating sections.txt, lgtm apart from a few comments

::: gst/rtsp-server/rtsp-client.c
@@ +1187,3 @@

+static gboolean
+do_send_data_list (GstBufferList * buffer_list, guint8 channel,

The implementation seems similar to what rtsp-stream-transport will do if not
callback was provided, is this mostly a placeholder for future extension?

::: gst/rtsp-server/rtsp-media.c
@@ +2187,3 @@
g_object_set (appsrc, "block", TRUE, "format", GST_FORMAT_TIME, "is-live",
+ TRUE, "emit-signals", FALSE, NULL);
+ g_object_set (appsink, "sync", FALSE, "async", FALSE, "emit-signals",

emit-signals is false by default for appsink

::: gst/rtsp-server/rtsp-stream-transport.c
@@ +219,3 @@
+ *
+ * Install callbacks that will be called when data for a stream should be sent
+ * to a client. This is usually used when sending RTP/RTCP over TCP.

Since:

@@ -531,0 +566,7 @@
+/**
+ * gst_rtsp_stream_transport_send_rtp_list:
+ * @trans: a #GstRTSPStreamTransport
... 4 more ...

Since:

@@ +614,3 @@
+ *
+ * Send @buffer_list to the installed RTCP callback for @trans.
+ *

Since:

::: gst/rtsp-server/rtsp-stream.c
@@ +2474,3 @@
+ send_ret = gst_rtsp_stream_transport_send_rtp (tr, buffer);
+ if (buffer_list)
+ send_ret = gst_rtsp_stream_transport_send_rtp_list (tr, buffer_list);

this will ignore any potential error if the sample contained both a buffer and
a buffer list, not sure if that is allowed to happen in practice

@@ +3332,3 @@
/* make appsink */
priv->appsink[i] = gst_element_factory_make ("appsink", NULL);
+ g_object_set (priv->appsink[i], "emit-signals", FALSE, "buffer-list",

emit-signals is FALSE by default in appsink

::: gst/rtsp-sink/gstrtspclientsink.c
@@ +3853,3 @@
+static gboolean
+do_send_data_list (GstBufferList * buffer_list, guint8 channel,
+ GstRTSPStreamContext * context)

Same comment as the do_send_list in rtsp-client

--- Comment #33 from Mathieu Duponchelle <***@gmail.com> ---
Review of attachment 373684:
--> (https://bugzilla.gnome.org/review?bug=771525&attachment=373684)

This will require updating sections.txt, lgtm apart from a few comments

::: gst/rtsp-server/rtsp-client.c
@@ +1187,3 @@

+static gboolean
+do_send_data_list (GstBufferList * buffer_list, guint8 channel,

The implementation seems similar to what rtsp-stream-transport will do if not
callback was provided, is this mostly a placeholder for future extension?

::: gst/rtsp-server/rtsp-media.c
@@ +2187,3 @@
g_object_set (appsrc, "block", TRUE, "format", GST_FORMAT_TIME, "is-live",
+ TRUE, "emit-signals", FALSE, NULL);
+ g_object_set (appsink, "sync", FALSE, "async", FALSE, "emit-signals",

emit-signals is false by default for appsink

::: gst/rtsp-server/rtsp-stream-transport.c
@@ +219,3 @@
+ *
+ * Install callbacks that will be called when data for a stream should be sent
+ * to a client. This is usually used when sending RTP/RTCP over TCP.

Since:

@@ -531,0 +566,7 @@
+/**
+ * gst_rtsp_stream_transport_send_rtp_list:
+ * @trans: a #GstRTSPStreamTransport
... 4 more ...

Since:

@@ +614,3 @@
+ *
+ * Send @buffer_list to the installed RTCP callback for @trans.
+ *

Since:

::: gst/rtsp-server/rtsp-stream.c
@@ +2474,3 @@
+ send_ret = gst_rtsp_stream_transport_send_rtp (tr, buffer);
+ if (buffer_list)
+ send_ret = gst_rtsp_stream_transport_send_rtp_list (tr, buffer_list);

this will ignore any potential error if the sample contained both a buffer and
a buffer list, not sure if that is allowed to happen in practice

@@ +3332,3 @@
/* make appsink */
priv->appsink[i] = gst_element_factory_make ("appsink", NULL);
+ g_object_set (priv->appsink[i], "emit-signals", FALSE, "buffer-list",

emit-signals is FALSE by default in appsink

::: gst/rtsp-sink/gstrtspclientsink.c
@@ +3853,3 @@
+static gboolean
+do_send_data_list (GstBufferList * buffer_list, guint8 channel,
+ GstRTSPStreamContext * context)

Same comment as the do_send_list in rtsp-client

--- Comment #34 from Mathieu Duponchelle <***@gmail.com> ---
Review of attachment 373684:
--> (https://bugzilla.gnome.org/review?bug=771525&attachment=373684)

This will require updating sections.txt, lgtm apart from a few comments

::: gst/rtsp-server/rtsp-client.c
@@ +1187,3 @@

+static gboolean
+do_send_data_list (GstBufferList * buffer_list, guint8 channel,

The implementation seems similar to what rtsp-stream-transport will do if not
callback was provided, is this mostly a placeholder for future extension?

::: gst/rtsp-server/rtsp-media.c
@@ +2187,3 @@
g_object_set (appsrc, "block", TRUE, "format", GST_FORMAT_TIME, "is-live",
+ TRUE, "emit-signals", FALSE, NULL);
+ g_object_set (appsink, "sync", FALSE, "async", FALSE, "emit-signals",

emit-signals is false by default for appsink

::: gst/rtsp-server/rtsp-stream-transport.c
@@ +219,3 @@
+ *
+ * Install callbacks that will be called when data for a stream should be sent
+ * to a client. This is usually used when sending RTP/RTCP over TCP.

Since:

@@ -531,0 +566,7 @@
+/**
+ * gst_rtsp_stream_transport_send_rtp_list:
+ * @trans: a #GstRTSPStreamTransport
... 4 more ...

Since:

@@ +614,3 @@
+ *
+ * Send @buffer_list to the installed RTCP callback for @trans.
+ *

Since:

::: gst/rtsp-server/rtsp-stream.c
@@ +2474,3 @@
+ send_ret = gst_rtsp_stream_transport_send_rtp (tr, buffer);
+ if (buffer_list)
+ send_ret = gst_rtsp_stream_transport_send_rtp_list (tr, buffer_list);

this will ignore any potential error if the sample contained both a buffer and
a buffer list, not sure if that is allowed to happen in practice

@@ +3332,3 @@
/* make appsink */
priv->appsink[i] = gst_element_factory_make ("appsink", NULL);
+ g_object_set (priv->appsink[i], "emit-signals", FALSE, "buffer-list",

emit-signals is FALSE by default in appsink

::: gst/rtsp-sink/gstrtspclientsink.c
@@ +3853,3 @@
+static gboolean
+do_send_data_list (GstBufferList * buffer_list, guint8 channel,
+ GstRTSPStreamContext * context)

Same comment as the do_send_list in rtsp-client
--
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.
"GStreamer" (GNOME Bugzilla)
2018-10-11 16:40:47 UTC
Permalink
https://bugzilla.gnome.org/show_bug.cgi?id=771525

Mathieu Duponchelle <***@gmail.com> changed:

What |Removed |Added
----------------------------------------------------------------------------
Attachment #373684|reviewed |needs-work
status| |
Attachment #373684|reviewed |needs-work
status| |
Attachment #373684|reviewed reviewed reviewed |needs-work needs-work
status| |needs-work

--- Comment #31 from Mathieu Duponchelle <***@gmail.com> ---
Review of attachment 373684:
--> (https://bugzilla.gnome.org/review?bug=771525&attachment=373684)

This will require updating sections.txt, lgtm apart from a few comments

::: gst/rtsp-server/rtsp-client.c
@@ +1187,3 @@

+static gboolean
+do_send_data_list (GstBufferList * buffer_list, guint8 channel,

The implementation seems similar to what rtsp-stream-transport will do if not
callback was provided, is this mostly a placeholder for future extension?

::: gst/rtsp-server/rtsp-media.c
@@ +2187,3 @@
g_object_set (appsrc, "block", TRUE, "format", GST_FORMAT_TIME, "is-live",
+ TRUE, "emit-signals", FALSE, NULL);
+ g_object_set (appsink, "sync", FALSE, "async", FALSE, "emit-signals",

emit-signals is false by default for appsink

::: gst/rtsp-server/rtsp-stream-transport.c
@@ +219,3 @@
+ *
+ * Install callbacks that will be called when data for a stream should be sent
+ * to a client. This is usually used when sending RTP/RTCP over TCP.

Since:

@@ -531,0 +566,7 @@
+/**
+ * gst_rtsp_stream_transport_send_rtp_list:
+ * @trans: a #GstRTSPStreamTransport
... 4 more ...

Since:

@@ +614,3 @@
+ *
+ * Send @buffer_list to the installed RTCP callback for @trans.
+ *

Since:

::: gst/rtsp-server/rtsp-stream.c
@@ +2474,3 @@
+ send_ret = gst_rtsp_stream_transport_send_rtp (tr, buffer);
+ if (buffer_list)
+ send_ret = gst_rtsp_stream_transport_send_rtp_list (tr, buffer_list);

this will ignore any potential error if the sample contained both a buffer and
a buffer list, not sure if that is allowed to happen in practice

@@ +3332,3 @@
/* make appsink */
priv->appsink[i] = gst_element_factory_make ("appsink", NULL);
+ g_object_set (priv->appsink[i], "emit-signals", FALSE, "buffer-list",

emit-signals is FALSE by default in appsink

::: gst/rtsp-sink/gstrtspclientsink.c
@@ +3853,3 @@
+static gboolean
+do_send_data_list (GstBufferList * buffer_list, guint8 channel,
+ GstRTSPStreamContext * context)

Same comment as the do_send_list in rtsp-client

--- Comment #32 from Mathieu Duponchelle <***@gmail.com> ---
Review of attachment 373684:
--> (https://bugzilla.gnome.org/review?bug=771525&attachment=373684)

This will require updating sections.txt, lgtm apart from a few comments

::: gst/rtsp-server/rtsp-client.c
@@ +1187,3 @@

+static gboolean
+do_send_data_list (GstBufferList * buffer_list, guint8 channel,

The implementation seems similar to what rtsp-stream-transport will do if not
callback was provided, is this mostly a placeholder for future extension?

::: gst/rtsp-server/rtsp-media.c
@@ +2187,3 @@
g_object_set (appsrc, "block", TRUE, "format", GST_FORMAT_TIME, "is-live",
+ TRUE, "emit-signals", FALSE, NULL);
+ g_object_set (appsink, "sync", FALSE, "async", FALSE, "emit-signals",

emit-signals is false by default for appsink

::: gst/rtsp-server/rtsp-stream-transport.c
@@ +219,3 @@
+ *
+ * Install callbacks that will be called when data for a stream should be sent
+ * to a client. This is usually used when sending RTP/RTCP over TCP.

Since:

@@ -531,0 +566,7 @@
+/**
+ * gst_rtsp_stream_transport_send_rtp_list:
+ * @trans: a #GstRTSPStreamTransport
... 4 more ...

Since:

@@ +614,3 @@
+ *
+ * Send @buffer_list to the installed RTCP callback for @trans.
+ *

Since:

::: gst/rtsp-server/rtsp-stream.c
@@ +2474,3 @@
+ send_ret = gst_rtsp_stream_transport_send_rtp (tr, buffer);
+ if (buffer_list)
+ send_ret = gst_rtsp_stream_transport_send_rtp_list (tr, buffer_list);

this will ignore any potential error if the sample contained both a buffer and
a buffer list, not sure if that is allowed to happen in practice

@@ +3332,3 @@
/* make appsink */
priv->appsink[i] = gst_element_factory_make ("appsink", NULL);
+ g_object_set (priv->appsink[i], "emit-signals", FALSE, "buffer-list",

emit-signals is FALSE by default in appsink

::: gst/rtsp-sink/gstrtspclientsink.c
@@ +3853,3 @@
+static gboolean
+do_send_data_list (GstBufferList * buffer_list, guint8 channel,
+ GstRTSPStreamContext * context)

Same comment as the do_send_list in rtsp-client

--- Comment #33 from Mathieu Duponchelle <***@gmail.com> ---
Review of attachment 373684:
--> (https://bugzilla.gnome.org/review?bug=771525&attachment=373684)

This will require updating sections.txt, lgtm apart from a few comments

::: gst/rtsp-server/rtsp-client.c
@@ +1187,3 @@

+static gboolean
+do_send_data_list (GstBufferList * buffer_list, guint8 channel,

The implementation seems similar to what rtsp-stream-transport will do if not
callback was provided, is this mostly a placeholder for future extension?

::: gst/rtsp-server/rtsp-media.c
@@ +2187,3 @@
g_object_set (appsrc, "block", TRUE, "format", GST_FORMAT_TIME, "is-live",
+ TRUE, "emit-signals", FALSE, NULL);
+ g_object_set (appsink, "sync", FALSE, "async", FALSE, "emit-signals",

emit-signals is false by default for appsink

::: gst/rtsp-server/rtsp-stream-transport.c
@@ +219,3 @@
+ *
+ * Install callbacks that will be called when data for a stream should be sent
+ * to a client. This is usually used when sending RTP/RTCP over TCP.

Since:

@@ -531,0 +566,7 @@
+/**
+ * gst_rtsp_stream_transport_send_rtp_list:
+ * @trans: a #GstRTSPStreamTransport
... 4 more ...

Since:

@@ +614,3 @@
+ *
+ * Send @buffer_list to the installed RTCP callback for @trans.
+ *

Since:

::: gst/rtsp-server/rtsp-stream.c
@@ +2474,3 @@
+ send_ret = gst_rtsp_stream_transport_send_rtp (tr, buffer);
+ if (buffer_list)
+ send_ret = gst_rtsp_stream_transport_send_rtp_list (tr, buffer_list);

this will ignore any potential error if the sample contained both a buffer and
a buffer list, not sure if that is allowed to happen in practice

@@ +3332,3 @@
/* make appsink */
priv->appsink[i] = gst_element_factory_make ("appsink", NULL);
+ g_object_set (priv->appsink[i], "emit-signals", FALSE, "buffer-list",

emit-signals is FALSE by default in appsink

::: gst/rtsp-sink/gstrtspclientsink.c
@@ +3853,3 @@
+static gboolean
+do_send_data_list (GstBufferList * buffer_list, guint8 channel,
+ GstRTSPStreamContext * context)

Same comment as the do_send_list in rtsp-client

--- Comment #34 from Mathieu Duponchelle <***@gmail.com> ---
Review of attachment 373684:
--> (https://bugzilla.gnome.org/review?bug=771525&attachment=373684)

This will require updating sections.txt, lgtm apart from a few comments

::: gst/rtsp-server/rtsp-client.c
@@ +1187,3 @@

+static gboolean
+do_send_data_list (GstBufferList * buffer_list, guint8 channel,

The implementation seems similar to what rtsp-stream-transport will do if not
callback was provided, is this mostly a placeholder for future extension?

::: gst/rtsp-server/rtsp-media.c
@@ +2187,3 @@
g_object_set (appsrc, "block", TRUE, "format", GST_FORMAT_TIME, "is-live",
+ TRUE, "emit-signals", FALSE, NULL);
+ g_object_set (appsink, "sync", FALSE, "async", FALSE, "emit-signals",

emit-signals is false by default for appsink

::: gst/rtsp-server/rtsp-stream-transport.c
@@ +219,3 @@
+ *
+ * Install callbacks that will be called when data for a stream should be sent
+ * to a client. This is usually used when sending RTP/RTCP over TCP.

Since:

@@ -531,0 +566,7 @@
+/**
+ * gst_rtsp_stream_transport_send_rtp_list:
+ * @trans: a #GstRTSPStreamTransport
... 4 more ...

Since:

@@ +614,3 @@
+ *
+ * Send @buffer_list to the installed RTCP callback for @trans.
+ *

Since:

::: gst/rtsp-server/rtsp-stream.c
@@ +2474,3 @@
+ send_ret = gst_rtsp_stream_transport_send_rtp (tr, buffer);
+ if (buffer_list)
+ send_ret = gst_rtsp_stream_transport_send_rtp_list (tr, buffer_list);

this will ignore any potential error if the sample contained both a buffer and
a buffer list, not sure if that is allowed to happen in practice

@@ +3332,3 @@
/* make appsink */
priv->appsink[i] = gst_element_factory_make ("appsink", NULL);
+ g_object_set (priv->appsink[i], "emit-signals", FALSE, "buffer-list",

emit-signals is FALSE by default in appsink

::: gst/rtsp-sink/gstrtspclientsink.c
@@ +3853,3 @@
+static gboolean
+do_send_data_list (GstBufferList * buffer_list, guint8 channel,
+ GstRTSPStreamContext * context)

Same comment as the do_send_list in rtsp-client

--- Comment #35 from Mathieu Duponchelle <***@gmail.com> ---
Review of attachment 373684:
--> (https://bugzilla.gnome.org/review?bug=771525&attachment=373684)

This will require updating sections.txt, lgtm apart from a few comments

::: gst/rtsp-server/rtsp-client.c
@@ +1187,3 @@

+static gboolean
+do_send_data_list (GstBufferList * buffer_list, guint8 channel,

The implementation seems similar to what rtsp-stream-transport will do if not
callback was provided, is this mostly a placeholder for future extension?

::: gst/rtsp-server/rtsp-media.c
@@ +2187,3 @@
g_object_set (appsrc, "block", TRUE, "format", GST_FORMAT_TIME, "is-live",
+ TRUE, "emit-signals", FALSE, NULL);
+ g_object_set (appsink, "sync", FALSE, "async", FALSE, "emit-signals",

emit-signals is false by default for appsink

::: gst/rtsp-server/rtsp-stream-transport.c
@@ +219,3 @@
+ *
+ * Install callbacks that will be called when data for a stream should be sent
+ * to a client. This is usually used when sending RTP/RTCP over TCP.

Since:

@@ -531,0 +566,7 @@
+/**
+ * gst_rtsp_stream_transport_send_rtp_list:
+ * @trans: a #GstRTSPStreamTransport
... 4 more ...

Since:

@@ +614,3 @@
+ *
+ * Send @buffer_list to the installed RTCP callback for @trans.
+ *

Since:

::: gst/rtsp-server/rtsp-stream.c
@@ +2474,3 @@
+ send_ret = gst_rtsp_stream_transport_send_rtp (tr, buffer);
+ if (buffer_list)
+ send_ret = gst_rtsp_stream_transport_send_rtp_list (tr, buffer_list);

this will ignore any potential error if the sample contained both a buffer and
a buffer list, not sure if that is allowed to happen in practice

@@ +3332,3 @@
/* make appsink */
priv->appsink[i] = gst_element_factory_make ("appsink", NULL);
+ g_object_set (priv->appsink[i], "emit-signals", FALSE, "buffer-list",

emit-signals is FALSE by default in appsink

::: gst/rtsp-sink/gstrtspclientsink.c
@@ +3853,3 @@
+static gboolean
+do_send_data_list (GstBufferList * buffer_list, guint8 channel,
+ GstRTSPStreamContext * context)

Same comment as the do_send_list in rtsp-client

--- Comment #36 from Mathieu Duponchelle <***@gmail.com> ---
Review of attachment 373684:
--> (https://bugzilla.gnome.org/review?bug=771525&attachment=373684)

This will require updating sections.txt, lgtm apart from a few comments

::: gst/rtsp-server/rtsp-client.c
@@ +1187,3 @@

+static gboolean
+do_send_data_list (GstBufferList * buffer_list, guint8 channel,

The implementation seems similar to what rtsp-stream-transport will do if not
callback was provided, is this mostly a placeholder for future extension?

::: gst/rtsp-server/rtsp-media.c
@@ +2187,3 @@
g_object_set (appsrc, "block", TRUE, "format", GST_FORMAT_TIME, "is-live",
+ TRUE, "emit-signals", FALSE, NULL);
+ g_object_set (appsink, "sync", FALSE, "async", FALSE, "emit-signals",

emit-signals is false by default for appsink

::: gst/rtsp-server/rtsp-stream-transport.c
@@ +219,3 @@
+ *
+ * Install callbacks that will be called when data for a stream should be sent
+ * to a client. This is usually used when sending RTP/RTCP over TCP.

Since:

@@ -531,0 +566,7 @@
+/**
+ * gst_rtsp_stream_transport_send_rtp_list:
+ * @trans: a #GstRTSPStreamTransport
... 4 more ...

Since:

@@ +614,3 @@
+ *
+ * Send @buffer_list to the installed RTCP callback for @trans.
+ *

Since:

::: gst/rtsp-server/rtsp-stream.c
@@ +2474,3 @@
+ send_ret = gst_rtsp_stream_transport_send_rtp (tr, buffer);
+ if (buffer_list)
+ send_ret = gst_rtsp_stream_transport_send_rtp_list (tr, buffer_list);

this will ignore any potential error if the sample contained both a buffer and
a buffer list, not sure if that is allowed to happen in practice

@@ +3332,3 @@
/* make appsink */
priv->appsink[i] = gst_element_factory_make ("appsink", NULL);
+ g_object_set (priv->appsink[i], "emit-signals", FALSE, "buffer-list",

emit-signals is FALSE by default in appsink

::: gst/rtsp-sink/gstrtspclientsink.c
@@ +3853,3 @@
+static gboolean
+do_send_data_list (GstBufferList * buffer_list, guint8 channel,
+ GstRTSPStreamContext * context)

Same comment as the do_send_list in rtsp-client

--- Comment #37 from Mathieu Duponchelle <***@gmail.com> ---
Review of attachment 373684:
--> (https://bugzilla.gnome.org/review?bug=771525&attachment=373684)

This will require updating sections.txt, lgtm apart from a few comments

::: gst/rtsp-server/rtsp-client.c
@@ +1187,3 @@

+static gboolean
+do_send_data_list (GstBufferList * buffer_list, guint8 channel,

The implementation seems similar to what rtsp-stream-transport will do if not
callback was provided, is this mostly a placeholder for future extension?

::: gst/rtsp-server/rtsp-media.c
@@ +2187,3 @@
g_object_set (appsrc, "block", TRUE, "format", GST_FORMAT_TIME, "is-live",
+ TRUE, "emit-signals", FALSE, NULL);
+ g_object_set (appsink, "sync", FALSE, "async", FALSE, "emit-signals",

emit-signals is false by default for appsink

::: gst/rtsp-server/rtsp-stream-transport.c
@@ +219,3 @@
+ *
+ * Install callbacks that will be called when data for a stream should be sent
+ * to a client. This is usually used when sending RTP/RTCP over TCP.

Since:

@@ -531,0 +566,7 @@
+/**
+ * gst_rtsp_stream_transport_send_rtp_list:
+ * @trans: a #GstRTSPStreamTransport
... 4 more ...

Since:

@@ +614,3 @@
+ *
+ * Send @buffer_list to the installed RTCP callback for @trans.
+ *

Since:

::: gst/rtsp-server/rtsp-stream.c
@@ +2474,3 @@
+ send_ret = gst_rtsp_stream_transport_send_rtp (tr, buffer);
+ if (buffer_list)
+ send_ret = gst_rtsp_stream_transport_send_rtp_list (tr, buffer_list);

this will ignore any potential error if the sample contained both a buffer and
a buffer list, not sure if that is allowed to happen in practice

@@ +3332,3 @@
/* make appsink */
priv->appsink[i] = gst_element_factory_make ("appsink", NULL);
+ g_object_set (priv->appsink[i], "emit-signals", FALSE, "buffer-list",

emit-signals is FALSE by default in appsink

::: gst/rtsp-sink/gstrtspclientsink.c
@@ +3853,3 @@
+static gboolean
+do_send_data_list (GstBufferList * buffer_list, guint8 channel,
+ GstRTSPStreamContext * context)

Same comment as the do_send_list in rtsp-client

--- Comment #38 from Mathieu Duponchelle <***@gmail.com> ---
Review of attachment 373684:
--> (https://bugzilla.gnome.org/review?bug=771525&attachment=373684)

This will require updating sections.txt, lgtm apart from a few comments

::: gst/rtsp-server/rtsp-client.c
@@ +1187,3 @@

+static gboolean
+do_send_data_list (GstBufferList * buffer_list, guint8 channel,

The implementation seems similar to what rtsp-stream-transport will do if not
callback was provided, is this mostly a placeholder for future extension?

::: gst/rtsp-server/rtsp-media.c
@@ +2187,3 @@
g_object_set (appsrc, "block", TRUE, "format", GST_FORMAT_TIME, "is-live",
+ TRUE, "emit-signals", FALSE, NULL);
+ g_object_set (appsink, "sync", FALSE, "async", FALSE, "emit-signals",

emit-signals is false by default for appsink

::: gst/rtsp-server/rtsp-stream-transport.c
@@ +219,3 @@
+ *
+ * Install callbacks that will be called when data for a stream should be sent
+ * to a client. This is usually used when sending RTP/RTCP over TCP.

Since:

@@ -531,0 +566,7 @@
+/**
+ * gst_rtsp_stream_transport_send_rtp_list:
+ * @trans: a #GstRTSPStreamTransport
... 4 more ...

Since:

@@ +614,3 @@
+ *
+ * Send @buffer_list to the installed RTCP callback for @trans.
+ *

Since:

::: gst/rtsp-server/rtsp-stream.c
@@ +2474,3 @@
+ send_ret = gst_rtsp_stream_transport_send_rtp (tr, buffer);
+ if (buffer_list)
+ send_ret = gst_rtsp_stream_transport_send_rtp_list (tr, buffer_list);

this will ignore any potential error if the sample contained both a buffer and
a buffer list, not sure if that is allowed to happen in practice

@@ +3332,3 @@
/* make appsink */
priv->appsink[i] = gst_element_factory_make ("appsink", NULL);
+ g_object_set (priv->appsink[i], "emit-signals", FALSE, "buffer-list",

emit-signals is FALSE by default in appsink

::: gst/rtsp-sink/gstrtspclientsink.c
@@ +3853,3 @@
+static gboolean
+do_send_data_list (GstBufferList * buffer_list, guint8 channel,
+ GstRTSPStreamContext * context)

Same comment as the do_send_list in rtsp-client
--
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.
"GStreamer" (GNOME Bugzilla)
2018-10-11 16:39:46 UTC
Permalink
https://bugzilla.gnome.org/show_bug.cgi?id=771525

--- Comment #31 from Mathieu Duponchelle <***@gmail.com> ---
Review of attachment 373684:
--> (https://bugzilla.gnome.org/review?bug=771525&attachment=373684)

This will require updating sections.txt, lgtm apart from a few comments

::: gst/rtsp-server/rtsp-client.c
@@ +1187,3 @@

+static gboolean
+do_send_data_list (GstBufferList * buffer_list, guint8 channel,

The implementation seems similar to what rtsp-stream-transport will do if not
callback was provided, is this mostly a placeholder for future extension?

::: gst/rtsp-server/rtsp-media.c
@@ +2187,3 @@
g_object_set (appsrc, "block", TRUE, "format", GST_FORMAT_TIME, "is-live",
+ TRUE, "emit-signals", FALSE, NULL);
+ g_object_set (appsink, "sync", FALSE, "async", FALSE, "emit-signals",

emit-signals is false by default for appsink

::: gst/rtsp-server/rtsp-stream-transport.c
@@ +219,3 @@
+ *
+ * Install callbacks that will be called when data for a stream should be sent
+ * to a client. This is usually used when sending RTP/RTCP over TCP.

Since:

@@ -531,0 +566,7 @@
+/**
+ * gst_rtsp_stream_transport_send_rtp_list:
+ * @trans: a #GstRTSPStreamTransport
... 4 more ...

Since:

@@ +614,3 @@
+ *
+ * Send @buffer_list to the installed RTCP callback for @trans.
+ *

Since:

::: gst/rtsp-server/rtsp-stream.c
@@ +2474,3 @@
+ send_ret = gst_rtsp_stream_transport_send_rtp (tr, buffer);
+ if (buffer_list)
+ send_ret = gst_rtsp_stream_transport_send_rtp_list (tr, buffer_list);

this will ignore any potential error if the sample contained both a buffer and
a buffer list, not sure if that is allowed to happen in practice

@@ +3332,3 @@
/* make appsink */
priv->appsink[i] = gst_element_factory_make ("appsink", NULL);
+ g_object_set (priv->appsink[i], "emit-signals", FALSE, "buffer-list",

emit-signals is FALSE by default in appsink

::: gst/rtsp-sink/gstrtspclientsink.c
@@ +3853,3 @@
+static gboolean
+do_send_data_list (GstBufferList * buffer_list, guint8 channel,
+ GstRTSPStreamContext * context)

Same comment as the do_send_list in rtsp-client
--
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.
"GStreamer" (GNOME Bugzilla)
2018-10-11 16:40:09 UTC
Permalink
https://bugzilla.gnome.org/show_bug.cgi?id=771525

--- Comment #31 from Mathieu Duponchelle <***@gmail.com> ---
Review of attachment 373684:
--> (https://bugzilla.gnome.org/review?bug=771525&attachment=373684)

This will require updating sections.txt, lgtm apart from a few comments

::: gst/rtsp-server/rtsp-client.c
@@ +1187,3 @@

+static gboolean
+do_send_data_list (GstBufferList * buffer_list, guint8 channel,

The implementation seems similar to what rtsp-stream-transport will do if not
callback was provided, is this mostly a placeholder for future extension?

::: gst/rtsp-server/rtsp-media.c
@@ +2187,3 @@
g_object_set (appsrc, "block", TRUE, "format", GST_FORMAT_TIME, "is-live",
+ TRUE, "emit-signals", FALSE, NULL);
+ g_object_set (appsink, "sync", FALSE, "async", FALSE, "emit-signals",

emit-signals is false by default for appsink

::: gst/rtsp-server/rtsp-stream-transport.c
@@ +219,3 @@
+ *
+ * Install callbacks that will be called when data for a stream should be sent
+ * to a client. This is usually used when sending RTP/RTCP over TCP.

Since:

@@ -531,0 +566,7 @@
+/**
+ * gst_rtsp_stream_transport_send_rtp_list:
+ * @trans: a #GstRTSPStreamTransport
... 4 more ...

Since:

@@ +614,3 @@
+ *
+ * Send @buffer_list to the installed RTCP callback for @trans.
+ *

Since:

::: gst/rtsp-server/rtsp-stream.c
@@ +2474,3 @@
+ send_ret = gst_rtsp_stream_transport_send_rtp (tr, buffer);
+ if (buffer_list)
+ send_ret = gst_rtsp_stream_transport_send_rtp_list (tr, buffer_list);

this will ignore any potential error if the sample contained both a buffer and
a buffer list, not sure if that is allowed to happen in practice

@@ +3332,3 @@
/* make appsink */
priv->appsink[i] = gst_element_factory_make ("appsink", NULL);
+ g_object_set (priv->appsink[i], "emit-signals", FALSE, "buffer-list",

emit-signals is FALSE by default in appsink

::: gst/rtsp-sink/gstrtspclientsink.c
@@ +3853,3 @@
+static gboolean
+do_send_data_list (GstBufferList * buffer_list, guint8 channel,
+ GstRTSPStreamContext * context)

Same comment as the do_send_list in rtsp-client

--- Comment #32 from Mathieu Duponchelle <***@gmail.com> ---
Review of attachment 373684:
--> (https://bugzilla.gnome.org/review?bug=771525&attachment=373684)

This will require updating sections.txt, lgtm apart from a few comments

::: gst/rtsp-server/rtsp-client.c
@@ +1187,3 @@

+static gboolean
+do_send_data_list (GstBufferList * buffer_list, guint8 channel,

The implementation seems similar to what rtsp-stream-transport will do if not
callback was provided, is this mostly a placeholder for future extension?

::: gst/rtsp-server/rtsp-media.c
@@ +2187,3 @@
g_object_set (appsrc, "block", TRUE, "format", GST_FORMAT_TIME, "is-live",
+ TRUE, "emit-signals", FALSE, NULL);
+ g_object_set (appsink, "sync", FALSE, "async", FALSE, "emit-signals",

emit-signals is false by default for appsink

::: gst/rtsp-server/rtsp-stream-transport.c
@@ +219,3 @@
+ *
+ * Install callbacks that will be called when data for a stream should be sent
+ * to a client. This is usually used when sending RTP/RTCP over TCP.

Since:

@@ -531,0 +566,7 @@
+/**
+ * gst_rtsp_stream_transport_send_rtp_list:
+ * @trans: a #GstRTSPStreamTransport
... 4 more ...

Since:

@@ +614,3 @@
+ *
+ * Send @buffer_list to the installed RTCP callback for @trans.
+ *

Since:

::: gst/rtsp-server/rtsp-stream.c
@@ +2474,3 @@
+ send_ret = gst_rtsp_stream_transport_send_rtp (tr, buffer);
+ if (buffer_list)
+ send_ret = gst_rtsp_stream_transport_send_rtp_list (tr, buffer_list);

this will ignore any potential error if the sample contained both a buffer and
a buffer list, not sure if that is allowed to happen in practice

@@ +3332,3 @@
/* make appsink */
priv->appsink[i] = gst_element_factory_make ("appsink", NULL);
+ g_object_set (priv->appsink[i], "emit-signals", FALSE, "buffer-list",

emit-signals is FALSE by default in appsink

::: gst/rtsp-sink/gstrtspclientsink.c
@@ +3853,3 @@
+static gboolean
+do_send_data_list (GstBufferList * buffer_list, guint8 channel,
+ GstRTSPStreamContext * context)

Same comment as the do_send_list in rtsp-client

--- Comment #33 from Mathieu Duponchelle <***@gmail.com> ---
Review of attachment 373684:
--> (https://bugzilla.gnome.org/review?bug=771525&attachment=373684)

This will require updating sections.txt, lgtm apart from a few comments

::: gst/rtsp-server/rtsp-client.c
@@ +1187,3 @@

+static gboolean
+do_send_data_list (GstBufferList * buffer_list, guint8 channel,

The implementation seems similar to what rtsp-stream-transport will do if not
callback was provided, is this mostly a placeholder for future extension?

::: gst/rtsp-server/rtsp-media.c
@@ +2187,3 @@
g_object_set (appsrc, "block", TRUE, "format", GST_FORMAT_TIME, "is-live",
+ TRUE, "emit-signals", FALSE, NULL);
+ g_object_set (appsink, "sync", FALSE, "async", FALSE, "emit-signals",

emit-signals is false by default for appsink

::: gst/rtsp-server/rtsp-stream-transport.c
@@ +219,3 @@
+ *
+ * Install callbacks that will be called when data for a stream should be sent
+ * to a client. This is usually used when sending RTP/RTCP over TCP.

Since:

@@ -531,0 +566,7 @@
+/**
+ * gst_rtsp_stream_transport_send_rtp_list:
+ * @trans: a #GstRTSPStreamTransport
... 4 more ...

Since:

@@ +614,3 @@
+ *
+ * Send @buffer_list to the installed RTCP callback for @trans.
+ *

Since:

::: gst/rtsp-server/rtsp-stream.c
@@ +2474,3 @@
+ send_ret = gst_rtsp_stream_transport_send_rtp (tr, buffer);
+ if (buffer_list)
+ send_ret = gst_rtsp_stream_transport_send_rtp_list (tr, buffer_list);

this will ignore any potential error if the sample contained both a buffer and
a buffer list, not sure if that is allowed to happen in practice

@@ +3332,3 @@
/* make appsink */
priv->appsink[i] = gst_element_factory_make ("appsink", NULL);
+ g_object_set (priv->appsink[i], "emit-signals", FALSE, "buffer-list",

emit-signals is FALSE by default in appsink

::: gst/rtsp-sink/gstrtspclientsink.c
@@ +3853,3 @@
+static gboolean
+do_send_data_list (GstBufferList * buffer_list, guint8 channel,
+ GstRTSPStreamContext * context)

Same comment as the do_send_list in rtsp-client
--
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.
"GStreamer" (GNOME Bugzilla)
2018-10-11 16:42:06 UTC
Permalink
https://bugzilla.gnome.org/show_bug.cgi?id=771525

Mathieu Duponchelle <***@gmail.com> changed:

What |Removed |Added
----------------------------------------------------------------------------
Attachment #373684|reviewed |needs-work
status| |
Attachment #373684|reviewed |needs-work
status| |
Attachment #373684|reviewed reviewed reviewed |needs-work needs-work
status| |needs-work
CC| |***@gmail.com

--- Comment #31 from Mathieu Duponchelle <***@gmail.com> ---
Review of attachment 373684:
--> (https://bugzilla.gnome.org/review?bug=771525&attachment=373684)

This will require updating sections.txt, lgtm apart from a few comments

::: gst/rtsp-server/rtsp-client.c
@@ +1187,3 @@

+static gboolean
+do_send_data_list (GstBufferList * buffer_list, guint8 channel,

The implementation seems similar to what rtsp-stream-transport will do if not
callback was provided, is this mostly a placeholder for future extension?

::: gst/rtsp-server/rtsp-media.c
@@ +2187,3 @@
g_object_set (appsrc, "block", TRUE, "format", GST_FORMAT_TIME, "is-live",
+ TRUE, "emit-signals", FALSE, NULL);
+ g_object_set (appsink, "sync", FALSE, "async", FALSE, "emit-signals",

emit-signals is false by default for appsink

::: gst/rtsp-server/rtsp-stream-transport.c
@@ +219,3 @@
+ *
+ * Install callbacks that will be called when data for a stream should be sent
+ * to a client. This is usually used when sending RTP/RTCP over TCP.

Since:

@@ -531,0 +566,7 @@
+/**
+ * gst_rtsp_stream_transport_send_rtp_list:
+ * @trans: a #GstRTSPStreamTransport
... 4 more ...

Since:

@@ +614,3 @@
+ *
+ * Send @buffer_list to the installed RTCP callback for @trans.
+ *

Since:

::: gst/rtsp-server/rtsp-stream.c
@@ +2474,3 @@
+ send_ret = gst_rtsp_stream_transport_send_rtp (tr, buffer);
+ if (buffer_list)
+ send_ret = gst_rtsp_stream_transport_send_rtp_list (tr, buffer_list);

this will ignore any potential error if the sample contained both a buffer and
a buffer list, not sure if that is allowed to happen in practice

@@ +3332,3 @@
/* make appsink */
priv->appsink[i] = gst_element_factory_make ("appsink", NULL);
+ g_object_set (priv->appsink[i], "emit-signals", FALSE, "buffer-list",

emit-signals is FALSE by default in appsink

::: gst/rtsp-sink/gstrtspclientsink.c
@@ +3853,3 @@
+static gboolean
+do_send_data_list (GstBufferList * buffer_list, guint8 channel,
+ GstRTSPStreamContext * context)

Same comment as the do_send_list in rtsp-client

--- Comment #32 from Mathieu Duponchelle <***@gmail.com> ---
Review of attachment 373684:
--> (https://bugzilla.gnome.org/review?bug=771525&attachment=373684)

This will require updating sections.txt, lgtm apart from a few comments

::: gst/rtsp-server/rtsp-client.c
@@ +1187,3 @@

+static gboolean
+do_send_data_list (GstBufferList * buffer_list, guint8 channel,

The implementation seems similar to what rtsp-stream-transport will do if not
callback was provided, is this mostly a placeholder for future extension?

::: gst/rtsp-server/rtsp-media.c
@@ +2187,3 @@
g_object_set (appsrc, "block", TRUE, "format", GST_FORMAT_TIME, "is-live",
+ TRUE, "emit-signals", FALSE, NULL);
+ g_object_set (appsink, "sync", FALSE, "async", FALSE, "emit-signals",

emit-signals is false by default for appsink

::: gst/rtsp-server/rtsp-stream-transport.c
@@ +219,3 @@
+ *
+ * Install callbacks that will be called when data for a stream should be sent
+ * to a client. This is usually used when sending RTP/RTCP over TCP.

Since:

@@ -531,0 +566,7 @@
+/**
+ * gst_rtsp_stream_transport_send_rtp_list:
+ * @trans: a #GstRTSPStreamTransport
... 4 more ...

Since:

@@ +614,3 @@
+ *
+ * Send @buffer_list to the installed RTCP callback for @trans.
+ *

Since:

::: gst/rtsp-server/rtsp-stream.c
@@ +2474,3 @@
+ send_ret = gst_rtsp_stream_transport_send_rtp (tr, buffer);
+ if (buffer_list)
+ send_ret = gst_rtsp_stream_transport_send_rtp_list (tr, buffer_list);

this will ignore any potential error if the sample contained both a buffer and
a buffer list, not sure if that is allowed to happen in practice

@@ +3332,3 @@
/* make appsink */
priv->appsink[i] = gst_element_factory_make ("appsink", NULL);
+ g_object_set (priv->appsink[i], "emit-signals", FALSE, "buffer-list",

emit-signals is FALSE by default in appsink

::: gst/rtsp-sink/gstrtspclientsink.c
@@ +3853,3 @@
+static gboolean
+do_send_data_list (GstBufferList * buffer_list, guint8 channel,
+ GstRTSPStreamContext * context)

Same comment as the do_send_list in rtsp-client

--- Comment #33 from Mathieu Duponchelle <***@gmail.com> ---
Review of attachment 373684:
--> (https://bugzilla.gnome.org/review?bug=771525&attachment=373684)

This will require updating sections.txt, lgtm apart from a few comments

::: gst/rtsp-server/rtsp-client.c
@@ +1187,3 @@

+static gboolean
+do_send_data_list (GstBufferList * buffer_list, guint8 channel,

The implementation seems similar to what rtsp-stream-transport will do if not
callback was provided, is this mostly a placeholder for future extension?

::: gst/rtsp-server/rtsp-media.c
@@ +2187,3 @@
g_object_set (appsrc, "block", TRUE, "format", GST_FORMAT_TIME, "is-live",
+ TRUE, "emit-signals", FALSE, NULL);
+ g_object_set (appsink, "sync", FALSE, "async", FALSE, "emit-signals",

emit-signals is false by default for appsink

::: gst/rtsp-server/rtsp-stream-transport.c
@@ +219,3 @@
+ *
+ * Install callbacks that will be called when data for a stream should be sent
+ * to a client. This is usually used when sending RTP/RTCP over TCP.

Since:

@@ -531,0 +566,7 @@
+/**
+ * gst_rtsp_stream_transport_send_rtp_list:
+ * @trans: a #GstRTSPStreamTransport
... 4 more ...

Since:

@@ +614,3 @@
+ *
+ * Send @buffer_list to the installed RTCP callback for @trans.
+ *

Since:

::: gst/rtsp-server/rtsp-stream.c
@@ +2474,3 @@
+ send_ret = gst_rtsp_stream_transport_send_rtp (tr, buffer);
+ if (buffer_list)
+ send_ret = gst_rtsp_stream_transport_send_rtp_list (tr, buffer_list);

this will ignore any potential error if the sample contained both a buffer and
a buffer list, not sure if that is allowed to happen in practice

@@ +3332,3 @@
/* make appsink */
priv->appsink[i] = gst_element_factory_make ("appsink", NULL);
+ g_object_set (priv->appsink[i], "emit-signals", FALSE, "buffer-list",

emit-signals is FALSE by default in appsink

::: gst/rtsp-sink/gstrtspclientsink.c
@@ +3853,3 @@
+static gboolean
+do_send_data_list (GstBufferList * buffer_list, guint8 channel,
+ GstRTSPStreamContext * context)

Same comment as the do_send_list in rtsp-client

--- Comment #34 from Mathieu Duponchelle <***@gmail.com> ---
Review of attachment 373684:
--> (https://bugzilla.gnome.org/review?bug=771525&attachment=373684)

This will require updating sections.txt, lgtm apart from a few comments

::: gst/rtsp-server/rtsp-client.c
@@ +1187,3 @@

+static gboolean
+do_send_data_list (GstBufferList * buffer_list, guint8 channel,

The implementation seems similar to what rtsp-stream-transport will do if not
callback was provided, is this mostly a placeholder for future extension?

::: gst/rtsp-server/rtsp-media.c
@@ +2187,3 @@
g_object_set (appsrc, "block", TRUE, "format", GST_FORMAT_TIME, "is-live",
+ TRUE, "emit-signals", FALSE, NULL);
+ g_object_set (appsink, "sync", FALSE, "async", FALSE, "emit-signals",

emit-signals is false by default for appsink

::: gst/rtsp-server/rtsp-stream-transport.c
@@ +219,3 @@
+ *
+ * Install callbacks that will be called when data for a stream should be sent
+ * to a client. This is usually used when sending RTP/RTCP over TCP.

Since:

@@ -531,0 +566,7 @@
+/**
+ * gst_rtsp_stream_transport_send_rtp_list:
+ * @trans: a #GstRTSPStreamTransport
... 4 more ...

Since:

@@ +614,3 @@
+ *
+ * Send @buffer_list to the installed RTCP callback for @trans.
+ *

Since:

::: gst/rtsp-server/rtsp-stream.c
@@ +2474,3 @@
+ send_ret = gst_rtsp_stream_transport_send_rtp (tr, buffer);
+ if (buffer_list)
+ send_ret = gst_rtsp_stream_transport_send_rtp_list (tr, buffer_list);

this will ignore any potential error if the sample contained both a buffer and
a buffer list, not sure if that is allowed to happen in practice

@@ +3332,3 @@
/* make appsink */
priv->appsink[i] = gst_element_factory_make ("appsink", NULL);
+ g_object_set (priv->appsink[i], "emit-signals", FALSE, "buffer-list",

emit-signals is FALSE by default in appsink

::: gst/rtsp-sink/gstrtspclientsink.c
@@ +3853,3 @@
+static gboolean
+do_send_data_list (GstBufferList * buffer_list, guint8 channel,
+ GstRTSPStreamContext * context)

Same comment as the do_send_list in rtsp-client

--- Comment #35 from Mathieu Duponchelle <***@gmail.com> ---
Review of attachment 373684:
--> (https://bugzilla.gnome.org/review?bug=771525&attachment=373684)

This will require updating sections.txt, lgtm apart from a few comments

::: gst/rtsp-server/rtsp-client.c
@@ +1187,3 @@

+static gboolean
+do_send_data_list (GstBufferList * buffer_list, guint8 channel,

The implementation seems similar to what rtsp-stream-transport will do if not
callback was provided, is this mostly a placeholder for future extension?

::: gst/rtsp-server/rtsp-media.c
@@ +2187,3 @@
g_object_set (appsrc, "block", TRUE, "format", GST_FORMAT_TIME, "is-live",
+ TRUE, "emit-signals", FALSE, NULL);
+ g_object_set (appsink, "sync", FALSE, "async", FALSE, "emit-signals",

emit-signals is false by default for appsink

::: gst/rtsp-server/rtsp-stream-transport.c
@@ +219,3 @@
+ *
+ * Install callbacks that will be called when data for a stream should be sent
+ * to a client. This is usually used when sending RTP/RTCP over TCP.

Since:

@@ -531,0 +566,7 @@
+/**
+ * gst_rtsp_stream_transport_send_rtp_list:
+ * @trans: a #GstRTSPStreamTransport
... 4 more ...

Since:

@@ +614,3 @@
+ *
+ * Send @buffer_list to the installed RTCP callback for @trans.
+ *

Since:

::: gst/rtsp-server/rtsp-stream.c
@@ +2474,3 @@
+ send_ret = gst_rtsp_stream_transport_send_rtp (tr, buffer);
+ if (buffer_list)
+ send_ret = gst_rtsp_stream_transport_send_rtp_list (tr, buffer_list);

this will ignore any potential error if the sample contained both a buffer and
a buffer list, not sure if that is allowed to happen in practice

@@ +3332,3 @@
/* make appsink */
priv->appsink[i] = gst_element_factory_make ("appsink", NULL);
+ g_object_set (priv->appsink[i], "emit-signals", FALSE, "buffer-list",

emit-signals is FALSE by default in appsink

::: gst/rtsp-sink/gstrtspclientsink.c
@@ +3853,3 @@
+static gboolean
+do_send_data_list (GstBufferList * buffer_list, guint8 channel,
+ GstRTSPStreamContext * context)

Same comment as the do_send_list in rtsp-client

--- Comment #36 from Mathieu Duponchelle <***@gmail.com> ---
Review of attachment 373684:
--> (https://bugzilla.gnome.org/review?bug=771525&attachment=373684)

This will require updating sections.txt, lgtm apart from a few comments

::: gst/rtsp-server/rtsp-client.c
@@ +1187,3 @@

+static gboolean
+do_send_data_list (GstBufferList * buffer_list, guint8 channel,

The implementation seems similar to what rtsp-stream-transport will do if not
callback was provided, is this mostly a placeholder for future extension?

::: gst/rtsp-server/rtsp-media.c
@@ +2187,3 @@
g_object_set (appsrc, "block", TRUE, "format", GST_FORMAT_TIME, "is-live",
+ TRUE, "emit-signals", FALSE, NULL);
+ g_object_set (appsink, "sync", FALSE, "async", FALSE, "emit-signals",

emit-signals is false by default for appsink

::: gst/rtsp-server/rtsp-stream-transport.c
@@ +219,3 @@
+ *
+ * Install callbacks that will be called when data for a stream should be sent
+ * to a client. This is usually used when sending RTP/RTCP over TCP.

Since:

@@ -531,0 +566,7 @@
+/**
+ * gst_rtsp_stream_transport_send_rtp_list:
+ * @trans: a #GstRTSPStreamTransport
... 4 more ...

Since:

@@ +614,3 @@
+ *
+ * Send @buffer_list to the installed RTCP callback for @trans.
+ *

Since:

::: gst/rtsp-server/rtsp-stream.c
@@ +2474,3 @@
+ send_ret = gst_rtsp_stream_transport_send_rtp (tr, buffer);
+ if (buffer_list)
+ send_ret = gst_rtsp_stream_transport_send_rtp_list (tr, buffer_list);

this will ignore any potential error if the sample contained both a buffer and
a buffer list, not sure if that is allowed to happen in practice

@@ +3332,3 @@
/* make appsink */
priv->appsink[i] = gst_element_factory_make ("appsink", NULL);
+ g_object_set (priv->appsink[i], "emit-signals", FALSE, "buffer-list",

emit-signals is FALSE by default in appsink

::: gst/rtsp-sink/gstrtspclientsink.c
@@ +3853,3 @@
+static gboolean
+do_send_data_list (GstBufferList * buffer_list, guint8 channel,
+ GstRTSPStreamContext * context)

Same comment as the do_send_list in rtsp-client

--- Comment #37 from Mathieu Duponchelle <***@gmail.com> ---
Review of attachment 373684:
--> (https://bugzilla.gnome.org/review?bug=771525&attachment=373684)

This will require updating sections.txt, lgtm apart from a few comments

::: gst/rtsp-server/rtsp-client.c
@@ +1187,3 @@

+static gboolean
+do_send_data_list (GstBufferList * buffer_list, guint8 channel,

The implementation seems similar to what rtsp-stream-transport will do if not
callback was provided, is this mostly a placeholder for future extension?

::: gst/rtsp-server/rtsp-media.c
@@ +2187,3 @@
g_object_set (appsrc, "block", TRUE, "format", GST_FORMAT_TIME, "is-live",
+ TRUE, "emit-signals", FALSE, NULL);
+ g_object_set (appsink, "sync", FALSE, "async", FALSE, "emit-signals",

emit-signals is false by default for appsink

::: gst/rtsp-server/rtsp-stream-transport.c
@@ +219,3 @@
+ *
+ * Install callbacks that will be called when data for a stream should be sent
+ * to a client. This is usually used when sending RTP/RTCP over TCP.

Since:

@@ -531,0 +566,7 @@
+/**
+ * gst_rtsp_stream_transport_send_rtp_list:
+ * @trans: a #GstRTSPStreamTransport
... 4 more ...

Since:

@@ +614,3 @@
+ *
+ * Send @buffer_list to the installed RTCP callback for @trans.
+ *

Since:

::: gst/rtsp-server/rtsp-stream.c
@@ +2474,3 @@
+ send_ret = gst_rtsp_stream_transport_send_rtp (tr, buffer);
+ if (buffer_list)
+ send_ret = gst_rtsp_stream_transport_send_rtp_list (tr, buffer_list);

this will ignore any potential error if the sample contained both a buffer and
a buffer list, not sure if that is allowed to happen in practice

@@ +3332,3 @@
/* make appsink */
priv->appsink[i] = gst_element_factory_make ("appsink", NULL);
+ g_object_set (priv->appsink[i], "emit-signals", FALSE, "buffer-list",

emit-signals is FALSE by default in appsink

::: gst/rtsp-sink/gstrtspclientsink.c
@@ +3853,3 @@
+static gboolean
+do_send_data_list (GstBufferList * buffer_list, guint8 channel,
+ GstRTSPStreamContext * context)

Same comment as the do_send_list in rtsp-client

--- Comment #38 from Mathieu Duponchelle <***@gmail.com> ---
Review of attachment 373684:
--> (https://bugzilla.gnome.org/review?bug=771525&attachment=373684)

This will require updating sections.txt, lgtm apart from a few comments

::: gst/rtsp-server/rtsp-client.c
@@ +1187,3 @@

+static gboolean
+do_send_data_list (GstBufferList * buffer_list, guint8 channel,

The implementation seems similar to what rtsp-stream-transport will do if not
callback was provided, is this mostly a placeholder for future extension?

::: gst/rtsp-server/rtsp-media.c
@@ +2187,3 @@
g_object_set (appsrc, "block", TRUE, "format", GST_FORMAT_TIME, "is-live",
+ TRUE, "emit-signals", FALSE, NULL);
+ g_object_set (appsink, "sync", FALSE, "async", FALSE, "emit-signals",

emit-signals is false by default for appsink

::: gst/rtsp-server/rtsp-stream-transport.c
@@ +219,3 @@
+ *
+ * Install callbacks that will be called when data for a stream should be sent
+ * to a client. This is usually used when sending RTP/RTCP over TCP.

Since:

@@ -531,0 +566,7 @@
+/**
+ * gst_rtsp_stream_transport_send_rtp_list:
+ * @trans: a #GstRTSPStreamTransport
... 4 more ...

Since:

@@ +614,3 @@
+ *
+ * Send @buffer_list to the installed RTCP callback for @trans.
+ *

Since:

::: gst/rtsp-server/rtsp-stream.c
@@ +2474,3 @@
+ send_ret = gst_rtsp_stream_transport_send_rtp (tr, buffer);
+ if (buffer_list)
+ send_ret = gst_rtsp_stream_transport_send_rtp_list (tr, buffer_list);

this will ignore any potential error if the sample contained both a buffer and
a buffer list, not sure if that is allowed to happen in practice

@@ +3332,3 @@
/* make appsink */
priv->appsink[i] = gst_element_factory_make ("appsink", NULL);
+ g_object_set (priv->appsink[i], "emit-signals", FALSE, "buffer-list",

emit-signals is FALSE by default in appsink

::: gst/rtsp-sink/gstrtspclientsink.c
@@ +3853,3 @@
+static gboolean
+do_send_data_list (GstBufferList * buffer_list, guint8 channel,
+ GstRTSPStreamContext * context)

Same comment as the do_send_list in rtsp-client

--- Comment #39 from Mathieu Duponchelle <***@gmail.com> ---
Hrmph, very sorry for the spam, bugzilla was unresponsive on publish and I
clicked quite a few times :(
--
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.
"GStreamer" (GNOME Bugzilla)
2018-10-12 08:49:03 UTC
Permalink
https://bugzilla.gnome.org/show_bug.cgi?id=771525

Sebastian Dröge (slomo) <***@coaxion.net> changed:

What |Removed |Added
----------------------------------------------------------------------------
Attachment #373685|reviewed |none
status| |
Attachment #373685|0 |1
is obsolete| |

--- Comment #41 from Sebastian Dröge (slomo) <***@coaxion.net> ---
Created attachment 373908
--> https://bugzilla.gnome.org/attachment.cgi?id=373908&action=edit
rtsp-client: Add support for sending buffer lists directly
--
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.
"GStreamer" (GNOME Bugzilla)
2018-10-12 08:48:56 UTC
Permalink
https://bugzilla.gnome.org/show_bug.cgi?id=771525

Sebastian Dröge (slomo) <***@coaxion.net> changed:

What |Removed |Added
----------------------------------------------------------------------------
Attachment #373684|needs-work |none
status| |
Attachment #373684|0 |1
is obsolete| |

--- Comment #40 from Sebastian Dröge (slomo) <***@coaxion.net> ---
Created attachment 373907
--> https://bugzilla.gnome.org/attachment.cgi?id=373907&action=edit
rtsp-server: Add support for buffer lists

This adds new functions for passing buffer lists through the different
layers without breaking API/ABI, and enables the appsink to actually
provide buffer lists.

This should already reduce CPU usage and potentially context switches a
bit by passing a whole buffer list from the appsink instead of
individual buffers. As a next step it would be necessary to
a) Add support for a vector of data for the GstRTSPMessage body
b) Add support for sending multiple messages at once to the
GstRTSPWatch and let it be handled internally
c) Adding API to GOutputStream that works like writev()
--
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.
"GStreamer" (GNOME Bugzilla)
2018-11-03 11:58:37 UTC
Permalink
https://bugzilla.gnome.org/show_bug.cgi?id=771525
Bug 771525 depends on bug 785684, which changed state.

Bug 785684 Summary: rtspconnection: Add API for sending multiple messages at once, and for having the message body consist of multiple chunks of memory
https://bugzilla.gnome.org/show_bug.cgi?id=785684

What |Removed |Added
----------------------------------------------------------------------------
Status|NEW |RESOLVED
Resolution|--- |OBSOLETE
--
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.
"GStreamer" (GNOME Bugzilla)
2018-11-03 15:40:50 UTC
Permalink
https://bugzilla.gnome.org/show_bug.cgi?id=771525

GStreamer system administrator <***@gstreamer.net> changed:

What |Removed |Added
----------------------------------------------------------------------------
Status|NEW |RESOLVED
Resolution|--- |OBSOLETE

--- Comment #42 from GStreamer system administrator <***@gstreamer.net> ---
-- 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/29.
--
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.
Loading...