[FFmpeg-devel] [PATCH 2/3] ffmpeg: use new decode API

[wm4]

On Fri, 30 Sep 2016 21:48:03 +0200
Michael Niedermayer <michael at niedermayer.cc> wrote:

> On Thu, Sep 29, 2016 at 08:25:18PM +0200, wm4 wrote:
> > This is a bit messy, mainly due to timestamp handling.
> > 
> > decode_video() relied on the fact that it could set dts on a flush/drain
> > packet. This is not possible with the old API, and won't be. (I think
> > doing this was very questionable with the old API. Flush packets should
> > not contain any information; they just cause a FIFO to be emptied.) This
> > is replaced with checking the best_effort_timestamp for AV_NOPTS_VALUE,
> > and using the suggested DTS in the drain case.
> > 
> > The fate-cavs test still fails due to dropping the last frame. This
> > happens because the timestamp of the last frame goes backwards
> > (ffprobe -show_frames shows the same thing). I suspect that this
> > "worked" due to the best effort timestamp logic picking the DTS
> > over the decreasing PTS. Since this logic is in libavcodec (where
> > it probably shouldn't be), this can't be easily fixed. The timestamps
> > of the cavs samples are weird anyway, so I chose not to fix it.
> > 
> > Another strange thing is the timestamp handling in the video path of
> > process_input_packet (after the decode_video() call). It looks like
> > the code to increase next_dts and next_pts should be run every time
> > a frame is decoded - but it's needed even if output is skipped.
> > ---
> >  ffmpeg.c            | 178 +++++++++++++++++++++++++++++++++++++---------------
> >  ffmpeg.h            |   3 +
> >  tests/ref/fate/cavs |   1 -
> >  3 files changed, 129 insertions(+), 53 deletions(-)  
> [...]
> 
> > @@ -2101,23 +2129,45 @@ static int decode_audio(InputStream *ist, AVPacket *pkt, int *got_output)
> >      return err < 0 ? err : ret;
> >  }
> >  
> > -static int decode_video(InputStream *ist, AVPacket *pkt, int *got_output)
> > +static int decode_video(InputStream *ist, AVPacket *pkt, int *got_output, int eof)
> >  {
> >      AVFrame *decoded_frame, *f;
> >      int i, ret = 0, err = 0, resample_changed;
> >      int64_t best_effort_timestamp;  
> 
> > +    int64_t dts = AV_NOPTS_VALUE;  
> [...]
> > -    pkt->dts  = av_rescale_q(ist->dts, AV_TIME_BASE_Q, ist->st->time_base);
> > +    if (ist->dts != AV_NOPTS_VALUE)
> > +        dts = av_rescale_q(ist->dts, AV_TIME_BASE_Q, ist->st->time_base);  
> 
> can be simplified with AV_ROUND_PASS_MINMAX if you like
> 
> [...]
> > diff --git a/ffmpeg.h b/ffmpeg.h
> > index 3ba62a1..1195055 100644
> > --- a/ffmpeg.h
> > +++ b/ffmpeg.h
> > @@ -348,6 +348,9 @@ typedef struct InputStream {
> >      // number of frames/samples retrieved from the decoder
> >      uint64_t frames_decoded;
> >      uint64_t samples_decoded;
> > +
> > +    uint64_t *dts_buffer;  
> 
> it makes no difference with the current code but timestamps are signed

Pushed with the type for timestamps adjusted, and the new failing tests
modified as discussed on IRC. Didn't change the NOPTS check above, but
I'm not opposed to it, feel free to push a commit doing that if you
think it's much of an improvement.