[vlc-devel] [PATCH 2/2] srt: Read several chunks per blocking read call

[Romain Vimont]

On Mon, Aug 20, 2018 at 10:20:10AM +0300, Olivier Crête wrote:
> Hi,
> 
> Re-ping about this patch?
> 
> Olivier
> 
> On Thu, 2018-07-26 at 17:47 -0400, Olivier Crête wrote:
> > From: Roman Diouskine <rdiouskine at haivision.com>
> > 
> > libsrt input is asynchonously buffered internally and it makes sense
> > to empty those receive buffers as much as possible on every signaled
> > receive event from epoll. Doing so reduces context
> > switching/re-scheduling and improves performance.
> > ---
> >  modules/access/srt.c | 62 +++++++++++++++++++++++++++++++++++++-----
> > --
> >  1 file changed, 53 insertions(+), 9 deletions(-)
> > 
> > diff --git a/modules/access/srt.c b/modules/access/srt.c
> > index 5b7734310b..c95123c738 100644
> > --- a/modules/access/srt.c
> > +++ b/modules/access/srt.c
> > @@ -40,6 +40,9 @@
> >  /* libsrt defines default packet size as 1316 internally
> >   * so srt module takes same value. */
> >  #define SRT_DEFAULT_CHUNK_SIZE 1316
> > +/* Minimum/Maximum chunks to allow reading at a time from libsrt */
> > +#define SRT_MIN_CHUNKS_TRYREAD 10
> > +#define SRT_MAX_CHUNKS_TRYREAD 100
> >  /* The default timeout is -1 (infinite) */
> >  #define SRT_DEFAULT_POLL_TIMEOUT -1
> >  /* The default latency is 125
> > @@ -64,6 +67,7 @@ typedef struct
> >      bool        b_interrupted;
> >      char       *psz_host;
> >      int         i_port;
> > +    int         i_chunks; /* Number of chunks to allocate in the
> > next read */
> >  } stream_sys_t;
> >  
> >  static void srt_wait_interrupted(void *p_data)
> > @@ -193,6 +197,11 @@ static bool srt_schedule_reconnect(stream_t
> > *p_stream)
> >          failed = true;
> >      }
> >  
> > +    /* Reset the number of chunks to allocate as the bitrate of
> > +     * the stream may have changed.
> > +     */
> > +    p_sys->i_chunks = SRT_MIN_CHUNKS_TRYREAD;
> > +
> >  out:
> >      if (failed && p_sys->sock != SRT_INVALID_SOCK)
> >      {
> > @@ -221,7 +230,13 @@ static block_t *BlockSRT(stream_t *p_stream,
> > bool *restrict eof)
> >          return NULL;
> >      }
> >  
> > -    block_t *pkt = block_Alloc( i_chunk_size );
> > +    if ( p_sys->i_chunks == 0 )
> > +        p_sys->i_chunks = SRT_MIN_CHUNKS_TRYREAD;
> > +
> > +    size_t i_chunk_size_actual = ( i_chunk_size > 0 )
> > +        ? i_chunk_size : SRT_DEFAULT_CHUNK_SIZE;
> > +    size_t bufsize = i_chunk_size_actual * p_sys->i_chunks;
> > +    block_t *pkt = block_Alloc( bufsize );
> >      if ( unlikely( pkt == NULL ) )
> >      {
> >          return NULL;
> > @@ -259,20 +274,49 @@ static block_t *BlockSRT(stream_t *p_stream,
> > bool *restrict eof)
> >                  continue;
> >          }
> >  
> > -        int stat = srt_recvmsg( p_sys->sock,
> > -            (char *)pkt->p_buffer, i_chunk_size );
> > -        if ( stat > 0 )
> > +        /* Try to get as much data as possible out of the lib, if
> > there
> > +         * is still some left, increase the number of chunks to read
> > so that
> > +         * it will read faster on the next iteration. This way the
> > buffer will
> > +         * grow until it reads fast enough to keep the library empty
> > after
> > +         * each iteration.
> > +         */
> > +        pkt->i_buffer = 0;
> > +        while ( ( bufsize - pkt->i_buffer ) >= i_chunk_size_actual )
> > +        {
> > +            int stat = srt_recvmsg( p_sys->sock,
> > +                (char *)( pkt->p_buffer + pkt->i_buffer ),
> > +                bufsize - pkt->i_buffer );
> > +            if ( stat <= 0 )
> > +            {
> > +                break;

If the first srt_recvmsg() returns <= 0, then BlockSRT() now returns an
empty block_t, while it returned NULL before the patch.

I don't know if this is important.

Otherwise, the logic looks good to me.

> > +            }
> > +            pkt->i_buffer += (size_t)stat;
> > +        }
> > +
> > +        msg_Dbg ( p_stream, "Read %zu bytes out of a max of %zu"
> > +            " (%d chunks of %zu bytes)", pkt->i_buffer,
> > +            p_sys->i_chunks * i_chunk_size_actual, p_sys->i_chunks,
> > +                 i_chunk_size_actual );
> > +
> > +
> > +        /* Gradually adjust number of chunks we read at a time
> > +        * up to a predefined maximum. The actual number we might
> > +        * settle on depends on stream's bit rate.
> > +        */
> > +        size_t rem = bufsize - pkt->i_buffer;
> > +        if ( rem < i_chunk_size_actual )
> >          {
> > -            pkt->i_buffer = stat;
> > -            goto out;
> > +            if ( p_sys->i_chunks < SRT_MAX_CHUNKS_TRYREAD )
> > +            {
> > +                p_sys->i_chunks++;
> > +            }
> >          }
> >  
> >          goto out;
> >      }
> >  
> > -    /* if the poll reports errors for any reason at all
> > -     * including a timeout, or there is a read error,
> > -     * we skip the turn.
> > +    /* if the poll reports errors for any reason at all,
> > +     * including a timeout, we skip the turn.
> >       */
> >  
> >      block_Release(pkt);
> -- 
> Olivier Crête
> olivier.crete at collabora.com
> 
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel