[FFmpeg-devel] [PATCH] lavf: make compute_pkt_fields2() return meaningful error values

[Stefano Sabatini]

On date Monday 2011-03-21 03:04:14 +0100, Michael Niedermayer encoded:
> On Sun, Mar 20, 2011 at 06:34:27PM +0100, Stefano Sabatini wrote:
> > On date Tuesday 2011-03-15 13:12:08 +0100, Stefano Sabatini wrote:
> > > ---
> > >  libavformat/utils.c |    6 +++---
> > >  1 files changed, 3 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/libavformat/utils.c b/libavformat/utils.c
> > > index ccaf77d..193707f 100644
> > > --- a/libavformat/utils.c
> > > +++ b/libavformat/utils.c
> > > @@ -2918,7 +2918,7 @@ static int compute_pkt_fields2(AVFormatContext *s, AVStream *st, AVPacket *pkt){
> > >              pkt->pts, pkt->dts, st->cur_dts, delay, pkt->size, pkt->stream_index);
> > >  
> > >  /*    if(pkt->pts == AV_NOPTS_VALUE && pkt->dts == AV_NOPTS_VALUE)
> > > -        return -1;*/
> > > +        return AVERROR(EINVAL);*/
> > >  
> > >      /* duration field */
> > >      if (pkt->duration == 0) {
> > > @@ -2953,11 +2953,11 @@ static int compute_pkt_fields2(AVFormatContext *s, AVStream *st, AVPacket *pkt){
> > >          av_log(s, AV_LOG_ERROR,
> > >                 "Application provided invalid, non monotonically increasing dts to muxer in stream %d: %"PRId64" >= %"PRId64"\n",
> > >                 st->index, st->cur_dts, pkt->dts);
> > > -        return -1;
> > > +        return AVERROR(EINVAL);
> > >      }
> > >      if(pkt->dts != AV_NOPTS_VALUE && pkt->pts != AV_NOPTS_VALUE && pkt->pts < pkt->dts){
> > >          av_log(s, AV_LOG_ERROR, "pts < dts in stream %d\n", st->index);
> > > -        return -1;
> > > +        return AVERROR(EINVAL);
> > >      }
> > >  
> > >  //    av_log(s, AV_LOG_DEBUG, "av_write_frame: pts2:%"PRId64" dts2:%"PRId64"\n", pkt->pts, pkt->dts);
> > 
> > Ping?
> 
> no objection, not sure the new error codes are ideal though but i cant really
> name better ones

Indeed, I'm quite unsatisfied as it is not very clear if these are
"invalid parameters" errors or rather should be considered "invalid
data errors".

These problems usually occur when copying malformed streams with
invalid timestamps, e.g. non monotonically increasing DTS, see for
example: https://roundup.ffmpeg.org/issue1551

Anyway I believe at the muxer level they should be considered an
application error, since it is the application providing the
timestamps which is doing something controversial (e.g. not
checking/fixing the input values).

I'll push it if I make up my mind (or if someone else has a better
suggestion), I'll post more about the TS issue soon.
-- 
FFmpeg = Fiendish and Fostering Meaningless Proud Ecstatic God