[2/2] rtsp: Expose the flag options via private AVOptions for sdp and rtp, too

Message ID 1318874895-89573-2-git-send-email-martin@martin.st
State Committed
Headers show

Commit Message

Martin Storsjö Oct. 17, 2011, 6:08 p.m.
This allows setting the filter_src option for these demuxers, too,
which wasn't possible at all before (where the option only was set
via URL parameters for RTSP).
---

Bikeshed: Should these flag fields be named sdp_flags/rtp_flags
or sdpflags/rtpflags?

The RTP muxer has "rtpflags" (which also is passed through from
the avoption with the same name in the mov and rtsp muxers, too),
but the RTSP demuxer and muxer have rtsp_flags.

The rtsp_flags flag field in the RTSP muxer/demuxer actually
is the same flag field as this one, but exposing it with a

Comments

Anton Khirnov Oct. 17, 2011, 6:26 p.m. | #1
On Mon, 17 Oct 2011 21:08:15 +0300, Martin Storsjö <martin@martin.st> wrote:
> This allows setting the filter_src option for these demuxers, too,
> which wasn't possible at all before (where the option only was set
> via URL parameters for RTSP).
> ---
> 
> Bikeshed: Should these flag fields be named sdp_flags/rtp_flags
> or sdpflags/rtpflags?
> 

I like sdp_flags, it's more readable IMO
/me paints his bikeshed black with red stripes

> The RTP muxer has "rtpflags" (which also is passed through from
> the avoption with the same name in the mov and rtsp muxers, too),
> but the RTSP demuxer and muxer have rtsp_flags.
> 
> The rtsp_flags flag field in the RTSP muxer/demuxer actually
> is the same flag field as this one, but exposing it with a
> different name for these (sdp, rtp) demuxers make sense IMO,
> since it doesn't matter to the user that these demuxers share
> implementation with the RTSP demuxer.
> 
> 
>  libavformat/rtsp.c |   31 +++++++++++++++++++++++++++++--
>  1 files changed, 29 insertions(+), 2 deletions(-)
> 
> diff --git a/libavformat/rtsp.c b/libavformat/rtsp.c
> index 4765d2f..a1d63c6 100644
> --- a/libavformat/rtsp.c
> +++ b/libavformat/rtsp.c
> @@ -77,6 +77,16 @@ const AVOption ff_rtsp_options[] = {
>      { NULL },
>  };
>  
> +static const AVOption sdp_options[] = {
> +    RTSP_FLAG_OPTS("sdp_flags", "SDP flags"),
> +    { NULL },
> +};
> +
> +static const AVOption rtp_options[] = {
> +    RTSP_FLAG_OPTS("rtp_flags", "RTP flags"),
> +    { NULL },
> +};
> +
>  static void get_word_until_chars(char *buf, int buf_size,
>                                   const char *sep, const char **pp)
>  {
> @@ -1835,8 +1845,9 @@ static int sdp_read_header(AVFormatContext *s, AVFormatParameters *ap)
>                      namebuf, sizeof(namebuf), NULL, 0, NI_NUMERICHOST);
>          ff_url_join(url, sizeof(url), "rtp", NULL,
>                      namebuf, rtsp_st->sdp_port,
> -                    "?localport=%d&ttl=%d", rtsp_st->sdp_port,
> -                    rtsp_st->sdp_ttl);
> +                    "?localport=%d&ttl=%d&connect=%d", rtsp_st->sdp_port,
> +                    rtsp_st->sdp_ttl,
> +                    rt->rtsp_flags & RTSP_FLAG_FILTER_SRC ? 1 : 0);
>          if (ffurl_open(&rtsp_st->rtp_handle, url, AVIO_FLAG_READ_WRITE) < 0) {
>              err = AVERROR_INVALIDDATA;
>              goto fail;
> @@ -1858,6 +1869,13 @@ static int sdp_read_close(AVFormatContext *s)
>      return 0;
>  }
>  
> +const AVClass sdp_demuxer_class = {
> +    .class_name     = "SDP demuxer",
> +    .item_name      = av_default_item_name,
> +    .option         = sdp_options,
> +    .version        = LIBAVUTIL_VERSION_INT,
> +};
> +

Should be static? same below
Martin Storsjö Oct. 17, 2011, 6:38 p.m. | #2
On Mon, 17 Oct 2011, Anton Khirnov wrote:

>
> On Mon, 17 Oct 2011 21:08:15 +0300, Martin Storsjö <martin@martin.st> wrote:
>> This allows setting the filter_src option for these demuxers, too,
>> which wasn't possible at all before (where the option only was set
>> via URL parameters for RTSP).
>> ---
>>
>> Bikeshed: Should these flag fields be named sdp_flags/rtp_flags
>> or sdpflags/rtpflags?
>>
>
> I like sdp_flags, it's more readable IMO
> /me paints his bikeshed black with red stripes

Ok, good, we agree on that. :-)

>> The RTP muxer has "rtpflags" (which also is passed through from
>> the avoption with the same name in the mov and rtsp muxers, too),
>> but the RTSP demuxer and muxer have rtsp_flags.
>>
>> The rtsp_flags flag field in the RTSP muxer/demuxer actually
>> is the same flag field as this one, but exposing it with a
>> different name for these (sdp, rtp) demuxers make sense IMO,
>> since it doesn't matter to the user that these demuxers share
>> implementation with the RTSP demuxer.
>>
>>
>>  libavformat/rtsp.c |   31 +++++++++++++++++++++++++++++--
>>  1 files changed, 29 insertions(+), 2 deletions(-)
>>
>> diff --git a/libavformat/rtsp.c b/libavformat/rtsp.c
>> index 4765d2f..a1d63c6 100644
>> --- a/libavformat/rtsp.c
>> +++ b/libavformat/rtsp.c
>> @@ -77,6 +77,16 @@ const AVOption ff_rtsp_options[] = {
>>      { NULL },
>>  };
>>
>> +static const AVOption sdp_options[] = {
>> +    RTSP_FLAG_OPTS("sdp_flags", "SDP flags"),
>> +    { NULL },
>> +};
>> +
>> +static const AVOption rtp_options[] = {
>> +    RTSP_FLAG_OPTS("rtp_flags", "RTP flags"),
>> +    { NULL },
>> +};
>> +
>>  static void get_word_until_chars(char *buf, int buf_size,
>>                                   const char *sep, const char **pp)
>>  {
>> @@ -1835,8 +1845,9 @@ static int sdp_read_header(AVFormatContext *s, AVFormatParameters *ap)
>>                      namebuf, sizeof(namebuf), NULL, 0, NI_NUMERICHOST);
>>          ff_url_join(url, sizeof(url), "rtp", NULL,
>>                      namebuf, rtsp_st->sdp_port,
>> -                    "?localport=%d&ttl=%d", rtsp_st->sdp_port,
>> -                    rtsp_st->sdp_ttl);
>> +                    "?localport=%d&ttl=%d&connect=%d", rtsp_st->sdp_port,
>> +                    rtsp_st->sdp_ttl,
>> +                    rt->rtsp_flags & RTSP_FLAG_FILTER_SRC ? 1 : 0);
>>          if (ffurl_open(&rtsp_st->rtp_handle, url, AVIO_FLAG_READ_WRITE) < 0) {
>>              err = AVERROR_INVALIDDATA;
>>              goto fail;
>> @@ -1858,6 +1869,13 @@ static int sdp_read_close(AVFormatContext *s)
>>      return 0;
>>  }
>>
>> +const AVClass sdp_demuxer_class = {
>> +    .class_name     = "SDP demuxer",
>> +    .item_name      = av_default_item_name,
>> +    .option         = sdp_options,
>> +    .version        = LIBAVUTIL_VERSION_INT,
>> +};
>> +
>
> Should be static? same below

Indeed, fixed locally.

// Martin
Anton Khirnov Oct. 17, 2011, 6:59 p.m. | #3
On Mon, 17 Oct 2011 21:38:27 +0300 (EEST), Martin Storsjö <martin@martin.st> wrote:
Non-text part: MULTIPART/MIXED
> On Mon, 17 Oct 2011, Anton Khirnov wrote:
> 
> >
> > On Mon, 17 Oct 2011 21:08:15 +0300, Martin Storsjö <martin@martin.st> wrote:
> >> This allows setting the filter_src option for these demuxers, too,
> >> which wasn't possible at all before (where the option only was set
> >> via URL parameters for RTSP).
> >> ---
> >>
> >> Bikeshed: Should these flag fields be named sdp_flags/rtp_flags
> >> or sdpflags/rtpflags?
> >>
> >
> > I like sdp_flags, it's more readable IMO
> > /me paints his bikeshed black with red stripes
> 
> Ok, good, we agree on that. :-)
> 
> >> The RTP muxer has "rtpflags" (which also is passed through from
> >> the avoption with the same name in the mov and rtsp muxers, too),
> >> but the RTSP demuxer and muxer have rtsp_flags.
> >>
> >> The rtsp_flags flag field in the RTSP muxer/demuxer actually
> >> is the same flag field as this one, but exposing it with a
> >> different name for these (sdp, rtp) demuxers make sense IMO,
> >> since it doesn't matter to the user that these demuxers share
> >> implementation with the RTSP demuxer.
> >>
> >>
> >>  libavformat/rtsp.c |   31 +++++++++++++++++++++++++++++--
> >>  1 files changed, 29 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/libavformat/rtsp.c b/libavformat/rtsp.c
> >> index 4765d2f..a1d63c6 100644
> >> --- a/libavformat/rtsp.c
> >> +++ b/libavformat/rtsp.c
> >> @@ -77,6 +77,16 @@ const AVOption ff_rtsp_options[] = {
> >>      { NULL },
> >>  };
> >>
> >> +static const AVOption sdp_options[] = {
> >> +    RTSP_FLAG_OPTS("sdp_flags", "SDP flags"),
> >> +    { NULL },
> >> +};
> >> +
> >> +static const AVOption rtp_options[] = {
> >> +    RTSP_FLAG_OPTS("rtp_flags", "RTP flags"),
> >> +    { NULL },
> >> +};
> >> +
> >>  static void get_word_until_chars(char *buf, int buf_size,
> >>                                   const char *sep, const char **pp)
> >>  {
> >> @@ -1835,8 +1845,9 @@ static int sdp_read_header(AVFormatContext *s, AVFormatParameters *ap)
> >>                      namebuf, sizeof(namebuf), NULL, 0, NI_NUMERICHOST);
> >>          ff_url_join(url, sizeof(url), "rtp", NULL,
> >>                      namebuf, rtsp_st->sdp_port,
> >> -                    "?localport=%d&ttl=%d", rtsp_st->sdp_port,
> >> -                    rtsp_st->sdp_ttl);
> >> +                    "?localport=%d&ttl=%d&connect=%d", rtsp_st->sdp_port,
> >> +                    rtsp_st->sdp_ttl,
> >> +                    rt->rtsp_flags & RTSP_FLAG_FILTER_SRC ? 1 : 0);
> >>          if (ffurl_open(&rtsp_st->rtp_handle, url, AVIO_FLAG_READ_WRITE) < 0) {
> >>              err = AVERROR_INVALIDDATA;
> >>              goto fail;
> >> @@ -1858,6 +1869,13 @@ static int sdp_read_close(AVFormatContext *s)
> >>      return 0;
> >>  }
> >>
> >> +const AVClass sdp_demuxer_class = {
> >> +    .class_name     = "SDP demuxer",
> >> +    .item_name      = av_default_item_name,
> >> +    .option         = sdp_options,
> >> +    .version        = LIBAVUTIL_VERSION_INT,
> >> +};
> >> +
> >
> > Should be static? same below
> 
> Indeed, fixed locally.
> 

