[vlc-devel] [PATCH] avcommon: refactor usage of `quiet' variable

[Marvin Scholz]

On 12 Dec 2019, at 10:01, Alexandre Janniaux wrote:

> The `quiet' VLC variable is used although it is exposed only by other
> non-mandatory modules. Rename it to avcodec/avformat-quiet so as to
> prevent assertion on platform that won't ship with a module exposing a
> quiet variable.

Hi, thanks for the patch!

While this approach works to fix the issue and seems certainly like a 
possible
solution to me, I think it would be more convenient to have just one 
`quiet`
variable and not a per-module one. So maybe the variable should be 
„owned“ by
the core instead?

Although it seems we already have several module-specific quiet options
so maybe we want to keep it that way even though to me it seems weird…

> ---
>  modules/codec/avcodec/avcodec.c   |  6 ++++++
>  modules/codec/avcodec/avcommon.h  | 12 ++++++++----
>  modules/demux/avformat/avformat.c |  5 +++++
>  3 files changed, 19 insertions(+), 4 deletions(-)
>
> diff --git a/modules/codec/avcodec/avcodec.c 
> b/modules/codec/avcodec/avcodec.c
> index ad2201cb1f..843a076162 100644
> --- a/modules/codec/avcodec/avcodec.c
> +++ b/modules/codec/avcodec/avcodec.c
> @@ -71,6 +71,9 @@ static const char *const enc_hq_list_text[] = {
>          "delivered by the FFmpeg library. This includes (MS)MPEG4, 
> DivX, SV1,"\
>          "H261, H263, H264, WMV, WMA, AAC, AMR, DV, MJPEG and other 
> codecs")
>
> +#define AVCODEC_QUIET_TEXT "Force quiet mode"
> +#define AVCODEC_QUIET_LONGTEXT "Force avcodec logs to quiet mode"
> +
>  vlc_module_begin ()
>      set_shortname( "FFmpeg")
>      set_category( CAT_INPUT )
> @@ -80,6 +83,9 @@ vlc_module_begin ()
>      set_help( MODULE_DESCRIPTION )
>      set_section( N_("Decoding") , NULL )
>
> +    add_bool("avcodec-quiet", false, AVCODEC_QUIET_TEXT,
> +             AVCODEC_QUIET_LONGTEXT, true)
> +
>      add_shortcut("ffmpeg")
>      set_capability("video decoder", 70)
>      set_callbacks(InitVideoDec, EndVideoDec)
> diff --git a/modules/codec/avcodec/avcommon.h 
> b/modules/codec/avcodec/avcommon.h
> index 1962162a7e..7d6bd7a473 100644
> --- a/modules/codec/avcodec/avcommon.h
> +++ b/modules/codec/avcodec/avcommon.h
> @@ -76,11 +76,11 @@ static inline void vlc_av_get_options(const char 
> *psz_opts, AVDictionary** pp_di
>      }
>  }
>
> -static inline void vlc_init_avutil(vlc_object_t *obj)
> +static inline void vlc_init_avutil(vlc_object_t *obj, bool quiet)
>  {
>      int level = AV_LOG_QUIET;
>
> -    if (!var_InheritBool(obj, "quiet")) {
> +    if (!quiet) {
>          int64_t verbose = var_InheritInteger(obj, "verbose");
>          if (verbose >= 0) switch(verbose + VLC_MSG_ERR) {
>          case VLC_MSG_ERR:
> @@ -112,9 +112,11 @@ static inline void vlc_init_avutil(vlc_object_t 
> *obj)
>  # include <libavformat/avformat.h>
>  static inline void vlc_init_avformat(vlc_object_t *obj)
>  {
> +    bool quiet = var_InheritBool(obj, "avformat-quiet");
> +
>      vlc_avcodec_lock();
>
> -    vlc_init_avutil(obj);
> +    vlc_init_avutil(obj, quiet);
>
>      avformat_network_init();
>
> @@ -128,9 +130,11 @@ static inline void vlc_init_avformat(vlc_object_t 
> *obj)
>  # include <libavcodec/avcodec.h>
>  static inline void vlc_init_avcodec(vlc_object_t *obj)
>  {
> +    bool quiet = var_InheritBool(obj, "avcodec-quiet");
> +
>      vlc_avcodec_lock();
>
> -    vlc_init_avutil(obj);
> +    vlc_init_avutil(obj, quiet);
>
>      avcodec_register_all();
>
> diff --git a/modules/demux/avformat/avformat.c 
> b/modules/demux/avformat/avformat.c
> index ac34a377c5..b47fcc9294 100644
> --- a/modules/demux/avformat/avformat.c
> +++ b/modules/demux/avformat/avformat.c
> @@ -32,6 +32,9 @@
>  #include "avformat.h"
>  #include "../../codec/avcodec/avcommon.h"
>
> +#define AVFORMAT_QUIET_TEXT "Force quiet mode"
> +#define AVFORMAT_QUIET_LONGTEXT "Force avformat logs to quiet mode"
> +
>  vlc_module_begin ()
>  #endif /* MERGE_FFMPEG */
>      add_shortcut( "ffmpeg", "avformat" )
> @@ -45,6 +48,8 @@ vlc_module_begin ()
>      add_string( "avformat-format", NULL, FORMAT_TEXT, 
> FORMAT_LONGTEXT, true )
>      add_obsolete_string("ffmpeg-format") /* removed since 2.1.0 */
>      add_string( "avformat-options", NULL, AV_OPTIONS_TEXT, 
> AV_OPTIONS_LONGTEXT, true )
> +    add_bool( "avformat-quiet", false, AVFORMAT_QUIET_TEXT,
> +               AVFORMAT_QUIET_LONGTEXT, true )
>
>  #ifdef ENABLE_SOUT
>      /* mux submodule */
> -- 
> 2.24.1
>
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel