Re: [FFmpeg-devel] Proposed vf_decimate enhancement

2015-12-14 Thread Lou Logan
On Mon, Dec 14, 2015, at 12:24 PM, Ray Cole wrote:
> Quite honestly I decided it isn't worth the frustration of trying to
> submit a patch. It works for me and I'm happy with it.
> 
> -- Ray

Our guidelines and peer reviews may seem stringent, but consider it a
quality control to help get the best code we can into FFmpeg and to make
it easier for us review and add your contributions.

We are just volunteers and some of us have already invested some time
providing input on your changes and suggestions on how to provide a
proper patch. If you need further help you can ask here or you may find
live help at the #ffmpeg-devel IRC channel.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] Proposed vf_decimate enhancement

2015-12-14 Thread Ray Cole

Quite honestly I decided it isn't worth the frustration of trying to submit a 
patch. It works for me and I'm happy with it.

-- Ray

On 12/14/2015 03:19 PM, Michael Niedermayer wrote:

On Tue, Sep 29, 2015 at 11:02:33AM -0500, Ray Cole wrote:

Here is an updated patch. I cleaned the code up to hopefully be closer to 
standards. It works well for me, but your mileage may vary...

--- vf_decimate.c   2015-09-29 10:56:46.171698492 -0500
+++ vf_decimatex.c  2015-09-29 10:59:50.679695685 -0500

a git patch with a commit message would be better
see:
git commit -a
git format-patch -1


[...]


@@ -51,6 +52,10 @@
  int bdiffsize;
  int64_t *bdiffs;
  
+/* Ray */

git keeps track of who changed what




+int lastdrop;
+int64_t drop_count[25]; // drop counts

The purpose of comments is to provide additional information not
already in the field name



+
  /* options */
  int cycle;
  double dupthresh_flt;
@@ -60,6 +65,9 @@
  int blockx, blocky;
  int ppsrc;
  int chroma;
+int force_drop;
+int lock_on;
+
  } DecimateContext;
  
  #define OFFSET(x) offsetof(DecimateContext, x)

