[vlc-devel] [PATCH] input: Don't probe the stream when controlling a demux_Filter
Rémi Denis-Courmont
remi at remlab.net
Thu Jan 11 05:38:12 CET 2018
Le 10 janvier 2018 19:06:31 GMT+02:00, "Hugo Beauzée-Luyssen" <hugo at beauzee.fr> a écrit :
>When a stream filter is active, attempting to read any file which
>demux doesn't implement DEMUX_CAN_PAUSE, DEMUX_CAN_CONTROL_PACE,
>DEMUX_GET_PTS_DELAY or DEMUX_SET_PAUSE_STATE will cause undefined
>behavior when accessing demux_t::s
>---
>src/input/demux.c | 139
>++++++++++++++++++++++++++++++------------------------
> 1 file changed, 77 insertions(+), 62 deletions(-)
>
>diff --git a/src/input/demux.c b/src/input/demux.c
>index 18b64b5f31..e594014ac0 100644
>--- a/src/input/demux.c
>+++ b/src/input/demux.c
>@@ -155,6 +155,7 @@ typedef struct demux_priv_t
> {
> demux_t demux;
> void (*destroy)(demux_t *);
>+ int (*control)( demux_t *, int, va_list );
> } demux_priv_t;
>
> static void demux_DestroyDemux(demux_t *demux)
>@@ -191,6 +192,80 @@ static int demux_Probe(void *func, va_list ap)
> return probe(VLC_OBJECT(demux));
> }
>
>+static int demux_ControlInternal( demux_t *demux, int query, ... )
>+{
>+ int ret;
>+ va_list ap;
>+
>+ va_start( ap, query );
>+ ret = demux->pf_control( demux, query, ap );
>+ va_end( ap );
>+ return ret;
>+}
>+
>+int demux_vaControl( demux_t *demux, int query, va_list args )
>+{
>+ demux_priv_t *priv = (demux_priv_t *)demux;
>+ return priv->control( demux, query, args );
>+}
>+
>+static int demux_vaControlInner( demux_t *demux, int query, va_list
>args )
>+{
>+ if( demux->s != NULL )
>+ switch( query )
>+ {
>+ /* Legacy fallback for missing getters in synchronous
>demuxers */
>+ case DEMUX_CAN_PAUSE:
>+ case DEMUX_CAN_CONTROL_PACE:
>+ case DEMUX_GET_PTS_DELAY:
>+ {
>+ int ret;
>+ va_list ap;
>+
>+ va_copy( ap, args );
>+ ret = demux->pf_control( demux, query, args );
>+ if( ret != VLC_SUCCESS )
>+ ret = vlc_stream_vaControl( demux->s, query, ap );
>+ va_end( ap );
>+ return ret;
>+ }
>+
>+ /* Some demuxers need to control pause directly (e.g.
>adaptive),
>+ * but many legacy demuxers do not understand pause at
>all.
>+ * If DEMUX_CAN_PAUSE is not implemented, bypass the
>demuxer and
>+ * byte stream. If DEMUX_CAN_PAUSE is implemented and
>pause is
>+ * supported, pause the demuxer normally. Else, something
>went very
>+ * wrong.
>+ *
>+ * Note that this requires asynchronous/threaded demuxers
>to
>+ * always return VLC_SUCCESS for DEMUX_CAN_PAUSE, so that
>they are
>+ * never bypassed. Otherwise, we would reenter demux->s
>callbacks
>+ * and break thread safety. At the time of writing,
>asynchronous or
>+ * threaded *non-access* demuxers do not exist and are not
>fully
>+ * supported by the input thread, so this is theoretical.
>*/
>+ case DEMUX_SET_PAUSE_STATE:
>+ {
>+ bool can_pause;
>+
>+ if( demux_ControlInternal( demux, DEMUX_CAN_PAUSE,
>+ &can_pause ) )
>+ return vlc_stream_vaControl( demux->s, query, args
>);
>+
>+ /* The caller shall not pause if pause is unsupported.
>*/
>+ assert( can_pause );
>+ break;
>+ }
>+ }
>+
>+ return demux->pf_control( demux, query, args );
>+}
>+
>+static int demux_vaControlFilter( demux_t *demux, int query, va_list
>args )
>+{
>+ // Do not probe the stream in the context of a demux filter.
>+ return demux->pf_control( demux, query, args );
>+}
>+
>/*****************************************************************************
> * demux_NewAdvanced:
> * if s is NULL then load a access_demux
>@@ -245,6 +320,7 @@ demux_t *demux_NewAdvanced( vlc_object_t *p_obj,
>input_thread_t *p_parent_input,
> p_demux->p_sys = NULL;
> p_demux->info.i_update = 0;
> priv->destroy = s ? demux_DestroyDemux : demux_DestroyAccessDemux;
>+ priv->control = demux_vaControlInner;
>
> if( s != NULL )
> {
>@@ -303,68 +379,6 @@ void demux_Delete( demux_t *p_demux )
> #define static_control_match(foo) \
> static_assert((unsigned) DEMUX_##foo == STREAM_##foo, "Mismatch")
>
>-static int demux_ControlInternal( demux_t *demux, int query, ... )
>-{
>- int ret;
>- va_list ap;
>-
>- va_start( ap, query );
>- ret = demux->pf_control( demux, query, ap );
>- va_end( ap );
>- return ret;
>-}
>-
>-int demux_vaControl( demux_t *demux, int query, va_list args )
>-{
>- if( demux->s != NULL )
>- switch( query )
>- {
>- /* Legacy fallback for missing getters in synchronous
>demuxers */
>- case DEMUX_CAN_PAUSE:
>- case DEMUX_CAN_CONTROL_PACE:
>- case DEMUX_GET_PTS_DELAY:
>- {
>- int ret;
>- va_list ap;
>-
>- va_copy( ap, args );
>- ret = demux->pf_control( demux, query, args );
>- if( ret != VLC_SUCCESS )
>- ret = vlc_stream_vaControl( demux->s, query, ap );
>- va_end( ap );
>- return ret;
>- }
>-
>- /* Some demuxers need to control pause directly (e.g.
>adaptive),
>- * but many legacy demuxers do not understand pause at
>all.
>- * If DEMUX_CAN_PAUSE is not implemented, bypass the
>demuxer and
>- * byte stream. If DEMUX_CAN_PAUSE is implemented and
>pause is
>- * supported, pause the demuxer normally. Else, something
>went very
>- * wrong.
>- *
>- * Note that this requires asynchronous/threaded demuxers
>to
>- * always return VLC_SUCCESS for DEMUX_CAN_PAUSE, so that
>they are
>- * never bypassed. Otherwise, we would reenter demux->s
>callbacks
>- * and break thread safety. At the time of writing,
>asynchronous or
>- * threaded *non-access* demuxers do not exist and are not
>fully
>- * supported by the input thread, so this is theoretical.
>*/
>- case DEMUX_SET_PAUSE_STATE:
>- {
>- bool can_pause;
>-
>- if( demux_ControlInternal( demux, DEMUX_CAN_PAUSE,
>- &can_pause ) )
>- return vlc_stream_vaControl( demux->s, query, args
>);
>-
>- /* The caller shall not pause if pause is unsupported.
>*/
>- assert( can_pause );
>- break;
>- }
>- }
>-
>- return demux->pf_control( demux, query, args );
>-}
>-
>/*****************************************************************************
> * demux_vaControlHelper:
>*****************************************************************************/
>@@ -593,6 +607,7 @@ static demux_t *demux_FilterNew( demux_t *p_next,
>const char *p_name )
> p_demux->psz_filepath = NULL;
> p_demux->out = NULL;
> priv->destroy = demux_DestroyDemuxFilter;
>+ priv->control = demux_vaControlFilter;
> p_demux->p_module =
> module_need( p_demux, "demux_filter", p_name, p_name != NULL );
>
>--
>2.11.0
>
>_______________________________________________
>vlc-devel mailing list
>To unsubscribe or modify your subscription options:
>https://mailman.videolan.org/listinfo/vlc-devel
This is UB even with the patch, only much more needlessly complicated. Nack.
We have been through this timed and times already.
--
Remi Denis-Courmont
More information about the vlc-devel
mailing list