Message ID | 1476979574-53380-1-git-send-email-rsbultje@gmail.com |
---|---|
State | Accepted |
Headers | show |
On 20/10/2016 17:06, Ronald S. Bultje wrote: > --- > libavfilter/vf_colorspace.c | 12 ++++++++++-- > 1 file changed, 10 insertions(+), 2 deletions(-) > > diff --git a/libavfilter/vf_colorspace.c b/libavfilter/vf_colorspace.c > index c74fe00..f64163f 100644 > --- a/libavfilter/vf_colorspace.c > +++ b/libavfilter/vf_colorspace.c > @@ -163,6 +163,8 @@ typedef struct ColorSpaceContext { > yuv2yuv_fn yuv2yuv; > double yuv2rgb_dbl_coeffs[3][3], rgb2yuv_dbl_coeffs[3][3]; > int in_y_rng, in_uv_rng, out_y_rng, out_uv_rng; > + > + int did_range_warn; > } ColorSpaceContext; > > // FIXME deal with odd width/heights (or just forbid it) > @@ -523,9 +525,15 @@ static int get_range_off(AVFilterContext *ctx, int *off, > enum AVColorRange rng, int depth) > { > switch (rng) { > - case AVCOL_RANGE_UNSPECIFIED: > - av_log(ctx, AV_LOG_WARNING, "Input range not set, assuming tv/mpeg\n"); > + case AVCOL_RANGE_UNSPECIFIED: { > + ColorSpaceContext *s = ctx->priv; > + > + if (!s->did_range_warn) { > + av_log(ctx, AV_LOG_WARNING, "Input range not set, assuming tv/mpeg\n"); > + s->did_range_warn = 1; > + } > // fall-through > + } > case AVCOL_RANGE_MPEG: > *off = 16 << (depth - 8); > *y_rng = 219 << (depth - 8); > I think did_warn_range would sound more natural. LGTM otherwise (with or without the change).
Hi, On Thu, Oct 20, 2016 at 1:08 PM, Josh de Kock <josh@itanimul.li> wrote: > On 20/10/2016 17:06, Ronald S. Bultje wrote: > >> --- >> libavfilter/vf_colorspace.c | 12 ++++++++++-- >> 1 file changed, 10 insertions(+), 2 deletions(-) >> >> diff --git a/libavfilter/vf_colorspace.c b/libavfilter/vf_colorspace.c >> index c74fe00..f64163f 100644 >> --- a/libavfilter/vf_colorspace.c >> +++ b/libavfilter/vf_colorspace.c >> @@ -163,6 +163,8 @@ typedef struct ColorSpaceContext { >> yuv2yuv_fn yuv2yuv; >> double yuv2rgb_dbl_coeffs[3][3], rgb2yuv_dbl_coeffs[3][3]; >> int in_y_rng, in_uv_rng, out_y_rng, out_uv_rng; >> + >> + int did_range_warn; >> } ColorSpaceContext; >> >> // FIXME deal with odd width/heights (or just forbid it) >> @@ -523,9 +525,15 @@ static int get_range_off(AVFilterContext *ctx, int >> *off, >> enum AVColorRange rng, int depth) >> { >> switch (rng) { >> - case AVCOL_RANGE_UNSPECIFIED: >> - av_log(ctx, AV_LOG_WARNING, "Input range not set, assuming >> tv/mpeg\n"); >> + case AVCOL_RANGE_UNSPECIFIED: { >> + ColorSpaceContext *s = ctx->priv; >> + >> + if (!s->did_range_warn) { >> + av_log(ctx, AV_LOG_WARNING, "Input range not set, assuming >> tv/mpeg\n"); >> + s->did_range_warn = 1; >> + } >> // fall-through >> + } >> case AVCOL_RANGE_MPEG: >> *off = 16 << (depth - 8); >> *y_rng = 219 << (depth - 8); >> >> > I think did_warn_range would sound more natural. LGTM otherwise (with or > without the change). Changed locally, thanks. I'll leave it out for a couple more hours before pushing so others have time to comment too. Ronald
On Thu, Oct 20, 2016 at 12:06:14 -0400, Ronald S. Bultje wrote: > + s->did_range_warn = 1; > + } > // fall-through > + } > case AVCOL_RANGE_MPEG: The fall-through comment seems misplaced now (both logically and probably for Coverity). Moritz
Hi, On Fri, Oct 21, 2016 at 4:36 PM, Moritz Barsnick <barsnick@gmx.net> wrote: > On Thu, Oct 20, 2016 at 12:06:14 -0400, Ronald S. Bultje wrote: > > + s->did_range_warn = 1; > > + } > > // fall-through > > + } > > case AVCOL_RANGE_MPEG: > > The fall-through comment seems misplaced now (both logically and > probably for Coverity). Is there documentation on what coverity expects? Ronald
On Sat, Oct 22, 2016 at 08:43:26 -0400, Ronald S. Bultje wrote:
> Is there documentation on what coverity expects?
I found this:
https://lost-contact.mit.edu/afs/cs.stanford.edu/pkg/prevent-4.3.1/i386_linux26/opt/prevent-linux-4.3.1/cgi-bin/doc/checker_ref.html#c_checker_MISSING_BREAK
Moritz
Hi, On Sat, Oct 22, 2016 at 12:19 PM, Moritz Barsnick <barsnick@gmx.net> wrote: > On Sat, Oct 22, 2016 at 08:43:26 -0400, Ronald S. Bultje wrote: > > Is there documentation on what coverity expects? > > I found this: > https://lost-contact.mit.edu/afs/cs.stanford.edu/pkg/ > prevent-4.3.1/i386_linux26/opt/prevent-linux-4.3.1/cgi- > bin/doc/checker_ref.html#c_checker_MISSING_BREAK I was hoping for documentation on what it expects, not what can be changed about it :) Anyway, I'll change it, I don't really care. Ronald
Hi, On Sat, Oct 22, 2016 at 10:02 PM, Ronald S. Bultje <rsbultje@gmail.com> wrote: > Hi, > > On Sat, Oct 22, 2016 at 12:19 PM, Moritz Barsnick <barsnick@gmx.net> > wrote: > >> On Sat, Oct 22, 2016 at 08:43:26 -0400, Ronald S. Bultje wrote: >> > Is there documentation on what coverity expects? >> >> I found this: >> https://lost-contact.mit.edu/afs/cs.stanford.edu/pkg/prevent >> -4.3.1/i386_linux26/opt/prevent-linux-4.3.1/cgi-bin/ >> doc/checker_ref.html#c_checker_MISSING_BREAK > > > I was hoping for documentation on what it expects, not what can be changed > about it :) Anyway, I'll change it, I don't really care. > Pushed with that modification. Ronald
On Sat, Oct 22, 2016 at 22:02:15 -0400, Ronald S. Bultje wrote: > I was hoping for documentation on what it expects, not what can be changed > about it :) Anyway, I'll change it, I don't really care. I had found this to be quite clear: > End with a comment. The checker assumes that this comment is > acknowledging a fallthrough. The comment can start anywhere on the > last line, or be a multi-line C comment. So: It expects a comment (*any* comment) on the last line (which, to me, is the exact line preceding the next case label). Sorry for the late remark, Moritz
diff --git a/libavfilter/vf_colorspace.c b/libavfilter/vf_colorspace.c index c74fe00..f64163f 100644 --- a/libavfilter/vf_colorspace.c +++ b/libavfilter/vf_colorspace.c @@ -163,6 +163,8 @@ typedef struct ColorSpaceContext { yuv2yuv_fn yuv2yuv; double yuv2rgb_dbl_coeffs[3][3], rgb2yuv_dbl_coeffs[3][3]; int in_y_rng, in_uv_rng, out_y_rng, out_uv_rng; + + int did_range_warn; } ColorSpaceContext; // FIXME deal with odd width/heights (or just forbid it) @@ -523,9 +525,15 @@ static int get_range_off(AVFilterContext *ctx, int *off, enum AVColorRange rng, int depth) { switch (rng) { - case AVCOL_RANGE_UNSPECIFIED: - av_log(ctx, AV_LOG_WARNING, "Input range not set, assuming tv/mpeg\n"); + case AVCOL_RANGE_UNSPECIFIED: { + ColorSpaceContext *s = ctx->priv; + + if (!s->did_range_warn) { + av_log(ctx, AV_LOG_WARNING, "Input range not set, assuming tv/mpeg\n"); + s->did_range_warn = 1; + } // fall-through + } case AVCOL_RANGE_MPEG: *off = 16 << (depth - 8); *y_rng = 219 << (depth - 8);