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

Michael Niedermayer michael at niedermayer.cc
Fri Nov 17 17:14:18 EET 2017


On Thu, Nov 16, 2017 at 11:13:27PM -0300, James Almer wrote:
> On 11/16/2017 10:43 PM, Michael Niedermayer wrote:
> > On Thu, Nov 16, 2017 at 07:47:55PM -0500, Ronald S. Bultje wrote:
> >> Hi,
> >>
> >> On Thu, Nov 16, 2017 at 4:41 PM, Michael Niedermayer <michael at niedermayer.cc
> >>> wrote:
> >>
> >>> On Thu, Nov 16, 2017 at 01:21:19PM -0500, Ronald S. Bultje wrote:
> >>>> Hi,
> >>>>
> >>>> On Thu, Nov 16, 2017 at 11:50 AM, Michael Niedermayer <
> >>>> michael at niedermayer.cc> wrote:
> >>>>
> >>>>> On Thu, Nov 16, 2017 at 06:26:06AM -0500, Ronald S. Bultje wrote:
> >>>>>> Hi,
> >>>>>>
> >>>>>> On Wed, Nov 15, 2017 at 10:15 PM, Carl Eugen Hoyos <
> >>> ceffmpeg at gmail.com>
> >>>>>> wrote:
> >>>>>>
> >>>>>>> 2017-11-16 4:06 GMT+01:00 Ronald S. Bultje <rsbultje at gmail.com>:
> >>>>>>>
> >>>>>>>> So, commit it without the error message? I really don't see the
> >>>>> issue.
> >>>>>>>
> >>>>>>> As explained, the issue is that without an error message, it
> >>>>>>> is impossible to parse any related bug report.
> >>>>>>
> >>>>>>
> >>>>>> We've been OK with that situation so far. Since it only happens for
> >>>>> fuzzed
> >>>>>> files, it's OK to continue going like that.
> >>>>>
> >>>>> Thats not the case, the snow spec contains no limit in the place where
> >>>>> we need to check. Its a natural and expected limit so likely all files
> >>>>> will be within that but a file outside would still be arguably valid.
> >>>>>
> >>>>> So a valid file could potentially be outside this range and the
> >>>>> maintainer (that being me) need to know about this.
> >>>>>
> >>>>> Please dont see every change that originated from a fuzzer generated
> >>>>> report as only related to fuzzed files.
> >>>>
> >>>>
> >>>> We are re-hashing old arguments here. I'm not really interested in that.
> >>>
> >>>>
> >>>> My review comment is and remains: please remove the log msg. Otherwise,
> >>> the
> >>>> patch is perfectly fine.
> >>>
> >>> Thank you for your review comment.
> >>>
> >>> please awnser my question, if this is just a suggestion or a
> >>> veto, so we can move forward. Its not clear from your wording to me
> >>> if you belive you have authority over other maintainers or not.
> >>
> >>
> >> I'm sorry, what? Authority? Are you kidding me? Amend the patch, stop being
> >> difficult. This whole discussion is ridiculous. I thought you were on a
> >> deadline for a remote root code execution exploit or something?
> > 
> > The patch should be commited theres no question about this.
> > Why should i amend it and remove the error message which help me to
> > maintain the code ?
> > 
> > Do you realize how ridiculous your request is ?
> > This is code i wrote, code i maintain, you dont work on this code
> > or have anything to do with it. And these error messages help me
> > maintain the code.
> > 
> > I think you should relax a bit and jump over your shadow and let me
> > maintain my code instead of threads like this.
> > 
> > ill remove the messages so this gets resolved but its harder to maintain
> > this way so this means fewer usefull contributions from me and lower
> > code quality.
> > I dont think thats the ideal outcome ...
> > 
> > Thanks
> 
> Use ff_dlog() for the log message. It'll still be enabled on debug
> builds but not on release builds. Ronald said he's ok with it for this
> case on IRC.
> 
> Unless this was suggested before and the idea discarded/rejected for
> some reason...

ff_dlog() is not enabled in user builds so it wont result in an
error message in a bug report.
Its also  not enabled when you use --enable-debug so very few
developers would see it either.

That is in effect ff_dlog() is the same as removing the log message

I also do not know if the build would be generally useable with all
ff_dlog() enabled or too noisy or too slow.

error message have a low volume, they are only shown when an
error occurs.
debug messages and ff_dlog() is high volume there alot
of stuff displayed for valid files.

As a maintainer If a file fails with an error i want to know
what error it is. Why is this so hard or controversal ?

Thanks

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Republics decline into democracies and democracies degenerate into
despotisms. -- 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/20171117/5779f134/attachment.sig>


More information about the ffmpeg-devel mailing list