diff mbox series

[FFmpeg-devel,v2] avformat/libsrt: log streamid in listener mode

Message ID tencent_37CEE1625A92346F815C344F3F2756679D09@qq.com
State Superseded
Headers show
Series [FFmpeg-devel,v2] avformat/libsrt: log streamid in listener mode | expand

Checks

Context Check Description
andriy/x86_make success Make finished
andriy/x86_make_fate success Make fate finished
andriy/PPC64_make success Make finished
andriy/PPC64_make_fate success Make fate finished

Commit Message

Zhao Zhili May 31, 2021, 4:02 p.m. UTC
It's useful for test client which pass streamid to ffmpeg/ffplay.
For example, use ffmpeg to test streamid support in VLC:
./ffmpeg -v info -re -i foo.mp4 -c copy -f mpegts -mode listener srt://127.0.0.1:9000
./vlc srt://127.0.0.1:9000?streamid=foobar
---
 libavformat/libsrt.c | 22 +++++++++++++---------
 1 file changed, 13 insertions(+), 9 deletions(-)

Comments

Marton Balint June 8, 2021, 10:04 p.m. UTC | #1
On Tue, 1 Jun 2021, Zhao Zhili wrote:

> It's useful for test client which pass streamid to ffmpeg/ffplay.
> For example, use ffmpeg to test streamid support in VLC:
> ./ffmpeg -v info -re -i foo.mp4 -c copy -f mpegts -mode listener srt://127.0.0.1:9000
> ./vlc srt://127.0.0.1:9000?streamid=foobar
> ---
> libavformat/libsrt.c | 22 +++++++++++++---------
> 1 file changed, 13 insertions(+), 9 deletions(-)
>
> diff --git a/libavformat/libsrt.c b/libavformat/libsrt.c
> index c1e96f700e..613997c501 100644
> --- a/libavformat/libsrt.c
> +++ b/libavformat/libsrt.c
> @@ -153,6 +153,15 @@ static int libsrt_neterrno(URLContext *h)
>     return os_errno ? AVERROR(os_errno) : AVERROR_UNKNOWN;
> }
>
> +static int libsrt_getsockopt(URLContext *h, int fd, SRT_SOCKOPT optname, const char * optnamestr, void * optval, int * optlen)
> +{
> +    if (srt_getsockopt(fd, 0, optname, optval, optlen) < 0) {
> +        av_log(h, AV_LOG_ERROR, "failed to get option %s on socket: %s\n", optnamestr, srt_getlasterror_str());
> +        return AVERROR(EIO);
> +    }
> +    return 0;
> +}
> +
> static int libsrt_socket_nonblock(int socket, int enable)
> {
>     int ret, blocking = enable ? 0 : 1;
> @@ -224,6 +233,8 @@ static int libsrt_listen(int eid, int fd, const struct sockaddr *addr, socklen_t
> {
>     int ret;
>     int reuse = 1;
> +    char streamid[512] = {};
> +    int streamid_len = sizeof(streamid);
>     if (srt_setsockopt(fd, SOL_SOCKET, SRTO_REUSEADDR, &reuse, sizeof(reuse))) {
>         av_log(h, AV_LOG_WARNING, "setsockopt(SRTO_REUSEADDR) failed\n");
>     }
> @@ -242,6 +253,8 @@ static int libsrt_listen(int eid, int fd, const struct sockaddr *addr, socklen_t
>         return libsrt_neterrno(h);
>     if (libsrt_socket_nonblock(ret, 1) < 0)
>         av_log(h, AV_LOG_DEBUG, "libsrt_socket_nonblock failed\n");
> +    if (!libsrt_getsockopt(h, ret, SRTO_STREAMID, "SRTO_STREAMID", streamid, &streamid_len))
> +        av_log(h, AV_LOG_VERBOSE, "accept streamid [%s], length %d\n", streamid, streamid_len);

Does it make sense to print the length? Shouldn't this be

("accepted streamid [%.*s]\n", streamid_len, streamid)

instead to avoid overreads at 512 byte streamids, or dumping garbage if it 
is not 0 terminated? (You can also avoid 0 initialization that way).

Thanks,
Marton

>
>     return ret;
> }
> @@ -276,15 +289,6 @@ static int libsrt_setsockopt(URLContext *h, int fd, SRT_SOCKOPT optname, const c
>     return 0;
> }
>
> -static int libsrt_getsockopt(URLContext *h, int fd, SRT_SOCKOPT optname, const char * optnamestr, void * optval, int * optlen)
> -{
> -    if (srt_getsockopt(fd, 0, optname, optval, optlen) < 0) {
> -        av_log(h, AV_LOG_ERROR, "failed to get option %s on socket: %s\n", optnamestr, srt_getlasterror_str());
> -        return AVERROR(EIO);
> -    }
> -    return 0;
> -}
> -
> /* - The "POST" options can be altered any time on a connected socket.
>      They MAY have also some meaning when set prior to connecting; such
>      option is SRTO_RCVSYN, which makes connect/accept call asynchronous.
> -- 
> 2.25.1
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
>
Zhao Zhili June 10, 2021, 3:17 a.m. UTC | #2
> On Jun 9, 2021, at 6:04 AM, Marton Balint <cus@passwd.hu> wrote:
> 
> 
> On Tue, 1 Jun 2021, Zhao Zhili wrote:
> 
>> It's useful for test client which pass streamid to ffmpeg/ffplay.
>> For example, use ffmpeg to test streamid support in VLC:
>> ./ffmpeg -v info -re -i foo.mp4 -c copy -f mpegts -mode listener srt://127.0.0.1:9000
>> ./vlc srt://127.0.0.1:9000?streamid=foobar
>> ---
>> libavformat/libsrt.c | 22 +++++++++++++---------
>> 1 file changed, 13 insertions(+), 9 deletions(-)
>> 
>> diff --git a/libavformat/libsrt.c b/libavformat/libsrt.c
>> index c1e96f700e..613997c501 100644
>> --- a/libavformat/libsrt.c
>> +++ b/libavformat/libsrt.c
>> @@ -153,6 +153,15 @@ static int libsrt_neterrno(URLContext *h)
>>    return os_errno ? AVERROR(os_errno) : AVERROR_UNKNOWN;
>> }
>> 
>> +static int libsrt_getsockopt(URLContext *h, int fd, SRT_SOCKOPT optname, const char * optnamestr, void * optval, int * optlen)
>> +{
>> +    if (srt_getsockopt(fd, 0, optname, optval, optlen) < 0) {
>> +        av_log(h, AV_LOG_ERROR, "failed to get option %s on socket: %s\n", optnamestr, srt_getlasterror_str());
>> +        return AVERROR(EIO);
>> +    }
>> +    return 0;
>> +}
>> +
>> static int libsrt_socket_nonblock(int socket, int enable)
>> {
>>    int ret, blocking = enable ? 0 : 1;
>> @@ -224,6 +233,8 @@ static int libsrt_listen(int eid, int fd, const struct sockaddr *addr, socklen_t
>> {
>>    int ret;
>>    int reuse = 1;
>> +    char streamid[512] = {};
>> +    int streamid_len = sizeof(streamid);
>>    if (srt_setsockopt(fd, SOL_SOCKET, SRTO_REUSEADDR, &reuse, sizeof(reuse))) {
>>        av_log(h, AV_LOG_WARNING, "setsockopt(SRTO_REUSEADDR) failed\n");
>>    }
>> @@ -242,6 +253,8 @@ static int libsrt_listen(int eid, int fd, const struct sockaddr *addr, socklen_t
>>        return libsrt_neterrno(h);
>>    if (libsrt_socket_nonblock(ret, 1) < 0)
>>        av_log(h, AV_LOG_DEBUG, "libsrt_socket_nonblock failed\n");
>> +    if (!libsrt_getsockopt(h, ret, SRTO_STREAMID, "SRTO_STREAMID", streamid, &streamid_len))
>> +        av_log(h, AV_LOG_VERBOSE, "accept streamid [%s], length %d\n", streamid, streamid_len);
> 
> Does it make sense to print the length? Shouldn't this be
> 
> ("accepted streamid [%.*s]\n", streamid_len, streamid)
> 
> instead to avoid overreads at 512 byte streamids, or dumping garbage if it is not 0 terminated? (You can also avoid 0 initialization that way).

srt_getsockopt() is guaranteed to return ‘\0’ terminated string on success, so the 0 initialization is unnecessary.

I made another mistake: the length of streamid array should be 513:

https://github.com/Haivision/srt/blob/master/docs/API/API-socket-options.md#SRTO_STREAMID
> string - a C string. When setting an option, a const char* character array pointer is expected to be passed in optval and the array length in optlen without the terminating NULL character. When getting, an array is expected to be passed in optval with a sufficient size with an extra space for the terminating NULL character provided in optlen. The return value of optlen does not count the terminating NULL character.

Print the streamid_len has some usercase. For example, to test the corner case of upper bound of streamid length, the caller can send streamid made from 512 ‘x'. Print the streamid_len so I don’t need to count or compare the received streamid to verify the integrity.

> 
> Thanks,
> Marton
> 
>> 
>>    return ret;
>> }
>> @@ -276,15 +289,6 @@ static int libsrt_setsockopt(URLContext *h, int fd, SRT_SOCKOPT optname, const c
>>    return 0;
>> }
>> 
>> -static int libsrt_getsockopt(URLContext *h, int fd, SRT_SOCKOPT optname, const char * optnamestr, void * optval, int * optlen)
>> -{
>> -    if (srt_getsockopt(fd, 0, optname, optval, optlen) < 0) {
>> -        av_log(h, AV_LOG_ERROR, "failed to get option %s on socket: %s\n", optnamestr, srt_getlasterror_str());
>> -        return AVERROR(EIO);
>> -    }
>> -    return 0;
>> -}
>> -
>> /* - The "POST" options can be altered any time on a connected socket.
>>     They MAY have also some meaning when set prior to connecting; such
>>     option is SRTO_RCVSYN, which makes connect/accept call asynchronous.
>> -- 
>> 2.25.1
>> 
>> _______________________________________________
>> ffmpeg-devel mailing list
>> ffmpeg-devel@ffmpeg.org
>> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>> 
>> To unsubscribe, visit link above, or email
>> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
>> 
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
diff mbox series

Patch

diff --git a/libavformat/libsrt.c b/libavformat/libsrt.c
index c1e96f700e..613997c501 100644
--- a/libavformat/libsrt.c
+++ b/libavformat/libsrt.c
@@ -153,6 +153,15 @@  static int libsrt_neterrno(URLContext *h)
     return os_errno ? AVERROR(os_errno) : AVERROR_UNKNOWN;
 }
 
+static int libsrt_getsockopt(URLContext *h, int fd, SRT_SOCKOPT optname, const char * optnamestr, void * optval, int * optlen)
+{
+    if (srt_getsockopt(fd, 0, optname, optval, optlen) < 0) {
+        av_log(h, AV_LOG_ERROR, "failed to get option %s on socket: %s\n", optnamestr, srt_getlasterror_str());
+        return AVERROR(EIO);
+    }
+    return 0;
+}
+
 static int libsrt_socket_nonblock(int socket, int enable)
 {
     int ret, blocking = enable ? 0 : 1;
@@ -224,6 +233,8 @@  static int libsrt_listen(int eid, int fd, const struct sockaddr *addr, socklen_t
 {
     int ret;
     int reuse = 1;
+    char streamid[512] = {};
+    int streamid_len = sizeof(streamid);
     if (srt_setsockopt(fd, SOL_SOCKET, SRTO_REUSEADDR, &reuse, sizeof(reuse))) {
         av_log(h, AV_LOG_WARNING, "setsockopt(SRTO_REUSEADDR) failed\n");
     }
@@ -242,6 +253,8 @@  static int libsrt_listen(int eid, int fd, const struct sockaddr *addr, socklen_t
         return libsrt_neterrno(h);
     if (libsrt_socket_nonblock(ret, 1) < 0)
         av_log(h, AV_LOG_DEBUG, "libsrt_socket_nonblock failed\n");
+    if (!libsrt_getsockopt(h, ret, SRTO_STREAMID, "SRTO_STREAMID", streamid, &streamid_len))
+        av_log(h, AV_LOG_VERBOSE, "accept streamid [%s], length %d\n", streamid, streamid_len);
 
     return ret;
 }
@@ -276,15 +289,6 @@  static int libsrt_setsockopt(URLContext *h, int fd, SRT_SOCKOPT optname, const c
     return 0;
 }
 
-static int libsrt_getsockopt(URLContext *h, int fd, SRT_SOCKOPT optname, const char * optnamestr, void * optval, int * optlen)
-{
-    if (srt_getsockopt(fd, 0, optname, optval, optlen) < 0) {
-        av_log(h, AV_LOG_ERROR, "failed to get option %s on socket: %s\n", optnamestr, srt_getlasterror_str());
-        return AVERROR(EIO);
-    }
-    return 0;
-}
-
 /* - The "POST" options can be altered any time on a connected socket.
      They MAY have also some meaning when set prior to connecting; such
      option is SRTO_RCVSYN, which makes connect/accept call asynchronous.