[FFmpeg-devel,2/4] ffmpeg: use reordered duration for stream PTS.

Signed-off-by: Nicolas George <george@nsup.org>
---
 ffmpeg.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)


Unchanged. Necessary for the next patches.

On Thu, Apr 6, 2017 at 10:44 AM, Nicolas George <george@nsup.org> wrote:
> Signed-off-by: Nicolas George <george@nsup.org>
> ---
>  ffmpeg.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
>
>
> Unchanged. Necessary for the next patches.
>
>
> diff --git a/ffmpeg.c b/ffmpeg.c
> index 4db8ea82ac..41f1f076ca 100644
> --- a/ffmpeg.c
> +++ b/ffmpeg.c
> @@ -2357,7 +2357,7 @@ 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, int eof,
> +static int decode_video(InputStream *ist, AVPacket *pkt, int *got_output, int *duration_pts, int eof,
>                          int *decode_failed)
>  {
>      AVFrame *decoded_frame;
> @@ -2448,6 +2448,7 @@ static int decode_video(InputStream *ist, AVPacket *pkt, int *got_output, int eo
>      ist->hwaccel_retrieved_pix_fmt = decoded_frame->format;
>
>      best_effort_timestamp= av_frame_get_best_effort_timestamp(decoded_frame);
> +    *duration_pts = decoded_frame->pkt_duration;
>
>      if (ist->framerate.num)
>          best_effort_timestamp = ist->cfr_next_pts++;
> @@ -2618,6 +2619,7 @@ static int process_input_packet(InputStream *ist, const AVPacket *pkt, int no_eo
>      // while we have more to decode or while the decoder did output something on EOF
>      while (ist->decoding_needed) {
>          int duration_dts = 0;
> +        int duration_pts = 0;
>          int got_output = 0;
>          int decode_failed = 0;
>
> @@ -2630,7 +2632,7 @@ static int process_input_packet(InputStream *ist, const AVPacket *pkt, int no_eo
>                                     &decode_failed);
>              break;
>          case AVMEDIA_TYPE_VIDEO:
> -            ret = decode_video    (ist, repeating ? NULL : &avpkt, &got_output, !pkt,
> +            ret = decode_video    (ist, repeating ? NULL : &avpkt, &got_output, &duration_pts, !pkt,
>                                     &decode_failed);
>              if (!repeating || !pkt || got_output) {
>                  if (pkt && pkt->duration) {
> @@ -2649,7 +2651,7 @@ static int process_input_packet(InputStream *ist, const AVPacket *pkt, int no_eo
>              }
>
>              if (got_output)
> -                ist->next_pts += duration_dts; //FIXME the duration is not correct in some cases
> +                ist->next_pts += av_rescale_q(duration_pts, ist->st->time_base, AV_TIME_BASE_Q);
>              break;
>          case AVMEDIA_TYPE_SUBTITLE:
>              if (repeating)
> --
> 2.11.0
>

As was pointed out before, pkt_duration is often missing or incorrect.
At the very last falling back to the old logic if its unset seems
sensible.

- Hendrik

Le septidi 17 germinal, an CCXXV, Hendrik Leppkes a écrit :
> pkt_duration is often missing or incorrect.

I have FATE-tested the resulting code and it exhibits no regression.

> At the very last falling back to the old logic if its unset seems
> sensible.

I do not think so. The old logic was utterly flawed, mixing PTS and DTS.
If the new logic did exhibit regressions, I would have tried to find a
proper fix before submitting. Since it does not, I say we wait for an
actual problematic case to implement workaround logic.

Regards,

On Thu, 6 Apr 2017 11:04:48 +0200
Nicolas George <george@nsup.org> wrote:

> Le septidi 17 germinal, an CCXXV, Hendrik Leppkes a écrit :
> > pkt_duration is often missing or incorrect.  
> 
> I have FATE-tested the resulting code and it exhibits no regression.

FATE doesn't test most of the potentially problematic decoders. Also,
I'm not sure how FATE is supposed to inform you about broken
pkt_durations if it passed before.