Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
 help / color / mirror / Atom feed
* [FFmpeg-devel] [PATCH 1/2] avcodec/proresenc: make transparency honored in mov/QT
@ 2023-12-26 22:25 Clément Bœsch
  2023-12-26 22:25 ` [FFmpeg-devel] [PATCH 2/2] avcodec/proresenc: fix alpha plane encoding bitstream Clément Bœsch
  2023-12-27 11:54 ` [FFmpeg-devel] [PATCH 1/2] avcodec/proresenc: make transparency honored in mov/QT Stefano Sabatini
  0 siblings, 2 replies; 5+ messages in thread
From: Clément Bœsch @ 2023-12-26 22:25 UTC (permalink / raw)
  To: ffmpeg-devel; +Cc: Clément Bœsch

In the mov muxer (in mov_write_video_tag()), bits_per_coded_sample will
be written under certain conditions and is required to be 32 for the
transparency to be honored in QuickTime.

prores_kostya already has this setting but prores_anatoliy and
prores_videotoolbox didn't.
---
 libavcodec/proresenc_anatoliy.c | 3 +++
 libavcodec/videotoolboxenc.c    | 2 ++
 2 files changed, 5 insertions(+)

diff --git a/libavcodec/proresenc_anatoliy.c b/libavcodec/proresenc_anatoliy.c
index a1cb46aa59..16741afd68 100644
--- a/libavcodec/proresenc_anatoliy.c
+++ b/libavcodec/proresenc_anatoliy.c
@@ -930,6 +930,9 @@ static av_cold int prores_encode_init(AVCodecContext *avctx)
         }
     }
 
+    if (ctx->need_alpha)
+        avctx->bits_per_coded_sample = 32;
+
     ff_fdctdsp_init(&ctx->fdsp, avctx);
 
     avctx->codec_tag = AV_RL32((const uint8_t*)profiles[avctx->profile].name);
diff --git a/libavcodec/videotoolboxenc.c b/libavcodec/videotoolboxenc.c
index 15e0e1fe29..644fd60b00 100644
--- a/libavcodec/videotoolboxenc.c
+++ b/libavcodec/videotoolboxenc.c
@@ -521,6 +521,8 @@ static CMVideoCodecType get_cm_codec_type(AVCodecContext *avctx,
         }
         return kCMVideoCodecType_HEVC;
     case AV_CODEC_ID_PRORES:
