[FFmpeg-devel] [PATCH] Issue more explicit error messages in compute_pkt_fields2().

[Stefano Sabatini]

On date Wednesday 2011-01-05 16:55:14 +0100, Michael Niedermayer encoded:
> On Tue, Jan 04, 2011 at 07:50:47PM +0100, Stefano Sabatini wrote:
> > ---
> >  libavformat/utils.c |    6 ++++--
> >  1 files changed, 4 insertions(+), 2 deletions(-)
> > 
> > diff --git a/libavformat/utils.c b/libavformat/utils.c
> > index fb93e3b..3b1b34e 100644
> > --- a/libavformat/utils.c
> > +++ b/libavformat/utils.c
> > @@ -2917,12 +2917,14 @@ static int compute_pkt_fields2(AVFormatContext *s, AVStream *st, AVPacket *pkt){
> >  
> >      if(st->cur_dts && st->cur_dts != AV_NOPTS_VALUE && st->cur_dts >= pkt->dts){
> >          av_log(s, AV_LOG_ERROR,
> > -               "st:%d error, non monotone timestamps %"PRId64" >= %"PRId64"\n",
> > +               "Non monotone timestamps in stream number %d: st->cur_dts:%"PRId64" >= pkt->dts:%"PRId64"\n",
> >                 st->index, st->cur_dts, pkt->dts);
> 
> Thats just confusing the user with internal details.
> And might make it look like its some problem internal to libav while truth,
> plain and simple the libav user (like ffmpeg.c) has provided invalid timestamps
> What libavformat internal variables are used to make this check has no relevance

Where the problem lays is secondary, I'd like the log message to
convey what was wrong in an intelligible way.
 
> >          return -1;
> >      }
> >      if(pkt->dts != AV_NOPTS_VALUE && pkt->pts != AV_NOPTS_VALUE && pkt->pts < pkt->dts){
> > -        av_log(s, AV_LOG_ERROR, "st:%d error, pts < dts\n", st->index);
> > +        av_log(s, AV_LOG_ERROR,
> > +               "pts < dts in stream number %d\n: pkt->pts:%"PRId64" < pkt->dts:%"PRId64"\n",
> > +               st->index, pkt->pts, pkt->dts);
> 
> This should be on one line for being grepable saneley and theres no need to
> say pts<dts twice.

> Also you loose the word error which is the most fundamental part of this, it
> is an error not a warning

Error is implied by the log level of the message, and also from
the fact that the function is returning an error code. I can add the
string "error" but this looks quite pointless to me.

Updated.
-- 
FFmpeg = Fancy and Fabulous Magical Pure Experimenting Guru