The rest looks fine.

Patch

different name for these (sdp, rtp) demuxers make sense IMO,
since it doesn't matter to the user that these demuxers share
implementation with the RTSP demuxer.


 libavformat/rtsp.c |   31 +++++++++++++++++++++++++++++--
 1 files changed, 29 insertions(+), 2 deletions(-)

diff --git a/libavformat/rtsp.c b/libavformat/rtsp.c
index 4765d2f..a1d63c6 100644
--- a/libavformat/rtsp.c
+++ b/libavformat/rtsp.c
@@ -77,6 +77,16 @@  const AVOption ff_rtsp_options[] = {
     { NULL },
 };
 
+static const AVOption sdp_options[] = {
+    RTSP_FLAG_OPTS("sdp_flags", "SDP flags"),
+    { NULL },
+};
+
+static const AVOption rtp_options[] = {
+    RTSP_FLAG_OPTS("rtp_flags", "RTP flags"),
+    { NULL },
+};
+
 static void get_word_until_chars(char *buf, int buf_size,
                                  const char *sep, const char **pp)
 {
@@ -1835,8 +1845,9 @@  static int sdp_read_header(AVFormatContext *s, AVFormatParameters *ap)
                     namebuf, sizeof(namebuf), NULL, 0, NI_NUMERICHOST);
         ff_url_join(url, sizeof(url), "rtp", NULL,
                     namebuf, rtsp_st->sdp_port,
-                    "?localport=%d&ttl=%d", rtsp_st->sdp_port,
-                    rtsp_st->sdp_ttl);
+                    "?localport=%d&ttl=%d&connect=%d", rtsp_st->sdp_port,
+                    rtsp_st->sdp_ttl,
+                    rt->rtsp_flags & RTSP_FLAG_FILTER_SRC ? 1 : 0);
         if (ffurl_open(&rtsp_st->rtp_handle, url, AVIO_FLAG_READ_WRITE) < 0) {
             err = AVERROR_INVALIDDATA;
             goto fail;
@@ -1858,6 +1869,13 @@  static int sdp_read_close(AVFormatContext *s)
     return 0;
 }
 
+const AVClass sdp_demuxer_class = {
+    .class_name     = "SDP demuxer",
+    .item_name      = av_default_item_name,
+    .option         = sdp_options,
+    .version        = LIBAVUTIL_VERSION_INT,
+};
+
 AVInputFormat ff_sdp_demuxer = {
     .name           = "sdp",
     .long_name      = NULL_IF_CONFIG_SMALL("SDP"),
@@ -1866,6 +1884,7 @@  AVInputFormat ff_sdp_demuxer = {
     .read_header    = sdp_read_header,
     .read_packet    = ff_rtsp_fetch_packet,
     .read_close     = sdp_read_close,
+    .priv_class     = &sdp_demuxer_class
 };
 #endif /* CONFIG_SDP_DEMUXER */
 
@@ -1962,6 +1981,13 @@  fail:
     return ret;
 }
 
+const AVClass rtp_demuxer_class = {
+    .class_name     = "RTP demuxer",
+    .item_name      = av_default_item_name,
+    .option         = rtp_options,
+    .version        = LIBAVUTIL_VERSION_INT,
+};
+
 AVInputFormat ff_rtp_demuxer = {
     .name           = "rtp",
     .long_name      = NULL_IF_CONFIG_SMALL("RTP input format"),
@@ -1971,6 +1997,7 @@  AVInputFormat ff_rtp_demuxer = {
     .read_packet    = ff_rtsp_fetch_packet,
     .read_close     = sdp_read_close,
     .flags = AVFMT_NOFILE,
+    .priv_class     = &rtp_demuxer_class
 };
 #endif /* CONFIG_RTP_DEMUXER */