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

[wm4]

On Thu, 6 Apr 2017 11:04:48 +0200
Nicolas George  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.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

[Nicolas George]

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,

-- 
  Nicolas George


signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

[Hendrik Leppkes]

On Thu, Apr 6, 2017 at 10:44 AM, Nicolas George  wrote:
> Signed-off-by: Nicolas George 
> ---
>  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
> _failed);
>  break;
>  case AVMEDIA_TYPE_VIDEO:
> -ret = decode_video(ist, repeating ? NULL : , 
> _output, !pkt,
> +ret = decode_video(ist, repeating ? NULL : , 
> _output, _pts, !pkt,
> _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
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

[Nicolas George]

Signed-off-by: Nicolas George 
---
 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
_failed);
 break;
 case AVMEDIA_TYPE_VIDEO:
-ret = decode_video(ist, repeating ? NULL : , 
_output, !pkt,
+ret = decode_video(ist, repeating ? NULL : , 
_output, _pts, !pkt,
_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

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel