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.
>
> - Hendrik
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
