Hi, On Thu, Sep 17, 2015 at 7:03 AM, Ganesh Ajjanagadde <gajjanaga...@gmail.com> wrote:
> On Thu, Sep 17, 2015 at 6:15 AM, Hendrik Leppkes <h.lepp...@gmail.com> > wrote: > > On Thu, Sep 17, 2015 at 12:19 AM, Ganesh Ajjanagadde > > <gajjanaga...@gmail.com> wrote: > >> This patch moves the pointer validity check outside the macro, > >> and silences the -Waddress observed with GCC 5.2. > >> > >> Signed-off-by: Ganesh Ajjanagadde <gajjanaga...@gmail.com> > >> --- > >> libavcodec/libx264.c | 8 +++++--- > >> 1 file changed, 5 insertions(+), 3 deletions(-) > >> > >> diff --git a/libavcodec/libx264.c b/libavcodec/libx264.c > >> index 58fcfb0..c7c772e 100644 > >> --- a/libavcodec/libx264.c > >> +++ b/libavcodec/libx264.c > >> @@ -346,7 +346,7 @@ static av_cold int X264_close(AVCodecContext *avctx) > >> #define OPT_STR(opt, param) > \ > >> do { > \ > >> int ret; > \ > >> - if (param && (ret = x264_param_parse(&x4->params, opt, param)) > < 0) { \ > >> + if ((ret = x264_param_parse(&x4->params, opt, param)) < 0) { \ > >> if(ret == X264_PARAM_BAD_NAME) > \ > >> av_log(avctx, AV_LOG_ERROR, > \ > >> "bad option '%s': '%s'\n", opt, param); > \ > >> @@ -437,7 +437,8 @@ static av_cold int X264_init(AVCodecContext *avctx) > >> x4->params.i_log_level = X264_LOG_DEBUG; > >> x4->params.i_csp = convert_pix_fmt(avctx->pix_fmt); > >> > >> - OPT_STR("weightp", x4->wpredp); > >> + if (x4->wpredp) > >> + OPT_STR("weightp", x4->wpredp); > >> > >> if (avctx->bit_rate) { > >> x4->params.rc.i_bitrate = avctx->bit_rate / 1000; > >> @@ -467,7 +468,8 @@ static av_cold int X264_init(AVCodecContext *avctx) > >> (float)avctx->rc_initial_buffer_occupancy / > avctx->rc_buffer_size; > >> } > >> > >> - OPT_STR("level", x4->level); > >> + if (x4->level) > >> + OPT_STR("level", x4->level); > >> > >> if (avctx->i_quant_factor > 0) > >> x4->params.rc.f_ip_factor = 1 / > fabs(avctx->i_quant_factor); > > > > > > Instead of adding explicit checks here, why not make the file more > > consistent and use PARSE_X264_OPT for the things from the x4 context > > (like its already done for a bunch of other variables), and only use > > OPT_STR for the two special cases further down (without the check > > then) > > The behavior then won't be identical before and after the patch; e.g > the log portion of PARSE_X264_OPT is different from that of OPT_STR. > The current patch retains identical behavior. In particular, your > change does change the "user-facing" output slightly. Unless you (or > someone else) can confirm that it is irrelevant; I do not think your > proposal is good. How does it change? I don't think that's necessarily a terrible thing. If it's just cosmetic, we can live with it. Ronald _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel