[FFmpeg-devel] [PATCH] avcodec/snowdec: Check intra block dc differences.

[Michael Niedermayer]

On Tue, Sep 05, 2017 at 07:13:00AM -0400, Ronald S. Bultje wrote:
> Hi,
> 
> On Mon, Sep 4, 2017 at 10:11 PM, James Almer <jamrial at gmail.com> wrote:
> 
> > On 9/4/2017 11:00 PM, Ronald S. Bultje wrote:
> > > Hi,
> > >
> > > On Mon, Sep 4, 2017 at 12:11 PM, Michael Niedermayer
> > <michael at niedermayer.cc
> > >> wrote:
> > >
> > >>          if(type){
> > >> +            int ld, cbd, crd;
> > >>              pred_mv(s, &mx, &my, 0, left, top, tr);
> > >> -            l += get_symbol(&s->c, &s->block_state[32], 1);
> > >> +            ld = get_symbol(&s->c, &s->block_state[32], 1);
> > >> +            if (ld < -255 || ld > 255) {
> > >> +                av_log(s->avctx, AV_LOG_ERROR, "Invalid (Out of range)
> > >> intra luma block DC difference %d\n", ld);
> > >> +                return AVERROR_INVALIDDATA;
> > >> +            }
> > >> +            l += ld;
> > >>              if (s->nb_planes > 2) {
> > >> -                cb+= get_symbol(&s->c, &s->block_state[64], 1);
> > >> -                cr+= get_symbol(&s->c, &s->block_state[96], 1);
> > >> +                cbd = get_symbol(&s->c, &s->block_state[64], 1);
> > >> +                crd = get_symbol(&s->c, &s->block_state[96], 1);
> > >> +                if (cbd < -255 || cbd > 255 || crd < -255 || crd >
> > 255) {
> > >> +                    av_log(s->avctx, AV_LOG_ERROR, "Invalid (Out of
> > >> range) intra chroma block DC difference %d, %d\n", cbd, crd);
> > >> +                    return AVERROR_INVALIDDATA;
> > >> +                }
> > >> +                cb += cbd;
> > >> +                cr += crd;
> > >>              }
> > >
> > >
> > > I recognize the great improvements in your messages. They are much better
> > > than before. They are still not appropriate for display to end users.
> > > Please use ff_tlog(), as was suggested in the original thread.
> >
> > Now that i looked at ff_tlog() closely it's only enabled if -DTRACE is
> > used during compilation, and then it prints stuff at the trace log
> > level, which is even lower than debug and used to print a bunch of
> > assorted values that are not error log messages.
> 
> 
> You're right, ff_dlog() seems better (see libavcodec/internal.h).

ff_dlog() is not part of a user build so a users bug report will not
contain it at any verbose / debug level.

If that is what the community wants, we can of course go that way.
My oppinion is that its a bad idea for the future maintainability
of the project to use ff_dlog() widely for error details.
But its the communities project and neither mine nor yours.
Its also the community that will have to live with the resulting
codebase and set of bug reports ...


[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

The worst form of inequality is to try to make unequal things equal.
-- Aristotle
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20170906/637cb587/attachment.sig>