vf_fade: Make sure to not miss the last lines of a frame

Message ID 20170216103336.24159-1-martin@martin.st
State Committed
Commit 1e497344f335b4cfef72cd0447ae63c8ebe6c0fe
Headers show

Commit Message

Martin Storsjö Feb. 16, 2017, 10:33 a.m.
When slice_h is rounded up due to chroma subsampling, there's
a risk that jobnr * slice_h exceeds frame->height.

Prior to a638e9184d63, this wasn't an issue for the last slice
of a frame, since slice_end was set to frame->height for the last
slice.

a638e9184d63 tried to fix the case where other slices than the
last one would exceed frame->height (which can happen where the
number of slices/threads is very large compared to the frame
height).

However, the fix in a638e9184d63 instead broke other cases,
where slice_h * nb_threads < frame->height. Therefore, make
sure the last slice always ends at frame->height.

CC: libav-stable@libav.org
---
 libavfilter/vf_fade.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

Diego Biurrun Feb. 16, 2017, 10:43 a.m. | #1
On Thu, Feb 16, 2017 at 12:33:36PM +0200, Martin Storsjö wrote:
> When slice_h is rounded up due to chroma subsampling, there's
> a risk that jobnr * slice_h exceeds frame->height.
> 
> Prior to a638e9184d63, this wasn't an issue for the last slice
> of a frame, since slice_end was set to frame->height for the last
> slice.
> 
> a638e9184d63 tried to fix the case where other slices than the
> last one would exceed frame->height (which can happen where the
> number of slices/threads is very large compared to the frame
> height).
> 
> However, the fix in a638e9184d63 instead broke other cases,
> where slice_h * nb_threads < frame->height. Therefore, make
> sure the last slice always ends at frame->height.
> 
> CC: libav-stable@libav.org
> ---
>  libavfilter/vf_fade.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)

Fixes fate-filter-fade for me on the box where I could reproduce this.
Good work!

Diego
Anton Khirnov Feb. 16, 2017, 10:52 a.m. | #2
Quoting Martin Storsjö (2017-02-16 11:33:36)
> When slice_h is rounded up due to chroma subsampling, there's
> a risk that jobnr * slice_h exceeds frame->height.
> 
> Prior to a638e9184d63, this wasn't an issue for the last slice
> of a frame, since slice_end was set to frame->height for the last
> slice.
> 
> a638e9184d63 tried to fix the case where other slices than the
> last one would exceed frame->height (which can happen where the
> number of slices/threads is very large compared to the frame
> height).
> 
> However, the fix in a638e9184d63 instead broke other cases,
> where slice_h * nb_threads < frame->height. Therefore, make
> sure the last slice always ends at frame->height.
> 
> CC: libav-stable@libav.org
> ---
>  libavfilter/vf_fade.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/libavfilter/vf_fade.c b/libavfilter/vf_fade.c
> index eb6d82a..fd8c6ef 100644
> --- a/libavfilter/vf_fade.c
> +++ b/libavfilter/vf_fade.c
> @@ -123,7 +123,8 @@ static int filter_slice_chroma(AVFilterContext *ctx, void *arg, int jobnr,
>      AVFrame *frame = arg;
>      int slice_h     = FFALIGN(frame->height / nb_jobs, 1 << s->vsub);
>      int slice_start = jobnr * slice_h;
> -    int slice_end   = FFMIN((jobnr + 1) * slice_h, frame->height);
> +    int slice_end   = (jobnr == nb_jobs - 1) ? frame->height :
> +                                               FFMIN((jobnr + 1) * slice_h, frame->height);
>      int i, j, plane;
>  
>      for (plane = 1; plane < 3; plane++) {
> -- 
> 2.10.1 (Apple Git-78)

Looks good, thanks for the fix.

Patch

diff --git a/libavfilter/vf_fade.c b/libavfilter/vf_fade.c
index eb6d82a..fd8c6ef 100644
--- a/libavfilter/vf_fade.c
+++ b/libavfilter/vf_fade.c
@@ -123,7 +123,8 @@  static int filter_slice_chroma(AVFilterContext *ctx, void *arg, int jobnr,
     AVFrame *frame = arg;
     int slice_h     = FFALIGN(frame->height / nb_jobs, 1 << s->vsub);
     int slice_start = jobnr * slice_h;
-    int slice_end   = FFMIN((jobnr + 1) * slice_h, frame->height);
+    int slice_end   = (jobnr == nb_jobs - 1) ? frame->height :
+                                               FFMIN((jobnr + 1) * slice_h, frame->height);
     int i, j, plane;
 
     for (plane = 1; plane < 3; plane++) {