@@ -71,9 +79,13 @@
  { "scthresh",  "set scene change threshold", OFFSET(scthresh_flt),  
AV_OPT_TYPE_DOUBLE, {.dbl = 15.0}, 0, 100, FLAGS },
  { "blockx","set the size of the x-axis blocks used during metric 
calculations", OFFSET(blockx), AV_OPT_TYPE_INT, {.i64 = 32}, 4, 1<<9, FLAGS },
  { "blocky","set the size of the y-axis blocks used during metric 
calculations", OFFSET(blocky), AV_OPT_TYPE_INT, {.i64 = 32}, 4, 1<<9, FLAGS },
-{ "ppsrc", "mark main input as a pre-processed input and activate clean 
source input stream", OFFSET(ppsrc), AV_OPT_TYPE_BOOL, {.i64=0}, 0, 1, FLAGS },
+{ "ppsrc", "mark main input as a pre-processed input and activate clean 
source input stream", OFFSET(ppsrc), AV_OPT_TYPE_INT, {.i64=0}, 0, 1, FLAGS },
-{ "chroma","set whether or not chroma is considered in the metric 
calculations", OFFSET(chroma), AV_OPT_TYPE_BOOL, {.i64=1}, 0, 1, FLAGS },
+{ "chroma","set whether or not chroma is considered in the metric 
calculations", OFFSET(chroma), AV_OPT_TYPE_INT, {.i64=1}, 0, 1, FLAGS },

this looks like a unintended mistake



+{ "force_drop","set to forcefully drop frame X in cycle", 
OFFSET(force_drop), AV_OPT_TYPE_INT, {.i64=-1}, -1, 4, FLAGS },
+{ "lock_on","set to lock on to a cycle", OFFSET(lock_on), 
AV_OPT_TYPE_INT, {.i64=0}, 0, 1, FLAGS },




+
  { NULL }
+
  };
  
  AVFILTER_DEFINE_CLASS(decimate);

@@ -140,13 +152,15 @@
  q->totdiff = 0;
  for (i = 0; i < dm->bdiffsize; i++)
  q->totdiff += bdiffs[i];
+
  q->maxbdiff = maxdiff;
+
  }

stray changes
please read your patch before submitting



  
  static int filter_frame(AVFilterLink *inlink, AVFrame *in)

  {
-int scpos = -1, duppos = -1;
-int drop = INT_MIN, i, lowest = 0, ret;
+int scpos = -1, duppos = -1, common = 0, start = 0;
+int drop = INT_MIN, i, lowest = 0, lowest_tot = 0, ret =0;
  AVFilterContext *ctx  = inlink->dst;
  AVFilterLink *outlink = ctx->outputs[0];
  DecimateContext *dm   = ctx->priv;
@@ -176,17 +190,128 @@
  dm->last = av_frame_clone(in);
  dm->fid = 0;
  
+

+// The major change starts here

git keeps track of changes, theres no need to put such notes in
the source


[...]

@@ -372,6 +499,7 @@
  fps = av_mul_q(fps, (AVRational){dm->cycle - 1, dm->cycle});
  av_log(ctx, AV_LOG_VERBOSE, "FPS: %d/%d -> %d/%d\n",
 inlink->frame_rate.num, inlink->frame_rate.den, fps.num, fps.den);
+outlink->flags |= FF_LINK_FLAG_REQUEST_LOOP;

this flag no longer exists

and please provide a commit message for the change which describes
what is changed, how and why


[...]



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


Re: [FFmpeg-devel] Proposed vf_decimate enhancement

2015-12-14 Thread Michael Niedermayer
On Tue, Sep 29, 2015 at 11:02:33AM -0500, Ray Cole wrote:
> Here is an updated patch. I cleaned the code up to hopefully be closer to 
> standards. It works well for me, but your mileage may vary...
> 

> --- vf_decimate.c 2015-09-29 10:56:46.171698492 -0500
> +++ vf_decimatex.c2015-09-29 10:59:50.679695685 -0500

a git patch with a commit message would be better
see:
git commit -a
git format-patch -1


[...]

> @@ -51,6 +52,10 @@
>  int bdiffsize;
>  int64_t *bdiffs;
>  
> +/* Ray */

git keeps track of who changed what



> +int lastdrop;

> +int64_t drop_count[25]; // drop counts

The purpose of comments is to provide additional information not
already in the field name


> +
>  /* options */
>  int cycle;
>  double dupthresh_flt;
> @@ -60,6 +65,9 @@
>  int blockx, blocky;
>  int ppsrc;
>  int chroma;
> +int force_drop;
> +int lock_on;
> +
>  } DecimateContext;
>  
>  #define OFFSET(x) offsetof(DecimateContext, x)
> @@ -71,9 +79,13 @@
>  { "scthresh",  "set scene change threshold", OFFSET(scthresh_flt),  
> AV_OPT_TYPE_DOUBLE, {.dbl = 15.0}, 0, 100, FLAGS },
>  { "blockx","set the size of the x-axis blocks used during metric 
> calculations", OFFSET(blockx), AV_OPT_TYPE_INT, {.i64 = 32}, 4, 1<<9, FLAGS },
>  { "blocky","set the size of the y-axis blocks used during metric 
> calculations", OFFSET(blocky), AV_OPT_TYPE_INT, {.i64 = 32}, 4, 1<<9, FLAGS },

> -{ "ppsrc", "mark main input as a pre-processed input and activate 
> clean source input stream", OFFSET(ppsrc), AV_OPT_TYPE_BOOL, {.i64=0}, 0, 1, 
> FLAGS },
> +{ "ppsrc", "mark main input as a pre-processed input and activate 
> clean source input stream", OFFSET(ppsrc), AV_OPT_TYPE_INT, {.i64=0}, 0, 1, 
> FLAGS },
> -{ "chroma","set whether or not chroma is considered in the metric 
> calculations", OFFSET(chroma), AV_OPT_TYPE_BOOL, {.i64=1}, 0, 1, FLAGS },
> +{ "chroma","set whether or not chroma is considered in the metric 
> calculations", OFFSET(chroma), AV_OPT_TYPE_INT, {.i64=1}, 0, 1, FLAGS },

this looks like a unintended mistake


> +{ "force_drop","set to forcefully drop frame X in cycle", 
> OFFSET(force_drop), AV_OPT_TYPE_INT, {.i64=-1}, -1, 4, FLAGS },
> +{ "lock_on","set to lock on to a cycle", OFFSET(lock_on), 
> AV_OPT_TYPE_INT, {.i64=0}, 0, 1, FLAGS },



> +
>  { NULL }
> +
>  };
>  
>  AVFILTER_DEFINE_CLASS(decimate);
> @@ -140,13 +152,15 @@
>  q->totdiff = 0;
>  for (i = 0; i < dm->bdiffsize; i++)
>  q->totdiff += bdiffs[i];
> +
>  q->maxbdiff = maxdiff;
> +
>  }

stray changes
please read your patch before submitting



>  
>  static int filter_frame(AVFilterLink *inlink, AVFrame *in)
>  {
> -int scpos = -1, duppos = -1;
> -int drop = INT_MIN, i, lowest = 0, ret;
> +int scpos = -1, duppos = -1, common = 0, start = 0;
> +int drop = INT_MIN, i, lowest = 0, lowest_tot = 0, ret =0;
>  AVFilterContext *ctx  = inlink->dst;
>  AVFilterLink *outlink = ctx->outputs[0];
>  DecimateContext *dm   = ctx->priv;

> @@ -176,17 +190,128 @@
>  dm->last = av_frame_clone(in);
>  dm->fid = 0;
>  
> +
> +// The major change starts here

git keeps track of changes, theres no need to put such notes in
the source


[...]
> @@ -372,6 +499,7 @@
>  fps = av_mul_q(fps, (AVRational){dm->cycle - 1, dm->cycle});
>  av_log(ctx, AV_LOG_VERBOSE, "FPS: %d/%d -> %d/%d\n",
> inlink->frame_rate.num, inlink->frame_rate.den, fps.num, fps.den);
> +outlink->flags |= FF_LINK_FLAG_REQUEST_LOOP;

this flag no longer exists

and please provide a commit message for the change which describes
what is changed, how and why


[...]

-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Opposition brings concord. Out of discord comes the fairest harmony.
-- Heraclitus


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


Re: [FFmpeg-devel] Proposed vf_decimate enhancement

2015-09-29 Thread Ray Cole

Here is an updated patch. I cleaned the code up to hopefully be closer to 
standards. It works well for me, but your mileage may vary...

--- vf_decimate.c   2015-09-29 10:56:46.171698492 -0500
+++ vf_decimatex.c  2015-09-29 10:59:50.679695685 -0500
@@ -27,6 +27,7 @@
 
 #define INPUT_MAIN 0
 #define INPUT_CLEANSRC 1
+#define DROP_HISTORY   8
 
 struct qitem {
 AVFrame *frame;
@@ -51,6 +52,10 @@
 int bdiffsize;
 int64_t *bdiffs;
 
+/* Ray */
+int lastdrop;
+int64_t drop_count[25]; // drop counts
+
 /* options */
 int cycle;
 double dupthresh_flt;
@@ -60,6 +65,9 @@
 int blockx, blocky;
 int ppsrc;
 int chroma;
+int force_drop;
+int lock_on;
+
 } DecimateContext;
 
 #define OFFSET(x) offsetof(DecimateContext, x)
@@ -71,9 +79,13 @@
 { "scthresh",  "set scene change threshold", OFFSET(scthresh_flt),  
AV_OPT_TYPE_DOUBLE, {.dbl = 15.0}, 0, 100, FLAGS },
 { "blockx","set the size of the x-axis blocks used during metric 
calculations", OFFSET(blockx), AV_OPT_TYPE_INT, {.i64 = 32}, 4, 1<<9, FLAGS },
 { "blocky","set the size of the y-axis blocks used during metric 
calculations", OFFSET(blocky), AV_OPT_TYPE_INT, {.i64 = 32}, 4, 1<<9, FLAGS },
-{ "ppsrc", "mark main input as a pre-processed input and activate 
clean source input stream", OFFSET(ppsrc), AV_OPT_TYPE_BOOL, {.i64=0}, 0, 1, 
FLAGS },
-{ "chroma","set whether or not chroma is considered in the metric 
calculations", OFFSET(chroma), AV_OPT_TYPE_BOOL, {.i64=1}, 0, 1, FLAGS },
+{ "ppsrc", "mark main input as a pre-processed input and activate 
clean source input stream", OFFSET(ppsrc), AV_OPT_TYPE_INT, {.i64=0}, 0, 1, 
FLAGS },
+{ "chroma","set whether or not chroma is considered in the metric 
calculations", OFFSET(chroma), AV_OPT_TYPE_INT, {.i64=1}, 0, 1, FLAGS },
+{ "force_drop","set to forcefully drop frame X in cycle", 
OFFSET(force_drop), AV_OPT_TYPE_INT, {.i64=-1}, -1, 4, FLAGS },
+{ "lock_on","set to lock on to a cycle", OFFSET(lock_on), 
AV_OPT_TYPE_INT, {.i64=0}, 0, 1, FLAGS },
+
 { NULL }
+
 };
 
 AVFILTER_DEFINE_CLASS(decimate);
@@ -140,13 +152,15 @@
 q->totdiff = 0;
 for (i = 0; i < dm->bdiffsize; i++)
 q->totdiff += bdiffs[i];
+
 q->maxbdiff = maxdiff;
+
 }
 
 static int filter_frame(AVFilterLink *inlink, AVFrame *in)
 {
-int scpos = -1, duppos = -1;
-int drop = INT_MIN, i, lowest = 0, ret;
+int scpos = -1, duppos = -1, common = 0, start = 0;
+int drop = INT_MIN, i, lowest = 0, lowest_tot = 0, ret =0;
 AVFilterContext *ctx  = inlink->dst;
 AVFilterLink *outlink = ctx->outputs[0];
 DecimateContext *dm   = ctx->priv;
@@ -176,17 +190,128 @@
 dm->last = av_frame_clone(in);
 dm->fid = 0;
 
+
+// The major change starts here
+
+// First we will NOT consider the 'next' frame in the drop detection because 
that would be wrong.
+// The old code would allow frame 0 to be dropped immediately after frame 4. 
I've not seen a case where that makes sense and
+// frame 0 could never be followed by a drop of 1, nor could frame 1 be 
followed by 2, etc. because of the way detection is
+// performed 5 frames at a time. So first we start at what _should_ be a 
reasonable point to be closer inline with what can
+// happen when frames 0, 1 and 2 are the drops.
+
+start = 0;
+
+if (dm->lastdrop == (dm->cycle - 1))  
+   start = 2; 
+else
+if (dm->lastdrop == (dm->cycle - 2))  
+   start = 1;
+
 /* we have a complete cycle, select the frame to drop */
-lowest = 0;
+lowest = start;  
+lowest_tot = start;
+
+// We will now locate the lowest frame by diff and by total. 
+
 for (i = 0; i < dm->cycle; i++) {
 if (dm->queue[i].totdiff > dm->scthresh)
 scpos = i;
-if (dm->queue[i].maxbdiff < dm->queue[lowest].maxbdiff)
-lowest = i;
+
+if (i >= start || scpos >= 0) {// if in range of eligible for 
pattern drop
+if (dm->queue[lowest].maxbdiff == 0 ||
+dm->queue[i].maxbdiff < dm->queue[lowest].maxbdiff)
+
+lowest = i;
+
+if (dm->queue[lowest_tot].totdiff == 0 ||
+dm->queue[i].totdiff < dm->queue[lowest_tot].totdiff)
+
+lowest_tot = i;
+}
+
 }
+
 if (dm->queue[lowest].maxbdiff < dm->dupthresh)
 duppos = lowest;
-drop = scpos >= 0 && duppos < 0 ? scpos : lowest;
+
+// If the lowest from by max and total do not agree, and this is not the first 
cycle,
+// then we need to figure out how to resolve the conflict.
+// To resolve it we first find out the most commonly dropped frame that we 
have been
+// seeing recently. If either of the two 'lowest' values match the 
commonly-dropped
+// frame then we assume the cycle continue

Re: [FFmpeg-devel] Proposed vf_decimate enhancement

2015-09-29 Thread Ray Cole

Thank you.

I know I have a number of coding style things to clean up before this could be 
accepted (as well as removing some output I'm logging as info) but perhaps 
those familiar with the decimate filter can see if the changes being proposed 
make sense.

-- Ray

On 09/29/2015 07:32 AM, compn wrote:

On Mon, 28 Sep 2015 21:31:59 -0500
Ray Cole  wrote:


This is the first time I've offered up code to any open source
project...so be gentle :-)

ehe, patches are what the devs want, either git diff or diff -u...

i've downloaded and diff'ed your filter to ffmpeg git master and
attached it in this mail.

-compn


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


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


Re: [FFmpeg-devel] Proposed vf_decimate enhancement

2015-09-29 Thread wm4
On Tue, 29 Sep 2015 06:42:53 -0500
Stephen Cole  wrote:

> Thank you for the responses. I assumed the comments would suffice for 
> discussing it initially so one wouldn't have to interpret the patch. I'll 
> produce a diff and resubmit.

A patch is _much_ easier to read.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] Proposed vf_decimate enhancement

2015-09-29 Thread compn
On Mon, 28 Sep 2015 21:31:59 -0500
Ray Cole  wrote:

> This is the first time I've offered up code to any open source
> project...so be gentle :-)

ehe, patches are what the devs want, either git diff or diff -u...

i've downloaded and diff'ed your filter to ffmpeg git master and
attached it in this mail.

-compn--- vf_decimate.c   Tue Sep 29 08:30:08 2015
+++ vf_decimatex.c  Tue Sep 29 08:28:22 2015
@@ -51,6 +51,10 @@
 int bdiffsize;
 int64_t *bdiffs;
 
+/* Ray */
+int lastdrop;
+int64_t drop_count[5]; // drop counts
+
 /* options */
 int cycle;
 double dupthresh_flt;
@@ -60,6 +64,9 @@
 int blockx, blocky;
 int ppsrc;
 int chroma;
+int force_drop;
+int lock_on;
+
 } DecimateContext;
 
 #define OFFSET(x) offsetof(DecimateContext, x)
@@ -71,9 +78,13 @@
 { "scthresh",  "set scene change threshold", OFFSET(scthresh_flt),  
AV_OPT_TYPE_DOUBLE, {.dbl = 15.0}, 0, 100, FLAGS },
 { "blockx","set the size of the x-axis blocks used during metric 
calculations", OFFSET(blockx), AV_OPT_TYPE_INT, {.i64 = 32}, 4, 1<<9, FLAGS },
 { "blocky","set the size of the y-axis blocks used during metric 
calculations", OFFSET(blocky), AV_OPT_TYPE_INT, {.i64 = 32}, 4, 1<<9, FLAGS },
-{ "ppsrc", "mark main input as a pre-processed input and activate 
clean source input stream", OFFSET(ppsrc), AV_OPT_TYPE_BOOL, {.i64=0}, 0, 1, 
FLAGS },
-{ "chroma","set whether or not chroma is considered in the metric 
calculations", OFFSET(chroma), AV_OPT_TYPE_BOOL, {.i64=1}, 0, 1, FLAGS },
+{ "ppsrc", "mark main input as a pre-processed input and activate 
clean source input stream", OFFSET(ppsrc), AV_OPT_TYPE_INT, {.i64=0}, 0, 1, 
FLAGS },
+{ "chroma","set whether or not chroma is considered in the metric 
calculations", OFFSET(chroma), AV_OPT_TYPE_INT, {.i64=1}, 0, 1, FLAGS },
+{ "force_drop","set to forcefully drop frame X in cycle", 
OFFSET(force_drop), AV_OPT_TYPE_INT, {.i64=-1}, -1, 4, FLAGS },
+{ "lock_on","set to lock on to a cycle", OFFSET(lock_on), 
AV_OPT_TYPE_INT, {.i64=0}, 0, 1, FLAGS },
+
 { NULL }
+
 };
 
 AVFILTER_DEFINE_CLASS(decimate);
@@ -140,13 +151,15 @@
 q->totdiff = 0;
 for (i = 0; i < dm->bdiffsize; i++)
 q->totdiff += bdiffs[i];
+
 q->maxbdiff = maxdiff;
+
 }
 
 static int filter_frame(AVFilterLink *inlink, AVFrame *in)
 {
 int scpos = -1, duppos = -1;
-int drop = INT_MIN, i, lowest = 0, ret;
+int drop = INT_MIN, i, lowest = 0, lowest_tot = 0, ret =0;
 AVFilterContext *ctx  = inlink->dst;
 AVFilterLink *outlink = ctx->outputs[0];
 DecimateContext *dm   = ctx->priv;
@@ -176,17 +189,166 @@
 dm->last = av_frame_clone(in);
 dm->fid = 0;
 
+
+// The major change starts here
+
+// First we will NOT consider the 'next' frame in the drop detection because 
that would be wrong.
+// The old code would allow frame 0 to be dropped immediately after frame 4. 
I've not seen a case where that makes sense and
+// frame 0 could never be followed by a drop of 1, nor could frame 1 be 
followed by 2, etc. because of the way detection is
+// performed 5 frames at a time. So first we start at what _should_ be a 
reasonable point to be closer inline with what can
+// happen when frames 0, 1 and 2 are the drops.
+
+   int iStart = 0;
+
+   if (dm->lastdrop == (dm->cycle - 1))  // Do NOT allow it to drop 2 
frames in a row...because that WOULD be wrong...
+  iStart = 2;// Last frame of cycle runs 
chance of consecutive frame drop without this...
+ // Perhaps should force 2 frames 
inbetween? Sure - let's start at 2 for 4, 1 for 3
+   else
+   if (dm->lastdrop == (dm->cycle - 2))  // Do NOT allow it to drop 2 
frames in a row...because that WOULD be wrong...
+  iStart = 1;// Make sure 2 frames skipped 
before next drop
+   
 /* we have a complete cycle, select the frame to drop */
-lowest = 0;
+lowest = iStart; 
+lowest_tot = iStart;
+
+// We will now locate the lowest frame by diff and by total. 
+
 for (i = 0; i < dm->cycle; i++) {
 if (dm->queue[i].totdiff > dm->scthresh)
 scpos = i;
-if (dm->queue[i].maxbdiff < dm->queue[lowest].maxbdiff)
-lowest = i;
+
+   if (i >= iStart || scpos >= 0)  // if in range of eligible for 
pattern drop
+   {
+   if (dm->queue[lowest].maxbdiff == 0 ||
+   dm->queue[i].maxbdiff < dm->queue[lowest].maxbdiff)
+   lowest = i;
+
+   if (dm->queue[lowest_tot].totdiff == 0 ||
+   dm->queue[i].totdiff < dm->queue[lowest_tot].totdiff)
+   lowest_tot = i;
+   };
 }
+
 if (dm->queue[lowest].maxbdiff < dm->dupthresh)
 duppos =

Re: [FFmpeg-devel] Proposed vf_decimate enhancement

2015-09-29 Thread Stephen Cole
Thank you for the responses. I assumed the comments would suffice for 
discussing it initially so one wouldn't have to interpret the patch. I'll 
produce a diff and resubmit.

> On Sep 29, 2015, at 2:16 AM, Paul B Mahol  wrote:
> 
>> On 9/29/15, Ray Cole  wrote:
>> I hope this is the appropriate place to propose an enhancement. This is the
>> first time I've offered up code to any open source project...so be gentle
>> :-)
>> 
>> First, I love ffmpeg. Wonderful software and thank you for your efforts.
>> 
>> I have been pulling down a number of movies back to 24 FPS (24000/1001)
>> using fieldmatch and decimate. However decimate seemed to drop incorrect
>> frames from time-to-time particularly on scenes with little motion. The
>> pullup filter likewise made poor decisions under similar circumstances.
>> 
>> So...I modified vf_decimate and it is working very well for me. I make no
>> claims that the enhancements would work for anyone else. My source is 1080i
>> 29.97 fps movies recording from component video. I'm pulling down to 24 fps
>> (24000/1001 actually).
>> 
>> The changes are:
>> 1) The total and max diffs are used to find the lowest frame to drop rather
>> than just the max diff. If these two methods disagree with one another then
>> it goes through a 'conflict resolution'. The conflict resolution checks to
>> see if either method matches the most commonly-dropped frame (a simple
>> short-term history of drops is retained for this purpose). If so, the most
>> commonly-dropped frame is assumed to be correct. If they do not match then
>> it uses the last dropped frame. This keeps the filter from varying the frame
>> drop so often and made a big difference in detection, at least with the
>> stuff I'm working with.
>> 
>> 2) The existing vf_decimate allows frame 4 to be dropped immediately
>> followed by frame 0 whereas frame 3 dropped could never be followed by frame
>> 4 dropped - similar with frames 0 through 2. Having 2 frames in a row
>> eligible to be dropped seems wrong and the biggest issue I had was when the
>> drop cycle was hitting frame 4. So I put in some code that says if the last
>> frame dropped was frame 4 then frame 0 and frame 1 is not eligible for drop.
>> If frame 3 was last dropped then frame 0 is not dropped. This enforces 2
>> undropped frames between drops. I'm not "married" to this...but it did help
>> quite a bit.
>> 
>> 3) I had one movie where frame 4 was ALWAYS the correct frame to drop...so I
>> added an option to 'lock on' to a pattern in 1 of 2 ways. The first way is
>> for someone to pass force_drop=x where x is the frame number to drop each
>> time. The other is passing lock_on=1 to tell it to figure out what frame it
>> should lock onto. I mainly used this to assist in finding places where the
>> code was dropping incorrect frames. I'm not sure I really consider this
>> 'useful' for anything other than such testing where you know what should be
>> dropped. It still goes through all the computations as before but insists on
>> dropping the specified frame and noting if the computations resulted in a
>> different frame than requested.
>> 
>> I realize the attached code needs cleanup to conform to standards but I just
>> wanted to put it up for discussion. The first change above is really the
>> major change and it could (obviously) be enabled/disabled by an option to
>> decimate if desired.
>> 
>> -- Ray Cole
> 
> Whole file is unacceptable, how one can find what changed?, please
> learn how to produce patches.
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] Proposed vf_decimate enhancement

2015-09-29 Thread Carl Eugen Hoyos
Ray Cole  gmail.com> writes:

> I hope this is the appropriate place to propose an enhancement.

Yes but please read https://ffmpeg.org/developer.html

Carl Eugen

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


Re: [FFmpeg-devel] Proposed vf_decimate enhancement

2015-09-29 Thread Anshul



On 09/29/2015 12:46 PM, Paul B Mahol wrote:

On 9/29/15, Ray Cole  wrote:

I hope this is the appropriate place to propose an enhancement. This is the
first time I've offered up code to any open source project...so be gentle
:-)

First, I love ffmpeg. Wonderful software and thank you for your efforts.

I have been pulling down a number of movies back to 24 FPS (24000/1001)
using fieldmatch and decimate. However decimate seemed to drop incorrect
frames from time-to-time particularly on scenes with little motion. The
pullup filter likewise made poor decisions under similar circumstances.

So...I modified vf_decimate and it is working very well for me. I make no
claims that the enhancements would work for anyone else. My source is 1080i
29.97 fps movies recording from component video. I'm pulling down to 24 fps
(24000/1001 actually).

The changes are:
1) The total and max diffs are used to find the lowest frame to drop rather
than just the max diff. If these two methods disagree with one another then
it goes through a 'conflict resolution'. The conflict resolution checks to
see if either method matches the most commonly-dropped frame (a simple
short-term history of drops is retained for this purpose). If so, the most
commonly-dropped frame is assumed to be correct. If they do not match then
it uses the last dropped frame. This keeps the filter from varying the frame
drop so often and made a big difference in detection, at least with the
stuff I'm working with.

2) The existing vf_decimate allows frame 4 to be dropped immediately
followed by frame 0 whereas frame 3 dropped could never be followed by frame
4 dropped - similar with frames 0 through 2. Having 2 frames in a row
eligible to be dropped seems wrong and the biggest issue I had was when the
drop cycle was hitting frame 4. So I put in some code that says if the last
frame dropped was frame 4 then frame 0 and frame 1 is not eligible for drop.
If frame 3 was last dropped then frame 0 is not dropped. This enforces 2
undropped frames between drops. I'm not "married" to this...but it did help
quite a bit.

3) I had one movie where frame 4 was ALWAYS the correct frame to drop...so I
added an option to 'lock on' to a pattern in 1 of 2 ways. The first way is
for someone to pass force_drop=x where x is the frame number to drop each
time. The other is passing lock_on=1 to tell it to figure out what frame it
should lock onto. I mainly used this to assist in finding places where the
code was dropping incorrect frames. I'm not sure I really consider this
'useful' for anything other than such testing where you know what should be
dropped. It still goes through all the computations as before but insists on
dropping the specified frame and noting if the computations resulted in a
different frame than requested.

I realize the attached code needs cleanup to conform to standards but I just
wanted to put it up for discussion. The first change above is really the
major change and it could (obviously) be enabled/disabled by an option to
decimate if desired.

-- Ray Cole


Whole file is unacceptable, how one can find what changed?, please
learn how to produce patches.

To produce patch
Hint: > git add
> git commit
> git send mail or > git format   patch

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


Re: [FFmpeg-devel] Proposed vf_decimate enhancement

2015-09-29 Thread Paul B Mahol
On 9/29/15, Ray Cole  wrote:
> I hope this is the appropriate place to propose an enhancement. This is the
> first time I've offered up code to any open source project...so be gentle
> :-)
>
> First, I love ffmpeg. Wonderful software and thank you for your efforts.
>
> I have been pulling down a number of movies back to 24 FPS (24000/1001)
> using fieldmatch and decimate. However decimate seemed to drop incorrect
> frames from time-to-time particularly on scenes with little motion. The
> pullup filter likewise made poor decisions under similar circumstances.
>
> So...I modified vf_decimate and it is working very well for me. I make no
> claims that the enhancements would work for anyone else. My source is 1080i
> 29.97 fps movies recording from component video. I'm pulling down to 24 fps
> (24000/1001 actually).
>
> The changes are:
> 1) The total and max diffs are used to find the lowest frame to drop rather
> than just the max diff. If these two methods disagree with one another then
> it goes through a 'conflict resolution'. The conflict resolution checks to
> see if either method matches the most commonly-dropped frame (a simple
> short-term history of drops is retained for this purpose). If so, the most
> commonly-dropped frame is assumed to be correct. If they do not match then
> it uses the last dropped frame. This keeps the filter from varying the frame
> drop so often and made a big difference in detection, at least with the
> stuff I'm working with.
>
> 2) The existing vf_decimate allows frame 4 to be dropped immediately
> followed by frame 0 whereas frame 3 dropped could never be followed by frame
> 4 dropped - similar with frames 0 through 2. Having 2 frames in a row
> eligible to be dropped seems wrong and the biggest issue I had was when the
> drop cycle was hitting frame 4. So I put in some code that says if the last
> frame dropped was frame 4 then frame 0 and frame 1 is not eligible for drop.
> If frame 3 was last dropped then frame 0 is not dropped. This enforces 2
> undropped frames between drops. I'm not "married" to this...but it did help
> quite a bit.
>
> 3) I had one movie where frame 4 was ALWAYS the correct frame to drop...so I
> added an option to 'lock on' to a pattern in 1 of 2 ways. The first way is
> for someone to pass force_drop=x where x is the frame number to drop each
> time. The other is passing lock_on=1 to tell it to figure out what frame it
> should lock onto. I mainly used this to assist in finding places where the
> code was dropping incorrect frames. I'm not sure I really consider this
> 'useful' for anything other than such testing where you know what should be
> dropped. It still goes through all the computations as before but insists on
> dropping the specified frame and noting if the computations resulted in a
> different frame than requested.
>
> I realize the attached code needs cleanup to conform to standards but I just
> wanted to put it up for discussion. The first change above is really the
> major change and it could (obviously) be enabled/disabled by an option to
> decimate if desired.
>
> -- Ray Cole
>

Whole file is unacceptable, how one can find what changed?, please
learn how to produce patches.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


[FFmpeg-devel] Proposed vf_decimate enhancement

2015-09-28 Thread Ray Cole

I hope this is the appropriate place to propose an enhancement. This is the 
first time I've offered up code to any open source project...so be gentle :-)

First, I love ffmpeg. Wonderful software and thank you for your efforts.

I have been pulling down a number of movies back to 24 FPS (24000/1001) using 
fieldmatch and decimate. However decimate seemed to drop incorrect frames from 
time-to-time particularly on scenes with little motion. The pullup filter 
likewise made poor decisions under similar circumstances.

So...I modified vf_decimate and it is working very well for me. I make no 
claims that the enhancements would work for anyone else. My source is 1080i 
29.97 fps movies recording from component video. I'm pulling down to 24 fps 
(24000/1001 actually).

The changes are:
1) The total and max diffs are used to find the lowest frame to drop rather 
than just the max diff. If these two methods disagree with one another then it 
goes through a 'conflict resolution'. The conflict resolution checks to see if 
either method matches the most commonly-dropped frame (a simple short-term 
history of drops is retained for this purpose). If so, the most 
commonly-dropped frame is assumed to be correct. If they do not match then it 
uses the last dropped frame. This keeps the filter from varying the frame drop 
so often and made a big difference in detection, at least with the stuff I'm 
working with.

2) The existing vf_decimate allows frame 4 to be dropped immediately followed by frame 0 
whereas frame 3 dropped could never be followed by frame 4 dropped - similar with frames 
0 through 2. Having 2 frames in a row eligible to be dropped seems wrong and the biggest 
issue I had was when the drop cycle was hitting frame 4. So I put in some code that says 
if the last frame dropped was frame 4 then frame 0 and frame 1 is not eligible for drop. 
If frame 3 was last dropped then frame 0 is not dropped. This enforces 2 undropped frames 
between drops. I'm not "married" to this...but it did help quite a bit.

3) I had one movie where frame 4 was ALWAYS the correct frame to drop...so I 
added an option to 'lock on' to a pattern in 1 of 2 ways. The first way is for 
someone to pass force_drop=x where x is the frame number to drop each time. The 
other is passing lock_on=1 to tell it to figure out what frame it should lock 
onto. I mainly used this to assist in finding places where the code was 
dropping incorrect frames. I'm not sure I really consider this 'useful' for 
anything other than such testing where you know what should be dropped. It 
still goes through all the computations as before but insists on dropping the 
specified frame and noting if the computations resulted in a different frame 
than requested.

I realize the attached code needs cleanup to conform to standards but I just 
wanted to put it up for discussion. The first change above is really the major 
change and it could (obviously) be enabled/disabled by an option to decimate if 
desired.

-- Ray Cole
/*
 * Copyright (c) 2012 Fredrik Mellbin
 * Copyright (c) 2013 Clément Bœsch
 *
 * This file is part of FFmpeg.
 *
 * FFmpeg is free software; you can redistribute it and/or
 * modify it under the terms of the GNU Lesser General Public
 * License as published by the Free Software Foundation; either
 * version 2.1 of the License, or (at your option) any later version.
 *
 * FFmpeg is distributed in the hope that it will be useful,
 * but WITHOUT ANY WARRANTY; without even the implied warranty of
 * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
 * Lesser General Public License for more details.
 *
 * You should have received a copy of the GNU Lesser General Public
 * License along with FFmpeg; if not, write to the Free Software
 * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
 */

#include "libavutil/opt.h"
#include "libavutil/pixdesc.h"
#include "libavutil/timestamp.h"
#include "avfilter.h"
#include "internal.h"

#define INPUT_MAIN 0
#define INPUT_CLEANSRC 1

struct qitem {
AVFrame *frame;
int64_t maxbdiff;
int64_t totdiff;
};

typedef struct {
const AVClass *class;
struct qitem *queue;///< window of cycle frames and the associated data diff
int fid;///< current frame id in the queue
int filled; ///< 1 if the queue is filled, 0 otherwise
AVFrame *last;  ///< last frame from the previous queue
AVFrame **clean_src;///< frame queue for the clean source
int got_frame[2];   ///< frame request flag for each input stream
double ts_unit; ///< timestamp units for the output frames
int64_t start_pts;  ///< base for output timestamps
uint32_t eof;   ///< bitmask for end of stream
int hsub, vsub; ///< chroma subsampling values
int depth;
int nxblocks, nyblocks;
int bdiffsize;
int64_t *bdiffs;

/* Ray */
int lastdrop;
int64_t drop_count[5]; // drop