On Mon, Sep 04, 2017 at 11:58:07AM -0300, James Almer wrote: > On 9/4/2017 6:48 AM, Michael Niedermayer wrote: > > On Mon, Sep 04, 2017 at 12:57:32AM +0000, Kieran Kunhya wrote: > >> On Mon, 4 Sep 2017 at 00:56 Michael Niedermayer <mich...@niedermayer.cc> > >> wrote: > >> > >>> We have always printed error messages for the case that an error > >>> occured. > >>> Its unprofessional to fail decoding a file but not provide any > >>> hint as to why a failure occured. > >>> > >>> If we remove all error messages and just print a generic "failed > >>> decoding header" or "failed to decode frame" > >>> We would leave users wondering about each error and we would no longer > >>> have differentiated bug reports. > >>> There would be one very huge bug report about > >>> "Error while decoding stream #0:0: Invalid data found when processing > >>> input" > >>> Because thats all detail the user can get if the message is not in the > >>> binary. > >>> That bug report then would be marked invalid of course and would help > >>> neither users nor developers. > >>> > >> > >> We ask users to report bugs with "--loglevel 99" so this is irrelevant if > >> it's a DEBUG. > > > > yes, true > > > > > >> > >> > >>> Iam not sure why error messages are since about a year or so > >>> considered controversial by some developers but not before > >>> > >> > >> In my case I get a lot of reports from users about ffmpeg spamming logs > >> with obscure warnings. > >> This happens on legal files as per my "SPS/PPS truncation" patch which > >> scares them into thinking the file is broken. > >> > >> The same goes with spamming logs when there is some data corruption with > >> warnings which are very obscure. > > > > The messages from libavcodec are by the nature of it quite technical. > > Does showing these messages to your users have any value for them at > > all ? > > > > AV_LOG_ERROR is documented as > > /** > > * Something went wrong and cannot losslessly be recovered. > > * However, not all future data is affected. > > */ > > #define AV_LOG_ERROR 16 > > > > This matches the case its used in. > > > > I am not against using DEBUG level for detailed error messages, and > > ERROR for generic ones if thats what more people prefer. > > But for this to achive any usefull effect it has to be done > > consistently accross the codebase. ATM we more commonly use > > AV_LOG_ERROR for these. > > What some people dislike here is not that you're adding new error > messages but that said error messages mean nothing even to developers in > general. > > "Failed to allocate a frame size %dx%d", "RGB not supported in profile > %d", "Requested reference %d not available", "Profile %d is not yet > supported" are concise and informative. You ran out of memory, the file > reports invalid parameters, the file is missing data, the file contains > features not yet supported. > Be it an user or a dev, you can easily realize what went wrong. > > However, "Invalid cbd %d", crd %d", "Invalid ld %d" mean nothing to > pretty much anyone except the person that wrote the relevant code. The > three are names for local variables you added as duplicate of another > three local variables for the purpose of doing range checks. > They are not even labels in the snow spec (assuming there's one), they > are just variable names that could as well have been x, y and z. >
> Maybe you could replace them with an error message that mentions > something like a block within a frame has out of range or otherwise > invalid data/values? Anything that actually informs whoever sees the > message that something is wrong in the file and not that some internal > variable has an invalid value. sure, ill reword so its more clear what is wrong and resubmit Thanks! [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB No great genius has ever existed without some touch of madness. -- Aristotle
signature.asc
Description: Digital signature
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