+        if (desc && (desc->flags & AV_PIX_FMT_FLAG_ALPHA))
+            avctx->bits_per_coded_sample = 32;
         switch (profile) {
         case AV_PROFILE_PRORES_PROXY:
             return MKBETAG('a','p','c','o'); // kCMVideoCodecType_AppleProRes422Proxy
-- 
2.43.0

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [FFmpeg-devel] [PATCH 2/2] avcodec/proresenc: fix alpha plane encoding bitstream
  2023-12-26 22:25 [FFmpeg-devel] [PATCH 1/2] avcodec/proresenc: make transparency honored in mov/QT Clément Bœsch
@ 2023-12-26 22:25 ` Clément Bœsch
  2023-12-27 12:29   ` Stefano Sabatini
  2023-12-27 11:54 ` [FFmpeg-devel] [PATCH 1/2] avcodec/proresenc: make transparency honored in mov/QT Stefano Sabatini
  1 sibling, 1 reply; 5+ messages in thread
From: Clément Bœsch @ 2023-12-26 22:25 UTC (permalink / raw)
  To: ffmpeg-devel; +Cc: Clément Bœsch

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);
 }
 
 static int encode_slice(AVCodecContext *avctx, const AVFrame *pic,
-- 
2.43.0

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [FFmpeg-devel] [PATCH 1/2] avcodec/proresenc: make transparency honored in mov/QT
  2023-12-26 22:25 [FFmpeg-devel] [PATCH 1/2] avcodec/proresenc: make transparency honored in mov/QT Clément Bœsch
  2023-12-26 22:25 ` [FFmpeg-devel] [PATCH 2/2] avcodec/proresenc: fix alpha plane encoding bitstream Clément Bœsch
@ 2023-12-27 11:54 ` Stefano Sabatini
  1 sibling, 0 replies; 5+ messages in thread
From: Stefano Sabatini @ 2023-12-27 11:54 UTC (permalink / raw)
  To: FFmpeg development discussions and patches; +Cc: Clément Bœsch

On date Tuesday 2023-12-26 23:25:15 +0100, Clément Bœsch wrote:
> In the mov muxer (in mov_write_video_tag()), bits_per_coded_sample will
> be written under certain conditions and is required to be 32 for the
> transparency to be honored in QuickTime.
> 
> prores_kostya already has this setting but prores_anatoliy and
> prores_videotoolbox didn't.
> ---
>  libavcodec/proresenc_anatoliy.c | 3 +++
>  libavcodec/videotoolboxenc.c    | 2 ++
>  2 files changed, 5 insertions(+)
> 
> diff --git a/libavcodec/proresenc_anatoliy.c b/libavcodec/proresenc_anatoliy.c
> index a1cb46aa59..16741afd68 100644
> --- a/libavcodec/proresenc_anatoliy.c
> +++ b/libavcodec/proresenc_anatoliy.c
> @@ -930,6 +930,9 @@ static av_cold int prores_encode_init(AVCodecContext *avctx)
>          }
>      }
>  
> +    if (ctx->need_alpha)
> +        avctx->bits_per_coded_sample = 32;
> +
>      ff_fdctdsp_init(&ctx->fdsp, avctx);
>  
>      avctx->codec_tag = AV_RL32((const uint8_t*)profiles[avctx->profile].name);
> diff --git a/libavcodec/videotoolboxenc.c b/libavcodec/videotoolboxenc.c
> index 15e0e1fe29..644fd60b00 100644
> --- a/libavcodec/videotoolboxenc.c
> +++ b/libavcodec/videotoolboxenc.c
> @@ -521,6 +521,8 @@ static CMVideoCodecType get_cm_codec_type(AVCodecContext *avctx,
>          }
>          return kCMVideoCodecType_HEVC;
>      case AV_CODEC_ID_PRORES:
> +        if (desc && (desc->flags & AV_PIX_FMT_FLAG_ALPHA))
> +            avctx->bits_per_coded_sample = 32;
>          switch (profile) {
>          case AV_PROFILE_PRORES_PROXY:
>              return MKBETAG('a','p','c','o'); // kCMVideoCodecType_AppleProRes422Proxy

LGTM.
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [FFmpeg-devel] [PATCH 2/2] avcodec/proresenc: fix alpha plane encoding bitstream
  2023-12-26 22:25 ` [FFmpeg-devel] [PATCH 2/2] avcodec/proresenc: fix alpha plane encoding bitstream Clément Bœsch
@ 2023-12-27 12:29   ` Stefano Sabatini
  2023-12-29 19:55     ` Clément Bœsch
  0 siblings, 1 reply; 5+ messages in thread
From: Stefano Sabatini @ 2023-12-27 12:29 UTC (permalink / raw)
  To: FFmpeg development discussions and patches; +Cc: Clément Bœsch

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.
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [FFmpeg-devel] [PATCH 2/2] avcodec/proresenc: fix alpha plane encoding bitstream
  2023-12-27 12:29   ` Stefano Sabatini
@ 2023-12-29 19:55     ` Clément Bœsch
  0 siblings, 0 replies; 5+ messages in thread
From: Clément Bœsch @ 2023-12-29 19:55 UTC (permalink / raw)
  To: FFmpeg development discussions and patches

On Wed, Dec 27, 2023 at 01:29:58PM +0100, Stefano Sabatini wrote:
[...]
> LGTM, thanks.

Thank you for the review. I will add to the commit description that this
change was made possible because of paid work for the jitter company
(https://jitter.video). If there is no objection, I will push this small
patchset in the coming days with the description amended.

-- 
Clément B.
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2023-12-29 19:55 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-12-26 22:25 [FFmpeg-devel] [PATCH 1/2] avcodec/proresenc: make transparency honored in mov/QT Clément Bœsch
2023-12-26 22:25 ` [FFmpeg-devel] [PATCH 2/2] avcodec/proresenc: fix alpha plane encoding bitstream Clément Bœsch
2023-12-27 12:29   ` Stefano Sabatini
2023-12-29 19:55     ` Clément Bœsch
2023-12-27 11:54 ` [FFmpeg-devel] [PATCH 1/2] avcodec/proresenc: make transparency honored in mov/QT Stefano Sabatini

Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \
		ffmpegdev@gitmailbox.com
	public-inbox-index ffmpegdev

Example config snippet for mirrors.


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git