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

[Michael Niedermayer]

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

--- 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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20150129/ca0b31ab/attachment.asc>