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

["zhilizhao(赵志立)"]

> On Jun 9, 2021, at 6:04 AM, Marton Balint <cus at 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 at ffmpeg.org
>> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>> 
>> To unsubscribe, visit link above, or email
>> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".
>> 
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".