After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 720219 - rtsptransport: allow getting mime type by profile
rtsptransport: allow getting mime type by profile
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-base
git master
Other Linux
: Normal normal
: 1.3.1
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2013-12-10 23:12 UTC by Aleix Conchillo Flaqué
Modified: 2014-01-07 19:37 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
allow using profile to get mime type (7.17 KB, patch)
2013-12-10 23:20 UTC, Aleix Conchillo Flaqué
needs-work Details | Review

Description Aleix Conchillo Flaqué 2013-12-10 23:12:54 UTC
RTSP tranport API allows getting the RTP manager and mime type by only the the transport mode. For SRTP (RTP + SAVP profile) the mime type is different (and may be the manager could be different).

Initially, I thought that the only way would be breaking ABI by changing gst_rtsp_transport_get_mime signature for:

 gst_rtsp_transport_get_mime     (GstRTSPTransMode trans,
                                  GstRTSPProfile profile,
                                  const gchar **mime)

And the same for gst_rtsp_transport_get_manager.

However, as it seems SRTP will use rtpbin at the end (see bug 719938) this might not be necessary and we could keep ABI and API compatibility.
Comment 1 Aleix Conchillo Flaqué 2013-12-10 23:20:27 UTC
Created attachment 263951 [details] [review]
allow using profile to get mime type

In this patch we add

  gst_rtsp_transport_get_profile_mime

that allows specifying a transport and a profile.

I think that the ideal case would be to modify by adding a profile argument

  gst_rtsp_transport_get_mime
  gst_rtsp_transport_get_manager

but this would break everything.
Comment 2 Sebastian Dröge (slomo) 2013-12-17 09:44:56 UTC
Review of attachment 263951 [details] [review]:

Is this still necessary with the current plans for SRTP?

::: gst-libs/gst/rtsp/gstrtsptransport.c
@@ +92,3 @@
+      "application/x-rdt", {"rdtmanager", NULL}},
+  {"srtp", GST_RTSP_TRANS_RTP, GST_RTSP_PROFILE_SAVP, "application/x-rtp",
+      "application/x-srtp", {"rtpbin", NULL}},

One time x-rtp, one time x-srtp? I think the difference between mime and profile_mime should be made clearer in the documentation of the two functions.
Comment 3 Aleix Conchillo Flaqué 2013-12-17 18:30:48 UTC
(In reply to comment #2)
> Review of attachment 263951 [details] [review]:
> 
> Is this still necessary with the current plans for SRTP?
> 
> ::: gst-libs/gst/rtsp/gstrtsptransport.c
> @@ +92,3 @@
> +      "application/x-rdt", {"rdtmanager", NULL}},
> +  {"srtp", GST_RTSP_TRANS_RTP, GST_RTSP_PROFILE_SAVP, "application/x-rtp",
> +      "application/x-srtp", {"rtpbin", NULL}},
> 
> One time x-rtp, one time x-srtp? I think the difference between mime and
> profile_mime should be made clearer in the documentation of the two functions.

I made the distinction so, for example, in rtspsrc you have incoming packets from the udpsrc and you need to use the "application/x-srtp" as the caps which would be the profile mime type. But the output of rtspsrc will have the caps of the transport "application/x-rtp".

