On Mon, May 23, 2016 at 1:59 PM, Michael Niedermayer
<mich...@niedermayer.cc> wrote:
> On Mon, May 23, 2016 at 07:24:23AM -0400, Ronald S. Bultje wrote:
>> Hi,
>>
>> On Sun, May 22, 2016 at 11:39 PM, Michael Niedermayer <
>> mich...@niedermayer.cc> wrote:
>>
>> > On Sun, May 22, 2016 at 01:51:05AM +0000, Davinder Singh wrote:
>> > > On Sun, May 22, 2016 at 2:09 AM Michael Niedermayer
>> > <mich...@niedermayer.cc>
>> > > wrote:
>> > >
>> > > > On Sat, May 21, 2016 at 02:21:17PM +0000, Davinder Singh wrote:
>> > > > > hi,
>> > > > >
>> > > > > this patch fixes following compiler warnings:
>> > > > >
>> > > > > libavcodec/cfhd.c:346:78: warning: format specifies type 'unsigned
>> > short'
>> > > > > but the argument has type 'int' [-Wformat]
>> > > > > av_log(avctx, AV_LOG_DEBUG, "Small chunk length %"PRIu16"
>> > > > > %s\n", data * 4, tag < 0 ? "optional" : "required");
>> > > > > ~~
>> > > > > ^~~~~~~~
>> > > > > libavcodec/cfhd.c:472:110: warning: format specifies type 'unsigned
>> > > > short'
>> > > > > but the argument has type 'int' [-Wformat]
>> > > > > av_log(avctx, AV_LOG_DEBUG, "Start of lowpass coeffs
>> > > > component
>> > > > > %"PRIu16" height:%d, width:%d\n", s->channel_num, lowpass_height,
>> > > > > lowpass_width);
>> > > > >
>> > > > > ~~ ^~~~~~~~~~~~~~
>> > > > > libavcodec/cfhd.c:490:77: warning: format specifies type 'unsigned
>> > short'
>> > > > > but the argument has type 'int' [-Wformat]
>> > > > > av_log(avctx, AV_LOG_DEBUG, "Lowpass coefficients
>> > > > %"PRIu16"\n",
>> > > > > lowpass_width * lowpass_height);
>> > > > > ~~
>> > > > > ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> > > > >
>> > > > >
>> > > > >
>> > > > > libavcodec/dv_tablegen.c:30:60: warning: format specifies type
>> > 'char' but
>> > > > > the argument has type 'uint32_t' (aka 'unsigned int') [-Wformat]
>> > > > > "{0x%"PRIx32", %"PRId8"}", data[i].vlc,
>> > data[i].size)
>> > > > >
>> > ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~
>> > > > > libavcodec/tableprint.h:37:29: note: expanded from macro
>> > > > > 'WRITE_1D_FUNC_ARGV'
>> > > > > printf(" "fmtstr",", __VA_ARGS__);\
>> > > > > ^~~~~~~~~~~
>> > > > > libavcodec/dv_tablegen.c:30:60: warning: format specifies type
>> > 'char' but
>> > > > > the argument has type 'uint32_t' (aka 'unsigned int') [-Wformat]
>> > > > > "{0x%"PRIx32", %"PRId8"}", data[i].vlc,
>> > data[i].size)
>> > > > >
>> > ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~
>> > > > >
>> > > > >
>> > > > >
>> > > > > libavfilter/af_hdcd.c:896:57: warning: shifting a negative signed
>> > value
>> > > > is
>> > > > > undefined [-Wshift-negative-value]
>> > > > > state->readahead = readaheadtab[bits & ~(-1 << 8)];
>> > > > >
>> > > > >
>> > > > >
>> > > > > libavfilter/vf_hwdownload.c:59:5: warning: ignoring return value of
>> > > > > function declared with warn_unused_result attribute [-Wunused-result]
>> > > > > ff_formats_ref(infmts, &avctx->inputs[0]->out_formats);
>> > > > > ^~~~~~~~~~~~~~ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> > > > > libavfilter/vf_hwdownload.c:60:5: warning: ignoring return value of
>> > > > > function declared with warn_unused_result attribute [-Wunused-result]
>> > > > > ff_formats_ref(outfmts, &avctx->outputs[0]->in_formats);
>> > > > > ^~~~~~~~~~~~~~ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> > > > >
>> > > > >
>> > > > >
>> > > > > libavutil/opencl.c:456:17: warning: variable 'kernel_source' is used
>> > > > > uninitialized whenever 'for' loop exits because its condition is
>> > false
>> > > > > [-Wsometimes-uninitialized]
>> > > > > for (i = 0; i < opencl_ctx.kernel_code_count; i++) {
>> > > > > ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> > > > > libavutil/opencl.c:466:10: note: uninitialized use occurs here
>> > > > > if (!kernel_source) {
>> > > > > ^~~~~~~~~~~~~
>> > > > > libavutil/opencl.c:456:17: note: remove the condition if it is always
>> > > > true
>> > > > > for (i = 0; i < opencl_ctx.kernel_code_count; i++) {
>> > > > > ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> > > > > libavutil/opencl.c:448:30: note: initialize the variable
>> > 'kernel_source'
>> > > > to
>> > > > > silence this warning
>> > > > > const char *kernel_source;
>> > > > > ^
>> > > > > = NULL
>> > > >
>> > > > > libavcodec/cfhd.c | 6 +++---
>> > > > > libavcodec/dv_tablegen.c | 2 +-
>> > > > > libavfilter/af_hdcd.c | 2 +-
>> > > > > libavfilter/vf_hwdownload.c | 6 ++++--
>> > > > > libavutil/opencl.c | 2 +-
>> > > >
>> > > > please split this patch
>> > > > the fixed warnings are unrelated to each other and possibly differnt
>> > > > developers would like to reply to different parts
>> > > >
>> > > > [...]
>> > > >
>> > > > --
>> > > > Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>> > > >
>> > > > I have often repented speaking, but never of holding my tongue.
>> > > > -- Xenocrates
>> > > > _______________________________________________
>> > > > ffmpeg-devel mailing list
>> > > > ffmpeg-devel@ffmpeg.org
>> > > > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>> > > >
>> >
>> > > af_hdcd.c | 2 +-
>> > > 1 file changed, 1 insertion(+), 1 deletion(-)
>> > > 0ff76093ae99c1ef8ae87b70d8bf5b6ef92c43b9
>> > 0003-libavfilter-af_hdcd-fixed-negative-signed-value-shif.patch
>> > > From c498d1a86f3cdbed94cc8bc4a9af7c87af03b275 Mon Sep 17 00:00:00 2001
>> > > From: dsmudhar <ds.mud...@gmail.com>
>> > > Date: Sun, 22 May 2016 06:18:58 +0530
>> > > Subject: [PATCH 3/7] libavfilter/af_hdcd: fixed negative signed value
>> > shift
>> > >
>> > > ---
>> > > libavfilter/af_hdcd.c | 2 +-
>> > > 1 file changed, 1 insertion(+), 1 deletion(-)
>> > >
>> > > diff --git a/libavfilter/af_hdcd.c b/libavfilter/af_hdcd.c
>> > > index 041b89f..fbf0836 100644
>> > > --- a/libavfilter/af_hdcd.c
>> > > +++ b/libavfilter/af_hdcd.c
>> > > @@ -893,7 +893,7 @@ static int integrate(hdcd_state_t *state, int *flag,
>> > const int32_t *samples, int
>> > > state->code_counterC++;
>> > > } else {
>> > > if (bits)
>> > > - state->readahead = readaheadtab[bits & ~(-1 << 8)];
>> > > + state->readahead = readaheadtab[bits & ~((unsigned) -1 <<
>> > 8)];
>> > > else
>> >
>> > applied
>>
>>
>> This is very obfuscated, why doesn't this just say bits & 0xff?
>
> i dont know that
> maybe the author of the original "& ~(-1 << 8)" or new code had a
> reason to prefer this style
>
> also obfuscation is in the eye of the beholder,
> i would guess 0xff is only clearer if you know what it is in binary
> if you dont then explicitly building it from a shift by 8 is clearer
> i suspect
>
So he wouldn't know what 0xff is, but he does know what casting -1 to
unsigned produces?
Seems like an odd argument to me. 0xff is far clearer than some odd
shift and not construct.
- Hendrik
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
