[vlc-commits] [Git][videolan/vlc][master] 13 commits: vector: add convenience access methods

Steve Lhomme (@robUx4) gitlab at videolan.org
Sat Nov 4 08:16:59 UTC 2023



Steve Lhomme pushed to branch master at VideoLAN / VLC


Commits:
5344b0a9 by Alaric Senat at 2023-11-04T08:03:19+00:00
vector: add convenience access methods

Accessing the last element of a vector is a common operation.
Having it named and bound checked would definetely be a plus for
`vlc_vector` usage. It avoids error-prones index accesses.

- - - - -
314391af by Alaric Senat at 2023-11-04T08:03:19+00:00
vector: add a reference iterator foreach loop

For storage of types larger than native types a ref based iterated loop
is handy to avoid copy.

- - - - -
101b65a6 by Alaric Senat at 2023-11-04T08:03:19+00:00
sout: duplicate: switch to `vlc_vector`

Switch from the legacy `TAB` API to the new vector implementation.
The vectors here will help:
- refactoring the code and get rid of a lot of index based access (with
  foreach loops mainly)
- check for allocation errors (`TAB_...` aborts on malloc failure)

- - - - -
efe92783 by Alaric Senat at 2023-11-04T08:03:19+00:00
sout: duplicate: group next ID with the out stream

- - - - -
695345c1 by Alaric Senat at 2023-11-04T08:03:19+00:00
sout: duplicate: group duplicated stream data

Will make it easier to expand fields stored per duplicated streams and
save one malloc and free calls per duplicated streams.

- - - - -
38694e85 by Alaric Senat at 2023-11-04T08:03:19+00:00
sout: duplicate: remove unnecessary pointer

The select chain can be assigned by accessing the last element of the
vector directly.

- - - - -
d0e36f55 by Alaric Senat at 2023-11-04T08:03:19+00:00
sout: duplicate: remove unnecessary debug messages

Module opening and closure are already generically printed.

- - - - -
28e1cf69 by Alaric Senat at 2023-11-04T08:03:19+00:00
sout: duplicate: stop storing NULL dup_ids

Storing NULL duplicated IDs was necessary before we had a reference to
the duplicate stream output stored with the duplicated id. Storing NULL
allowed to have an index equivalence between the stream array and the ID
array.

Now that IDs can be independently stored, lets remove the NULL pushes
and the unneeded NULL checks!

- - - - -
e6a74456 by Alaric Senat at 2023-11-04T08:03:19+00:00
sout: duplicate: drop frame chain support

Frame chains are not supported anymore in sout's `pf_send`.

- - - - -
d16fe36b by Alaric Senat at 2023-11-04T08:03:19+00:00
sout: duplicate: handle failing alloc in `pf_send`

- - - - -
9a499d52 by Alaric Senat at 2023-11-04T08:03:19+00:00
sout: duplicate: refactor `pf_send`

To also handle the last ID in the for loop.

No functional changes.

- - - - -
835775d4 by Alaric Senat at 2023-11-04T08:03:19+00:00
sout: duplicate: handle OOM errors

Switching to `vlc_vector_t` made it possible to handle memory failures.

- - - - -
43258706 by Alaric Senat at 2023-11-04T08:03:19+00:00
sout: duplicate: switch to `vlc_frame_t`

- - - - -


3 changed files:

- include/vlc_vector.h
- modules/stream_out/duplicate.c
- src/test/vector.c


Changes:

=====================================
include/vlc_vector.h
=====================================
@@ -664,6 +664,38 @@ vlc_vector_move_(char *array, size_t index, size_t count, size_t target)
              ((item) = (pv)->data[vlc_vector_idx_##item], true); \
          ++vlc_vector_idx_##item)
 
+/**
+ * For-each loop with a reference iterator.
+ *
+ * Should be used for vector holding non-trivially copyable data.
+ *
+ * \param[out] ref The reference iterator
+ * \param[in] pv a pointer to the vector
+ */
+#define vlc_vector_foreach_ref(ref, pv) \
+    for (size_t vlc_vector_idx_##ref = 0; \
+         vlc_vector_idx_##ref < (pv)->size && \
+             ((ref) = &(pv)->data[vlc_vector_idx_##ref], true); \
+         ++vlc_vector_idx_##ref)
+
+/**
+ * Returns the vector's last element.
+ */
+#define vlc_vector_last(pv) \
+( \
+    assert((pv)->size != 0), \
+    (pv)->data[(pv)->size - 1] \
+)
+
+/**
+ * Returns a reference on the vector's last element.
+ */
+#define vlc_vector_last_ref(pv) \
+( \
+    assert((pv)->size != 0), \
+    &(pv)->data[(pv)->size - 1] \
+)
+
 /** @} */
 
 #endif


=====================================
modules/stream_out/duplicate.c
=====================================
@@ -29,12 +29,13 @@
 #endif
 
 #include <vlc_common.h>
-#include <vlc_arrays.h>
+
 #include <vlc_configuration.h>
+#include <vlc_frame.h>
 #include <vlc_plugin.h>
 #include <vlc_sout.h>
-#include <vlc_block.h>
 #include <vlc_subpicture.h>
+#include <vlc_vector.h>
 
 /*****************************************************************************
  * Module descriptor
@@ -60,22 +61,28 @@ vlc_module_end ()
  *****************************************************************************/
 static void *Add( sout_stream_t *, const es_format_t *, const char * );
 static void  Del( sout_stream_t *, void * );
-static int   Send( sout_stream_t *, void *, block_t * );
+static int   Send( sout_stream_t *, void *, vlc_frame_t * );
 static void  SetPCR( sout_stream_t *, vlc_tick_t );
 
+typedef struct {
+    sout_stream_t *stream;
+    char *select_chain;
+} duplicated_stream_t;
+
 typedef struct
 {
-    int             i_nb_streams;
-    sout_stream_t   **pp_streams;
-
-    int             i_nb_select;
-    char            **ppsz_select;
+    struct VLC_VECTOR(duplicated_stream_t) streams;
 } sout_stream_sys_t;
 
+typedef struct {
+    void *id;
+    /* Reference to the duplicated output stream. */
+    sout_stream_t *stream_owner;
+} duplicated_id_t;
+
 typedef struct
 {
-    int                 i_nb_ids;
-    void                **pp_ids;
+    struct VLC_VECTOR(duplicated_id_t) dup_ids;
 } sout_stream_id_sys_t;
 
 static bool ESSelected( struct vlc_logger *, const es_format_t *fmt,
@@ -86,8 +93,6 @@ static bool ESSelected( struct vlc_logger *, const es_format_t *fmt,
  *****************************************************************************/
 static int Control( sout_stream_t *p_stream, int i_query, va_list args )
 {
-    sout_stream_sys_t *p_sys = p_stream->p_sys;
-
     /* Fanout controls */
     switch( i_query )
     {
@@ -95,17 +100,19 @@ static int Control( sout_stream_t *p_stream, int i_query, va_list args )
         {
             sout_stream_id_sys_t *id = va_arg(args, void *);
             const vlc_spu_highlight_t *spu_hl = va_arg(args, const vlc_spu_highlight_t *);
-            for( int i = 0; i < id->i_nb_ids; i++ )
+
+            duplicated_id_t *dup_id;
+            vlc_vector_foreach_ref( dup_id, &id->dup_ids )
             {
-                if( id->pp_ids[i] )
-                    sout_StreamControl( p_sys->pp_streams[i], i_query,
-                                        id->pp_ids[i], spu_hl );
+                sout_StreamControl(
+                    dup_id->stream_owner, i_query, dup_id->id, spu_hl );
             }
             return VLC_SUCCESS;
         }
     }
 
     return VLC_EGENERIC;
+    (void)p_stream;
 }
 
 static const struct sout_stream_operations ops = {
@@ -121,32 +128,26 @@ static int Open( vlc_object_t *p_this )
     sout_stream_sys_t *p_sys;
     config_chain_t        *p_cfg;
 
-    msg_Dbg( p_stream, "creating 'duplicate'" );
-
     p_sys = malloc( sizeof( sout_stream_sys_t ) );
     if( !p_sys )
         return VLC_ENOMEM;
 
-    TAB_INIT( p_sys->i_nb_streams, p_sys->pp_streams );
-    TAB_INIT( p_sys->i_nb_select, p_sys->ppsz_select );
-
-    char **ppsz_select = NULL;
+    vlc_vector_init( &p_sys->streams );
 
     for( p_cfg = p_stream->p_cfg; p_cfg != NULL; p_cfg = p_cfg->p_next )
     {
         if( !strncmp( p_cfg->psz_name, "dst", strlen( "dst" ) ) )
         {
-            sout_stream_t *s;
+            duplicated_stream_t dup_stream = {0};
 
             msg_Dbg( p_stream, " * adding `%s'", p_cfg->psz_value );
-            s = sout_StreamChainNew( VLC_OBJECT(p_stream), p_cfg->psz_value,
-                p_stream->p_next );
+            dup_stream.stream = sout_StreamChainNew(
+                VLC_OBJECT(p_stream), p_cfg->psz_value, p_stream->p_next );
 
-            if( s )
+            if( dup_stream.stream != NULL )
             {
-                TAB_APPEND( p_sys->i_nb_streams, p_sys->pp_streams, s );
-                TAB_APPEND( p_sys->i_nb_select,  p_sys->ppsz_select, NULL );
-                ppsz_select = &p_sys->ppsz_select[p_sys->i_nb_select - 1];
+                if( !vlc_vector_push(&p_sys->streams, dup_stream) )
+                    goto nomem;
             }
         }
         else if( !strncmp( p_cfg->psz_name, "select", strlen( "select" ) ) )
@@ -155,15 +156,19 @@ static int Open( vlc_object_t *p_this )
 
             if( psz && *psz )
             {
-                if( ppsz_select == NULL )
+                if( p_sys->streams.size == 0 )
                 {
                     msg_Err( p_stream, " * ignore selection `%s'", psz );
                 }
                 else
                 {
                     msg_Dbg( p_stream, " * apply selection `%s'", psz );
-                    *ppsz_select = strdup( psz );
-                    ppsz_select = NULL;
+                    char *select_chain = strdup( psz );
+                    if( unlikely(select_chain == NULL) )
+                        goto nomem;
+
+                    vlc_vector_last_ref( &p_sys->streams )->select_chain =
+                        select_chain;
                 }
             }
         }
@@ -173,7 +178,7 @@ static int Open( vlc_object_t *p_this )
         }
     }
 
-    if( p_sys->i_nb_streams == 0 )
+    if( p_sys->streams.size == 0 )
     {
         msg_Err( p_stream, "no destination given" );
         free( p_sys );
@@ -184,6 +189,10 @@ static int Open( vlc_object_t *p_this )
     p_stream->p_sys = p_sys;
     p_stream->ops = &ops;
     return VLC_SUCCESS;
+nomem:
+    p_stream->p_sys = p_sys;
+    Close( p_this );
+    return VLC_ENOMEM;
 }
 
 /*****************************************************************************
@@ -194,14 +203,13 @@ static void Close( vlc_object_t * p_this )
     sout_stream_t     *p_stream = (sout_stream_t*)p_this;
     sout_stream_sys_t *p_sys = p_stream->p_sys;
 
-    msg_Dbg( p_stream, "closing a duplication" );
-    for( int i = 0; i < p_sys->i_nb_streams; i++ )
+    duplicated_stream_t *dup_stream;
+    vlc_vector_foreach_ref( dup_stream, &p_sys->streams )
     {
-        sout_StreamChainDelete(p_sys->pp_streams[i], p_stream->p_next);
-        free( p_sys->ppsz_select[i] );
+        sout_StreamChainDelete( dup_stream->stream, p_stream->p_next );
+        free( dup_stream->select_chain );
     }
-    free( p_sys->pp_streams );
-    free( p_sys->ppsz_select );
+    vlc_vector_destroy( &p_sys->streams );
 
     free( p_sys );
 }
@@ -214,56 +222,54 @@ Add( sout_stream_t *p_stream, const es_format_t *p_fmt, const char *es_id )
 {
     sout_stream_sys_t *p_sys = p_stream->p_sys;
     sout_stream_id_sys_t  *id;
-    int i_stream, i_valid_streams = 0;
 
     id = malloc( sizeof( sout_stream_id_sys_t ) );
     if( !id )
         return NULL;
 
-    TAB_INIT( id->i_nb_ids, id->pp_ids );
+    vlc_vector_init( &id->dup_ids );
 
     msg_Dbg( p_stream, "duplicated a new stream codec=%4.4s (es=%d group=%d)",
              (char*)&p_fmt->i_codec, p_fmt->i_id, p_fmt->i_group );
 
-    for( i_stream = 0; i_stream < p_sys->i_nb_streams; i_stream++ )
+    duplicated_stream_t *dup_stream;
+    vlc_vector_foreach_ref( dup_stream, &p_sys->streams )
     {
-        void *id_new = NULL;
+        const size_t idx = vlc_vector_idx_dup_stream;
 
-        if( ESSelected( p_stream->obj.logger, p_fmt,
-                        p_sys->ppsz_select[i_stream] ) )
+        if( ESSelected(p_stream->obj.logger, p_fmt, dup_stream->select_chain) )
         {
-            sout_stream_t *out = p_sys->pp_streams[i_stream];
-
             /* FIXME(Alaric): suffix the string id with the duplicated track
              * count. */
-            id_new = (void*)sout_StreamIdAdd( out, p_fmt, es_id );
-            if( id_new )
+            void *next_id = sout_StreamIdAdd( dup_stream->stream,
+                                              p_fmt, es_id );
+            if( next_id != NULL )
             {
-                msg_Dbg( p_stream, "    - added for output %d", i_stream );
-                i_valid_streams++;
+                msg_Dbg( p_stream, "    - added for output %zu", idx );
+                const duplicated_id_t dup_id = {
+                    .stream_owner = dup_stream->stream, .id = next_id};
+                if( !vlc_vector_push(&id->dup_ids, dup_id) )
+                {
+                    sout_StreamIdDel( dup_stream->stream, next_id );
+                    goto error;
+                }
             }
             else
             {
-                msg_Dbg( p_stream, "    - failed for output %d", i_stream );
+                msg_Dbg( p_stream, "    - failed for output %zu", idx);
             }
         }
         else
         {
-            msg_Dbg( p_stream, "    - ignored for output %d", i_stream );
+            msg_Dbg( p_stream, "    - ignored for output %zu", idx );
         }
-
-        /* Append failed attempts as well to keep track of which pp_id
-         * belongs to which duplicated stream */
-        TAB_APPEND( id->i_nb_ids, id->pp_ids, id_new );
-    }
-
-    if( i_valid_streams <= 0 )
-    {
-        Del( p_stream, id );
-        return NULL;
     }
 
-    return id;
+    if( id->dup_ids.size > 0 )
+        return id;
+error:
+    Del( p_stream, id );
+    return NULL;
 }
 
 /*****************************************************************************
@@ -271,75 +277,55 @@ Add( sout_stream_t *p_stream, const es_format_t *p_fmt, const char *es_id )
  *****************************************************************************/
 static void Del( sout_stream_t *p_stream, void *_id )
 {
-    sout_stream_sys_t *p_sys = p_stream->p_sys;
     sout_stream_id_sys_t *id = (sout_stream_id_sys_t *)_id;
-    int               i_stream;
 
-    for( i_stream = 0; i_stream < p_sys->i_nb_streams; i_stream++ )
+    duplicated_id_t *dup_id;
+    vlc_vector_foreach_ref( dup_id, &id->dup_ids )
     {
-        if( id->pp_ids[i_stream] )
-        {
-            sout_stream_t *out = p_sys->pp_streams[i_stream];
-            sout_StreamIdDel( out, id->pp_ids[i_stream] );
-        }
+        sout_StreamIdDel( dup_id->stream_owner, dup_id->id );
     }
+    vlc_vector_destroy( &id->dup_ids );
 
-    free( id->pp_ids );
     free( id );
+    (void)p_stream;
 }
 
 /*****************************************************************************
  * Send:
  *****************************************************************************/
-static int Send( sout_stream_t *p_stream, void *_id, block_t *p_buffer )
+static int Send( sout_stream_t *p_stream, void *_id, vlc_frame_t *frame )
 {
-    sout_stream_sys_t *p_sys = p_stream->p_sys;
     sout_stream_id_sys_t *id = (sout_stream_id_sys_t *)_id;
-    sout_stream_t     *p_dup_stream;
-    int               i_stream;
-
-    /* Loop through the linked list of buffers */
-    while( p_buffer )
-    {
-        block_t *p_next = p_buffer->p_next;
-
-        p_buffer->p_next = NULL;
 
-        for( i_stream = 0; i_stream < p_sys->i_nb_streams - 1; i_stream++ )
-        {
-            p_dup_stream = p_sys->pp_streams[i_stream];
-
-            if( id->pp_ids[i_stream] )
-            {
-                block_t *p_dup = block_Duplicate( p_buffer );
+    /* Should be ensured in `Add`. */
+    assert(id->dup_ids.size > 0);
 
-                if( p_dup )
-                    sout_StreamIdSend( p_dup_stream, id->pp_ids[i_stream], p_dup );
-            }
-        }
-
-        if( i_stream < p_sys->i_nb_streams && id->pp_ids[i_stream] )
-        {
-            p_dup_stream = p_sys->pp_streams[i_stream];
-            sout_StreamIdSend( p_dup_stream, id->pp_ids[i_stream], p_buffer );
-        }
-        else
+    duplicated_id_t *dup_id;
+    vlc_vector_foreach_ref( dup_id, &id->dup_ids )
+    {
+        const bool is_last = dup_id == vlc_vector_last_ref( &id->dup_ids );
+        vlc_frame_t *to_send = (is_last) ? frame : vlc_frame_Duplicate( frame );
+        if ( unlikely(to_send == NULL) )
         {
-            block_Release( p_buffer );
+            vlc_frame_Release( frame );
+            return VLC_ENOMEM;
         }
-
-        p_buffer = p_next;
+            
+        sout_StreamIdSend( dup_id->stream_owner, dup_id->id, to_send );
     }
+
     return VLC_SUCCESS;
+    (void)p_stream;
 }
 
 static void SetPCR( sout_stream_t *stream, vlc_tick_t pcr )
 {
     sout_stream_sys_t *sys = stream->p_sys;
 
-    for ( int i = 0; i < sys->i_nb_streams; ++i )
+    duplicated_stream_t *dup_stream;
+    vlc_vector_foreach_ref( dup_stream, &sys->streams )
     {
-        sout_StreamSetPCR( sys->pp_streams[i], pcr );
+        sout_StreamSetPCR( dup_stream->stream, pcr );
     }
 }
 


=====================================
src/test/vector.c
=====================================
@@ -39,6 +39,8 @@ static void test_vector_insert_remove(void)
     assert(ok);
     assert(vec.data[0] == 42);
     assert(vec.size == 1);
+    assert(vlc_vector_last(&vec) == 42);
+    assert(*vlc_vector_last_ref(&vec) == 42);
 
     ok = vlc_vector_push(&vec, 37);
     assert(ok);
@@ -66,6 +68,8 @@ static void test_vector_insert_remove(void)
     assert(vec.data[0] == 42);
     assert(vec.data[1] == 37);
     assert(vec.data[2] == 77);
+    assert(vlc_vector_last(&vec) == 77);
+    assert(*vlc_vector_last_ref(&vec) == 77);
 
     vlc_vector_clear(&vec);
     assert(vec.size == 0);
@@ -253,6 +257,15 @@ static void test_vector_foreach(void)
     }
     assert(count == 10);
 
+    count = 0;
+    const int *ref;
+    vlc_vector_foreach_ref(ref, &vec)
+    {
+        assert(*ref == count);
+        assert(ref == &vec.data[count]);
+        count++;
+    }
+
     vlc_vector_destroy(&vec);
 }
 



View it on GitLab: https://code.videolan.org/videolan/vlc/-/compare/fbb608d18c727db4a05c03abf03cd2bef7d7ae60...43258706e3ec925bd90e6c958a196e1a28bc9a8a

-- 
View it on GitLab: https://code.videolan.org/videolan/vlc/-/compare/fbb608d18c727db4a05c03abf03cd2bef7d7ae60...43258706e3ec925bd90e6c958a196e1a28bc9a8a
You're receiving this email because of your account on code.videolan.org.


VideoLAN code repository instance


More information about the vlc-commits mailing list