[vlc-devel] [PATCH 07/28] core: change var_InheritURational to output a vlc_rational_t

[Steve Lhomme]

On Mon, Apr 3, 2017 at 9:52 AM, Filip Roséen <filip at atch.se> wrote:
> Hi Steve,
>
> On 2017-04-03 09:47, Steve Lhomme wrote:
>
>  On Sun, Apr 2, 2017 at 11:24 PM, Filip Roséen <filip at atch.se> wrote:
>
>  On 2017-03-31 18:14, Steve Lhomme wrote:
>
>   ---
>    include/vlc_variables.h                  |  4 ++--
>    modules/access/decklink.cpp              |  2 +-
>    modules/access/imem.c                    |  4 ++--
>    modules/access/timecode.c                |  3 +--
>    modules/access/v4l2/video.c              |  9 ++++-----
>    modules/demux/image.c                    |  4 ++--
>    modules/demux/rawvid.c                   |  5 ++---
>    modules/stream_out/transcode/transcode.c |  2 +-
>    modules/video_filter/fps.c               |  2 +-
>    src/misc/variables.c                     | 16 ++++++++--------
>    src/video_output/video_output.c          |  3 +--
>    test/src/misc/variables.c                | 32
>  ++++++++++++++++----------------
>    12 files changed, 41 insertions(+), 45 deletions(-)
>
>   diff --git a/include/vlc_variables.h b/include/vlc_variables.h
>   index 83752b171f..59ad99367c 100644
>   --- a/include/vlc_variables.h
>   +++ b/include/vlc_variables.h
>   @@ -652,8 +652,8 @@ static inline void *var_InheritAddress( vlc_object_t
>  *obj, const char *name )
>    }
>    #define var_InheritAddress(o, n) var_InheritAddress(VLC_OBJECT(o), n)
>
>   -VLC_API int var_InheritURational( vlc_object_t *, unsigned *num, unsigned
>  *den, const char *var );
>   -#define var_InheritURational(a,b,c,d) var_InheritURational(VLC_OBJECT(a),
>  b, c, d)
>   +VLC_API int var_InheritURational( vlc_object_t *, vlc_rational_t *dst,
>  const char *var );
>   +#define var_InheritURational(a,b,d) var_InheritURational(VLC_OBJECT(a),
> b,
>  d)
>
>    #define var_GetInteger(a,b)   var_GetInteger( VLC_OBJECT(a),b)
>    #define var_GetBool(a,b)   var_GetBool( VLC_OBJECT(a),b)
>   diff --git a/modules/access/decklink.cpp b/modules/access/decklink.cpp
>   index c4c043559b..248009ed15 100644
>   --- a/modules/access/decklink.cpp
>   +++ b/modules/access/decklink.cpp
>   @@ -197,7 +197,7 @@ static es_format_t GetModeSettings(demux_t *demux,
>  IDeckLinkDisplayMode *m)
>        video_fmt.i_bitrate = video_fmt.video.i_width *
>  video_fmt.video.i_height * video_fmt.video.i_frame_rate * 2 * 8;
>
>        vlc_rational_t aspect;
>   -    if (!var_InheritURational(demux, &aspect.num, &aspect.den,
>  "decklink-aspect-ratio") &&
>   +    if (!var_InheritURational(demux, &aspect, "decklink-aspect-ratio") &&
>             aspect_num > 0 && aspect_den > 0) {
>
>  The above leaves usage of aspect_num and aspect_den, even though these are
>  no longer written to by var_InheritURational - which cannot be correct.
>
>  ??? These variables went away in the previous patch.
>
> Then how come they are still present in the above diff? I am not sure if
> those “???” are directed to me or something else, though.

This is patch 7. In patch 6 you have the following:

> -    unsigned aspect_num, aspect_den;
> -    if (!var_InheritURational(demux, &aspect_num, &aspect_den, "decklink-aspect-ratio") &&
> +    vlc_rational_t aspect;
> +    if (!var_InheritURational(demux, &aspect.num, &aspect.den, "decklink-aspect-ratio") &&
>           aspect_num > 0 && aspect_den > 0) {
> -        video_fmt.video.i_sar_num = aspect_num * video_fmt.video.i_height;
> -        video_fmt.video.i_sar_den = aspect_den * video_fmt.video.i_width;
> +        video_fmt.video.i_sar_num = aspect.num * video_fmt.video.i_height;
> +        video_fmt.video.i_sar_den = aspect.den * video_fmt.video.i_width;
>      }

So I have no idea what you mean by "Then how come they are still
present in the above diff"

You may confuse aspect_num and aspect.num.

> As already stated, I have not applied your patches locally nor tried to
> compile them - but the diff certainly states that aspect_num > 0 and
> aspect_den > 0 are still there.
>
> Best Regards,
> Filip
>
>
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel