[FFmpeg-devel,3/3] avformat/udp: Enable FIFO when using windows sockets.

Submitted by Matt Oliver on Dec. 12, 2016, 5 a.m.

Details

Message ID CAHVN4mguNyUjjDQ-Wb8cV_R8a92pGURm1PWo2EmVVWNK3WVnaQ@mail.gmail.com
State New
Headers show

Commit Message

Matt Oliver Dec. 12, 2016, 5 a.m.
---
 libavformat/udp.c | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

             av_log(h, AV_LOG_ERROR, "pthread_join(): %s\n", strerror(ret));
--

Comments

Nicolas George Dec. 15, 2016, 10:42 a.m.
Le duodi 22 frimaire, an CCXXV, Matt Oliver a écrit :
> ---
>  libavformat/udp.c | 17 ++++++++++++++++-
>  1 file changed, 16 insertions(+), 1 deletion(-)
> 
> diff --git a/libavformat/udp.c b/libavformat/udp.c
> index f8c861d..3cafb32 100644
> --- a/libavformat/udp.c
> +++ b/libavformat/udp.c
> @@ -64,6 +64,14 @@
>  #define HAVE_PTHREAD_CANCEL 0
>  #endif
> 
> +#if HAVE_THREADS && HAVE_WINSOCK2_H
> +/* Winsock2 recv function can be unblocked by shutting down the socket */
> +#define pthread_setcancelstate(x, y)
> +#define pthread_cancel
> +#undef HAVE_PTHREAD_CANCEL
> +#define HAVE_PTHREAD_CANCEL 1
> +#endif
> +
>  #if HAVE_PTHREAD_CANCEL
>  #include "libavutil/thread.h"
>  #endif
> @@ -526,6 +534,8 @@ static void *circular_buffer_task_rx( void *_URLContext)
>                  goto end;
>              }
>              continue;

> +        } else if (len == 0) {
> +            goto end;

Unfortunately, UDP packets of size 0 exist and are returned to the
application. If len == 0 is the only criterion to detect a read
interrupted by shutdown(), it will not be usable for UDP.

You can still combine with your original idea of an atomic flag, though.

>          }
>          AV_WL32(s->tmp, len);
> 
> @@ -1144,8 +1154,13 @@ static int udp_close(URLContext *h)
>      if (s->thread_started) {
>          int ret;
>          // Cancel only read, as write has been signaled as success to the
> user
> -        if (h->flags & AVIO_FLAG_READ)
> +        if (h->flags & AVIO_FLAG_READ) {
> +#   if HAVE_THREADS && HAVE_WINSOCK2_H
> +            shutdown(s->udp_fd, SD_BOTH);
> +#   else
>              pthread_cancel(s->circular_buffer_thread);
> +#   endif
> +        }
>          ret = pthread_join(s->circular_buffer_thread, NULL);
>          if (ret != 0)
>              av_log(h, AV_LOG_ERROR, "pthread_join(): %s\n", strerror(ret));

Regards,
Matt Oliver Dec. 16, 2016, 1:17 a.m.
On 15 December 2016 at 21:42, Nicolas George <george@nsup.org> wrote:

> Le duodi 22 frimaire, an CCXXV, Matt Oliver a écrit :
> > ---
> >  libavformat/udp.c | 17 ++++++++++++++++-
> >  1 file changed, 16 insertions(+), 1 deletion(-)
> >
> > diff --git a/libavformat/udp.c b/libavformat/udp.c
> > index f8c861d..3cafb32 100644
> > --- a/libavformat/udp.c
> > +++ b/libavformat/udp.c
> > @@ -64,6 +64,14 @@
> >  #define HAVE_PTHREAD_CANCEL 0
> >  #endif
> >
> > +#if HAVE_THREADS && HAVE_WINSOCK2_H
> > +/* Winsock2 recv function can be unblocked by shutting down the socket
> */
> > +#define pthread_setcancelstate(x, y)
> > +#define pthread_cancel
> > +#undef HAVE_PTHREAD_CANCEL
> > +#define HAVE_PTHREAD_CANCEL 1
> > +#endif
> > +
> >  #if HAVE_PTHREAD_CANCEL
> >  #include "libavutil/thread.h"
> >  #endif
> > @@ -526,6 +534,8 @@ static void *circular_buffer_task_rx( void
> *_URLContext)
> >                  goto end;
> >              }
> >              continue;
>
> > +        } else if (len == 0) {
> > +            goto end;
>
> Unfortunately, UDP packets of size 0 exist and are returned to the
> application. If len == 0 is the only criterion to detect a read
> interrupted by shutdown(), it will not be usable for UDP.
>
> You can still combine with your original idea of an atomic flag, though.


On further reading it turns out the zero length check is not required (as
its not needed for connectionless sockets such as udp). So it turns out it
works without the addition of that check as instead it just returns an
error of WSAESHUTDOW. So ive updated the patch locally to remove then
len==0 check.

Patch hide | download patch | download mbox

diff --git a/libavformat/udp.c b/libavformat/udp.c
index f8c861d..3cafb32 100644
--- a/libavformat/udp.c
+++ b/libavformat/udp.c
@@ -64,6 +64,14 @@ 
 #define HAVE_PTHREAD_CANCEL 0
 #endif

+#if HAVE_THREADS && HAVE_WINSOCK2_H
+/* Winsock2 recv function can be unblocked by shutting down the socket */
+#define pthread_setcancelstate(x, y)
+#define pthread_cancel
+#undef HAVE_PTHREAD_CANCEL
+#define HAVE_PTHREAD_CANCEL 1
+#endif
+
 #if HAVE_PTHREAD_CANCEL
 #include "libavutil/thread.h"
 #endif
@@ -526,6 +534,8 @@  static void *circular_buffer_task_rx( void *_URLContext)
                 goto end;
             }
             continue;
+        } else if (len == 0) {
+            goto end;
         }
         AV_WL32(s->tmp, len);

@@ -1144,8 +1154,13 @@  static int udp_close(URLContext *h)
     if (s->thread_started) {
         int ret;
         // Cancel only read, as write has been signaled as success to the
user
-        if (h->flags & AVIO_FLAG_READ)
+        if (h->flags & AVIO_FLAG_READ) {
+#   if HAVE_THREADS && HAVE_WINSOCK2_H
+            shutdown(s->udp_fd, SD_BOTH);
+#   else
             pthread_cancel(s->circular_buffer_thread);
+#   endif
+        }
         ret = pthread_join(s->circular_buffer_thread, NULL);
         if (ret != 0)