[FFmpeg-devel] [PATCH 1/2] avcodec/hap: fix for missing pixels at the bottom of frames

[Michael Niedermayer]

On Wed, Jul 22, 2015 at 09:46:24PM +0100, Tom Butterworth wrote:
> A bug was introduced in 977105407cae55876041dddbf4ce0934cdd4cd6c whereby when
> frame height wasn't divisible by the number of threads, pixels would be omitted
> from the bottom rows during decode.
> ---
>  libavcodec/hap.h    |  1 +
>  libavcodec/hapdec.c | 17 +++++++++++------
>  2 files changed, 12 insertions(+), 6 deletions(-)
> 
> diff --git a/libavcodec/hap.h b/libavcodec/hap.h
> index 893b4c5..a46738e 100644
> --- a/libavcodec/hap.h
> +++ b/libavcodec/hap.h
> @@ -77,6 +77,7 @@ typedef struct HapContext {
>  
>      size_t max_snappy;       /* Maximum compressed size for snappy buffer */
>  
> +    int slice_count;         /* Number of slices for threaded operations */
>      int slice_size;          /* Optimal slice size */
>  
>      /* Pointer to the selected compress or decompress function */
> diff --git a/libavcodec/hapdec.c b/libavcodec/hapdec.c
> index c05d4c0..69a925d 100644
> --- a/libavcodec/hapdec.c
> +++ b/libavcodec/hapdec.c
> @@ -275,7 +275,12 @@ static int decompress_texture_thread(AVCodecContext *avctx, void *arg,
>      int start_slice, end_slice;
>  
>      start_slice = slice * ctx->slice_size;
> -    end_slice   = FFMIN(start_slice + ctx->slice_size, avctx->coded_height);
> +    /* If the frame height isn't divisible by the number of slices then the
> +     * last slice must cover more rows than ctx->slice_size */
> +    if (slice == ctx->slice_count - 1)
> +        end_slice = avctx->coded_height;
> +    else
> +        end_slice = FFMIN(start_slice + ctx->slice_size, avctx->coded_height);
>  
>      start_slice /= TEXTURE_BLOCK_H;
>      end_slice   /= TEXTURE_BLOCK_H;
> @@ -298,10 +303,6 @@ static int hap_decode(AVCodecContext *avctx, void *data,
>      HapContext *ctx = avctx->priv_data;
>      ThreadFrame tframe;
>      int ret, i;
> -    int slices = FFMIN(avctx->thread_count,
> -                       avctx->coded_height / TEXTURE_BLOCK_H);
> -
> -    ctx->slice_size = avctx->coded_height / slices;
>  
>      bytestream2_init(&ctx->gbc, avpkt->data, avpkt->size);
>  
> @@ -340,7 +341,7 @@ static int hap_decode(AVCodecContext *avctx, void *data,
>      }
>  
>      /* Use the decompress function on the texture, one block per thread */
> -    avctx->execute2(avctx, decompress_texture_thread, tframe.f, NULL, slices);
> +    avctx->execute2(avctx, decompress_texture_thread, tframe.f, NULL, ctx->slice_count);
>  
>      /* Frame is ready to be output */
>      tframe.f->pict_type = AV_PICTURE_TYPE_I;
> @@ -391,6 +392,10 @@ static av_cold int hap_init(AVCodecContext *avctx)
>          return AVERROR_DECODER_NOT_FOUND;
>      }
>  
> +    ctx->slice_count = av_clip(avctx->thread_count, 1,
> +                               avctx->coded_height / TEXTURE_BLOCK_H);
> +    ctx->slice_size = avctx->coded_height / ctx->slice_count;

wont this potentially result in a last slice that could be alot
larger than the other slices ?
as in height= 248 threads=32
ctx->slice_count = 32
slice_size = 7
31 slices of height 7 and 1 of height 31

it might be better to use something like

slice start = slice * height / slice_count

if iam not missing something

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

I am the wisest man alive, for I know one thing, and that is that I know
nothing. -- Socrates
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20150722/c608b156/attachment.sig>