[FFmpeg-devel] [PATCH] avfilter: Port mp=softpulldown to lavfi

[Paul B Mahol]

On 1/29/15, Michael Niedermayer <michaelni at gmx.at> wrote:
> On Thu, Jan 29, 2015 at 09:20:43AM +0000, Paul B Mahol wrote:
>> On 1/29/15, Michael Niedermayer <michaelni at gmx.at> wrote:
>> > On Wed, Jan 28, 2015 at 03:13:27PM +0000, Paul B Mahol wrote:
>> >> Signed-off-by: Paul B Mahol <onemda at gmail.com>
>> >> ---
>> >> Bit-exact with mp=softpulldown except first frame which is
>> >> uninitialized
>> >> data in mp filter.
>> >
>> >> TODO: pts needs update, workaround is to use setpts filter.
>> >
>> > patch LGTM except the pts / time_base issue
>>
>> mp=softpulldown sets all frames to MP_NOPTS_VALUE.
>>
>> I do not see way how to set filter to use variable frame rate, which
>> is possible as repeat_pict can change during stream.
>
> Heres my attempt to fix the issue
> The problem which i saw was that the first frames timestamp was
> stored in the temporary internal frame and then returned for many
> frames so they all had a incorrect and equal to the video starttime
> value
>
> Below this timestamp is reset to AV_NOPTS_VALUE and
> the code below takes the somewhat conservative approuch of only
> adjusting timestamps for the common 30000/1001 case and otherwise
> only passing untouched timestamps through or if they are off by a
> field period seting them to AV_NOPTS_VALUE if fps != 30000/1001
>
> timebases and frame rates are identical between input and output
> they needed no adjustment for the testcase i could find

What testcase?
The pattern can change to almost anything.

>
> --- a/libavfilter/vf_softpulldown.c
> +++ b/libavfilter/vf_softpulldown.c
> @@ -72,6 +72,16 @@ static int config_input(AVFilterLink *inlink)
>      return 0;
>  }
>
> +static void update_pts(AVFilterLink *link, AVFrame *f, int64_t pts, int
> fields)
> +{
> +    if (av_cmp_q(link->frame_rate, (AVRational){30000, 1001}) == 0 &&
> +         av_cmp_q(link->time_base, (AVRational){1001, 60000}) <= 0
> +    ) {
> +        f->pts = pts + av_rescale_q(fields, (AVRational){1001, 60000},
> link->time_base);
> +    } else
> +        f->pts = AV_NOPTS_VALUE;
> +}
> +
>  static int filter_frame(AVFilterLink *inlink, AVFrame *in) {
>      AVFilterContext *ctx = inlink->dst;
>      AVFilterLink *outlink = inlink->dst->outputs[0];
> @@ -84,6 +94,7 @@ static int filter_frame(AVFilterLink *inlink, AVFrame *in)
> {
>          s->frame = av_frame_clone(in);
>          if (!s->frame)
>              return AVERROR(ENOMEM);
> +        s->frame->pts = AV_NOPTS_VALUE;
>      }
>
>      out = s->frame;
> @@ -107,6 +118,7 @@ static int filter_frame(AVFilterLink *inlink, AVFrame
> *in) {
>
>          if (in->repeat_pict) {
>              av_frame_make_writable(out);
> +            update_pts(outlink, out, in->pts, 2);
>              for (i = 0; i < s->nb_planes; i++) {
>                  av_image_copy_plane(out->data[i], out->linesize[i] * 2,
>                                      in->data[i], in->linesize[i] * 2,
> @@ -135,6 +147,7 @@ static int filter_frame(AVFilterLink *inlink, AVFrame
> *in) {
>              state = 0;
>          } else {
>              av_frame_make_writable(out);
> +            update_pts(outlink, out, in->pts, 1);
>              for (i = 0; i < s->nb_planes; i++) {
>                  av_image_copy_plane(out->data[i], out->linesize[i] * 2,
>                                      in->data[i], in->linesize[i] * 2,
>
>
> [...]
>
> --
> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> What does censorship reveal? It reveals fear. -- Julian Assange
>