[FFmpeg-devel] [PATCH 2/2] avcodec/proresenc: fix alpha plane encoding bitstream

[Stefano Sabatini]

On date Tuesday 2023-12-26 23:25:16 +0100, Clément Bœsch wrote:
> These functions encode a slice of alpha (1 to 8 macroblocks) which are
> expected to be encoded as a repeated sequence of "[diff][run-1]", where
> diff is the running difference of the alpha value and run is how many
> times that value is expected to be duplicated (within the limit of a
> grand total of 2048 unpacked samples, corresponding to a slice of 8 MB).
> 
> Even when run==0 (the run variable semantic is actually "run minus 1"),
> there is always a diff previously encoded that needs a counter of at
> least 1. This means we need to call put_alpha_run() unconditionally at
> the end of the bitstream to account for the last running diff.
> 
> This commit fixes glitchy playbacks on QuickTime with M2 and M3 hardware
> (but not M1 for some mysterious reason) with files generated with
> commands such as:
> 
>   ffmpeg -f lavfi -i testsrc2=d=5:s=912x320,chromakey -c:v prores_aw -profile:v 4    -y aw.mov
>   ffmpeg -f lavfi -i testsrc2=d=5:s=912x320,chromakey -c:v prores_ks -profile:v 4444 -y ks.mov
> 
> The glitch expresses itself deterministically as blinking black
> rectangles on random frames (for example on frame 21, 54, 71, 79, ...).
> 
> Even with the proresdec from FFmpeg, overreads actually happens while
> reading the run-minus-1 value (around val = get_bits(gb, 4) in
> unpack_alpha()). This doesn't seem to cause any particular issue because
> it simply overreads into the next slice, and because the decoder is
> resilient, but it's still a problem.
> 
> Fixes ticket #10255.
> ---
>  libavcodec/proresenc_anatoliy.c | 3 +--
>  libavcodec/proresenc_kostya.c   | 3 +--
>  2 files changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/libavcodec/proresenc_anatoliy.c b/libavcodec/proresenc_anatoliy.c
> index 16741afd68..9b9ffa03be 100644
> --- a/libavcodec/proresenc_anatoliy.c
> +++ b/libavcodec/proresenc_anatoliy.c
> @@ -486,8 +486,7 @@ static av_always_inline int encode_alpha_slice_data(AVCodecContext *avctx, int8_
>              run++;
>          }
>      } while (idx < num_coeffs);
> -    if (run)
> -        put_alpha_run(&pb, run);
> +    put_alpha_run(&pb, run);
>      flush_put_bits(&pb);
>      *a_data_size = put_bytes_output(&pb);
>  
> diff --git a/libavcodec/proresenc_kostya.c b/libavcodec/proresenc_kostya.c
> index e904632f8e..8d45e42d1a 100644
> --- a/libavcodec/proresenc_kostya.c
> +++ b/libavcodec/proresenc_kostya.c
> @@ -562,8 +562,7 @@ static void encode_alpha_plane(ProresContext *ctx, PutBitContext *pb,
>              run++;
>          }
>      } while (idx < num_coeffs);
> -    if (run)
> -        put_alpha_run(pb, run);
> +    put_alpha_run(pb, run);
>  }

LGTM, thanks.