On 1/26/17, wm4 <nfx...@googlemail.com> wrote:
> On Thu, 26 Jan 2017 03:20:02 +0100
> Michael Niedermayer <michae...@gmx.at> wrote:
>
>> On Thu, Jan 26, 2017 at 02:58:07AM +0100, Andreas Cadhalpun wrote:
>> > On 26.01.2017 02:29, Ronald S. Bultje wrote:
>> > > On Wed, Jan 25, 2017 at 8:12 PM, Andreas Cadhalpun <
>> > > andreas.cadhal...@googlemail.com> wrote:
>> > >
>> > >> Signed-off-by: Andreas Cadhalpun <andreas.cadhal...@googlemail.com>
>> > >> ---
>> > >> libavformat/nistspheredec.c | 11 +++++++++++
>> > >> 1 file changed, 11 insertions(+)
>> > >>
>> > >> diff --git a/libavformat/nistspheredec.c
>> > >> b/libavformat/nistspheredec.c
>> > >> index 782d1dfbfb..3386497682 100644
>> > >> --- a/libavformat/nistspheredec.c
>> > >> +++ b/libavformat/nistspheredec.c
>> > >> @@ -21,6 +21,7 @@
>> > >>
>> > >> #include "libavutil/avstring.h"
>> > >> #include "libavutil/intreadwrite.h"
>> > >> +#include "libavcodec/internal.h"
>> > >> #include "avformat.h"
>> > >> #include "internal.h"
>> > >> #include "pcm.h"
>> > >> @@ -90,6 +91,11 @@ static int nist_read_header(AVFormatContext *s)
>> > >> return 0;
>> > >> } else if (!memcmp(buffer, "channel_count", 13)) {
>> > >> sscanf(buffer, "%*s %*s %"SCNd32,
>> > >> &st->codecpar->channels);
>> > >> + if (st->codecpar->channels > FF_SANE_NB_CHANNELS) {
>> > >> + av_log(s, AV_LOG_ERROR, "Too many channels %d >
>> > >> %d\n",
>> > >> + st->codecpar->channels, FF_SANE_NB_CHANNELS);
>> > >> + return AVERROR(ENOSYS);
>> > >> + }
>> > >
>> > >
>> > > I've said this before, but again - please don't add useless log
>> > > messages.
>> >
>> > I disagree that these log messages are useless and I'm not the only one
>> > [1].
>>
>> +1
>>
>> Log messages make debuging the code easier, as a developer i like to
>> know why something fails having a hard failure but no clear and easy
>> findable error message is bad
>>
>>
>> [...]
>
> -1
>
> This kind of things bloat the code with rare corner cases. One point
> would be that this increases binary size (why do we even still have the
> NULL_IF_CONFIG_SMALL macro? I'm not saying we should use it here, but
> the current use of the macro will barely even make a dent into the
> number of bytes "wasted" for optional strings, yet we bother).
>
> Another point is that code becomes unreadable if obscure corner cases
> take up most of the code. I think that's w worrisome direction we're
> taking here.
>
> When I debug FFmpeg API use, the thing that annoys me most is that I
> can't know where an error happens. I have to trace the origin manually.
> Error codes are almost always completely useless. This patchset adds a
> lot of NOSYS, which is used 254 times in FFmpeg and which is about 99%
> meaningless and resolves to an even worse error message when using
> av_err2str(). Adding an av_log to error error point would "work" (at
> least you could use grep), but isn't a good solution.
>
> In this particular case, I'd question why the code calling this
> function (avformat_open_inputavcodec_open) doesn't print an error
> message instead, or so.
>
> But of course not every API user wants such an error message (but still
> wants others).
I don't want those log messages in there.
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