I couldn't find another easy way to do this.
Comment 4 Aleix Conchillo Flaqué 2013-12-18 23:39:21 UTC
(In reply to comment #2)

> One time x-rtp, one time x-srtp? I think the difference between mime and
> profile_mime should be made clearer in the documentation of the two functions.

It is also true that the documentation would need some update to help understand the difference between the two. I guess the first thing would be to know if this makes sense or not.

So far, I have used this change as well as the patch for bug 719938 to add SRTP support in rtspsrc.
Comment 5 Wim Taymans 2013-12-26 16:52:04 UTC
We should have had something like:

 application/x-rtp-<profile>

then we could handle the secure and feedback variants as well along with RDT with

 application/x-rdt
 application/x-rtp-avp
 application/x-rtp-savp
 application/x-rtp-avpf
 application/x-rtp-savpf

I'm not sure we really need to expose the feedback variants in the caps so adding 

 application/x-srtp

Could be enough for now.
Comment 6 Aleix Conchillo Flaqué 2013-12-31 07:55:55 UTC
(In reply to comment #5)
> We should have had something like:
> 
>  application/x-rtp-<profile>
> 
> then we could handle the secure and feedback variants as well along with RDT
> with
> 
>  application/x-rdt
>  application/x-rtp-avp
>  application/x-rtp-savp
>  application/x-rtp-avpf
>  application/x-rtp-savpf
> 
> I'm not sure we really need to expose the feedback variants in the caps so
> adding 
> 
>  application/x-srtp
> 
> Could be enough for now.

Not sure if I understand. If we do so, how we disintguish between rtp managers (both rtp and srtp using rtpbin)? This is what I did initially, and that's why I created srtpbin.
Comment 7 Aleix Conchillo Flaqué 2013-12-31 07:56:59 UTC
(In reply to comment #6)
> 
> Not sure if I understand. If we do so, how we disintguish between rtp managers
> (both rtp and srtp using rtpbin)? This is what I did initially, and that's why
> I created srtpbin.

With the current API, I mean.
Comment 8 Aleix Conchillo Flaqué 2013-12-31 07:58:19 UTC
(In reply to comment #6)
> 
> Not sure if I understand. If we do so, how we disintguish between rtp managers
> (both rtp and srtp using rtpbin)? This is what I did initially, and that's why
> I created srtpbin.

And also getting the right mime type.
Comment 9 Aleix Conchillo Flaqué 2014-01-02 15:28:31 UTC
Let me rephrase this: with the current API (that only depends on the transport mode) how would you get the "application/x-srtp" mime type?

Btw, happy new year!
Comment 10 Wim Taymans 2014-01-07 13:10:38 UTC
(In reply to comment #9)
> Let me rephrase this: with the current API (that only depends on the transport
> mode) how would you get the "application/x-srtp" mime type?

Why would you want to get that?

> 
> Btw, happy new year!

Happy new year!
Comment 11 Wim Taymans 2014-01-07 13:16:29 UTC
(In reply to comment #10)
> (In reply to comment #9)
> > Let me rephrase this: with the current API (that only depends on the transport
> > mode) how would you get the "application/x-srtp" mime type?
> 
> Why would you want to get that?

Ok, so rtspsrc uses it to generate the caps on the srcpad. Let's make a new method then that takes all relevant parameters and deprecate the old method.
Comment 12 Wim Taymans 2014-01-07 13:55:21 UTC
commit 594dd4287bd132ac250dba447abaf47930b84fc4
Author: Wim Taymans <wtaymans@redhat.com>
Date:   Tue Jan 7 14:46:05 2014 +0100

    rtsptransport: calculate default lower transport
    
    Add an internal method to calculate the default lower transport whan it
    is missing.

commit 124cf22d5d2ddc5b3c5d1a3e5495e12c193fa9a9
Author: Wim Taymans <wtaymans@redhat.com>
Date:   Tue Jan 7 14:31:09 2014 +0100

    rtsptransport: add method to get media-type from transport
    
    Add a method to make a media-type from the transport. Deprecate the old
    method that only used the mode.
    
    Based on patch from Aleix Conchillo Flaqué <aleix@oblong.com>
    
    Fixes https://bugzilla.gnome.org/show_bug.cgi?id=720219
Comment 13 Wim Taymans 2014-01-07 14:07:55 UTC
I add a new get_media_type and deprecated the old method.

We only need one media_type, the one that describes the stream and thus the one that goes into rtpbin. What comes out of rtpbin is not relevant to rtspsrc and it will simply forward those caps.
Comment 14 Aleix Conchillo Flaqué 2014-01-07 19:37:30 UTC
(In reply to comment #11)
> (In reply to comment #10)
> > (In reply to comment #9)
> > > Let me rephrase this: with the current API (that only depends on the transport
> > > mode) how would you get the "application/x-srtp" mime type?
> > 
> > Why would you want to get that?
> 
> Ok, so rtspsrc uses it to generate the caps on the srcpad. Let's make a new
> method then that takes all relevant parameters and deprecate the old method.

Exactly.

Deprecating the old method was the ideal solution. Thanks!