[FFmpeg-devel] [PATCH 0/6] lavc/videotoolboxdec: improve HEVC stream compatibility

[FFmpeg-devel] [PATCH 0/6] lavc/videotoolboxdec: improve HEVC stream compatibility

[ffmpegagent]

 * Improve logging in error conditions
 * Improve NAL-escape function behavior
 * Fix some syntax issues in hvcc
 * Correctly escape NALs in hvcc
 * Make sure we expose the stop bits in H264 PS NALs

These changes fix support for several samples I've found or generated
locally.

rcombs (6):
  lavc/videotoolboxdec: warn on nonzero status in the callback
  lavc/videotoolboxdec: fix escaping sequential zero sequences
  lavc/videotoolboxdec: fix generating HEVC
    general_profile_compatibility_flags
  lavc/videotoolboxdec: fix writing too many 1 bits for the reserved
    fields
  lavc/videotoolboxdec: insert emu-prevention bytes for HEVC as well
  lavc/h264_ps: always include the stop bit in [s|p]ps->data

 libavcodec/h264_ps.c      |  8 ++++++++
 libavcodec/videotoolbox.c | 32 ++++++++++++++++++++------------
 2 files changed, 28 insertions(+), 12 deletions(-)


base-commit: f77ac5131cd1c623aa54ec432cba0d922e9aa470
Published-As: https://github.com/ffstaging/FFmpeg/releases/tag/pr-ffstaging-23%2Frcombs%2Fvtdec-hevc-3byte-v1
Fetch-It-Via: git fetch https://github.com/ffstaging/FFmpeg pr-ffstaging-23/rcombs/vtdec-hevc-3byte-v1
Pull-Request: https://github.com/ffstaging/FFmpeg/pull/23
-- 
ffmpeg-codebot
_______________________________________________
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".

[FFmpeg-devel] [PATCH 1/6] lavc/videotoolboxdec: warn on nonzero status in the callback

[rcombs]

From: rcombs <rcombs@rcombs.me>

Signed-off-by: rcombs <rcombs@rcombs.me>
---
 libavcodec/videotoolbox.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavcodec/videotoolbox.c b/libavcodec/videotoolbox.c
index 9083f6ff29..7c4c4c6e1b 100644
--- a/libavcodec/videotoolbox.c
+++ b/libavcodec/videotoolbox.c
@@ -691,7 +691,7 @@ static void videotoolbox_decoder_callback(void *opaque,
     }
 
     if (!image_buffer) {
-        av_log(avctx, AV_LOG_DEBUG, "vt decoder cb: output image buffer is null\n");
+        av_log(avctx, status ? AV_LOG_WARNING : AV_LOG_DEBUG, "vt decoder cb: output image buffer is null: %i\n", status);
         return;
     }
 
-- 
ffmpeg-codebot

_______________________________________________
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".

[FFmpeg-devel] [PATCH 2/6] lavc/videotoolboxdec: fix escaping sequential zero sequences

[rcombs]

From: rcombs <rcombs@rcombs.me>

This ensure that e.g. 0000000000 becomes 00000300 000300,
rather than 00000300 0000.

Signed-off-by: rcombs <rcombs@rcombs.me>
---
 libavcodec/videotoolbox.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/libavcodec/videotoolbox.c b/libavcodec/videotoolbox.c
index 7c4c4c6e1b..a1933f03f2 100644
--- a/libavcodec/videotoolbox.c
+++ b/libavcodec/videotoolbox.c
@@ -166,14 +166,13 @@ static int escape_ps(uint8_t* dst, const uint8_t* src, int src_size)
             src[i + 2] <= 0x03) {
             if (dst) {
                 *p++ = src[i++];
-                *p++ = src[i++];
+                *p++ = src[i];
                 *p++ = 0x03;
             } else {
-                i += 2;
+                i++;
             }
             size++;
-        }
-        if (dst)
+        } else if (dst)
             *p++ = src[i];
     }
 
-- 
ffmpeg-codebot

_______________________________________________
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".

[FFmpeg-devel] [PATCH 3/6] lavc/videotoolboxdec: fix generating HEVC general_profile_compatibility_flags

[rcombs]

From: rcombs <rcombs@rcombs.me>

We store this as an array of bools, not a bitfield.

Signed-off-by: rcombs <rcombs@rcombs.me>
---
 libavcodec/videotoolbox.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/libavcodec/videotoolbox.c b/libavcodec/videotoolbox.c
index a1933f03f2..c95c53fcf0 100644
--- a/libavcodec/videotoolbox.c
+++ b/libavcodec/videotoolbox.c
@@ -273,7 +273,16 @@ CFDataRef ff_videotoolbox_hvcc_extradata_create(AVCodecContext *avctx)
                  ptlc.profile_idc);
 
     /* unsigned int(32) general_profile_compatibility_flags; */
-    memcpy(p + 2, ptlc.profile_compatibility_flag, 4);
+    for (i = 0; i < 4; i++) {
+        AV_W8(p + 2 + i, ptlc.profile_compatibility_flag[i * 8] << 7 |
+                         ptlc.profile_compatibility_flag[i * 8 + 1] << 6 |
+                         ptlc.profile_compatibility_flag[i * 8 + 2] << 5 |
+                         ptlc.profile_compatibility_flag[i * 8 + 3] << 4 |
+                         ptlc.profile_compatibility_flag[i * 8 + 4] << 3 |
+                         ptlc.profile_compatibility_flag[i * 8 + 5] << 2 |
+                         ptlc.profile_compatibility_flag[i * 8 + 6] << 1 |
+                         ptlc.profile_compatibility_flag[i * 8 + 7]);
+    }
 
     /* unsigned int(48) general_constraint_indicator_flags; */
     AV_W8(p + 6, ptlc.progressive_source_flag    << 7 |
-- 
ffmpeg-codebot

_______________________________________________
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".

[FFmpeg-devel] [PATCH 4/6] lavc/videotoolboxdec: fix writing too many 1 bits for the reserved fields

[rcombs]

From: rcombs <rcombs@rcombs.me>

Signed-off-by: rcombs <rcombs@rcombs.me>
---
 libavcodec/videotoolbox.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/libavcodec/videotoolbox.c b/libavcodec/videotoolbox.c
index c95c53fcf0..921fed9619 100644
--- a/libavcodec/videotoolbox.c
+++ b/libavcodec/videotoolbox.c
@@ -328,13 +328,13 @@ CFDataRef ff_videotoolbox_hvcc_extradata_create(AVCodecContext *avctx)
      * bit(5) reserved = ‘11111’b;
      * unsigned int(3) bitDepthLumaMinus8;
      */
-    AV_W8(p + 17, (sps->bit_depth - 8) | 0xfc);
+    AV_W8(p + 17, (sps->bit_depth - 8) | 0xf8);
 
     /*
      * bit(5) reserved = ‘11111’b;
      * unsigned int(3) bitDepthChromaMinus8;
      */
-    AV_W8(p + 18, (sps->bit_depth_chroma - 8) | 0xfc);
+    AV_W8(p + 18, (sps->bit_depth_chroma - 8) | 0xf8);
 
     /* bit(16) avgFrameRate; */
     AV_WB16(p + 19, 0);
-- 
ffmpeg-codebot

_______________________________________________
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".

[FFmpeg-devel] [PATCH 5/6] lavc/videotoolboxdec: insert emu-prevention bytes for HEVC as well

[rcombs]

From: rcombs <rcombs@rcombs.me>

Fixes decoding of files with sync-fooling sequences in their PSs.

Signed-off-by: rcombs <rcombs@rcombs.me>
---
 libavcodec/videotoolbox.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/libavcodec/videotoolbox.c b/libavcodec/videotoolbox.c
index 921fed9619..ce83c2594a 100644
--- a/libavcodec/videotoolbox.c
+++ b/libavcodec/videotoolbox.c
@@ -246,7 +246,7 @@ CFDataRef ff_videotoolbox_hvcc_extradata_create(AVCodecContext *avctx)
     for (i = 0; i < HEVC_MAX_##T##PS_COUNT; i++) { \
         if (h->ps.t##ps_list[i]) { \
             const HEVC##T##PS *lps = (const HEVC##T##PS *)h->ps.t##ps_list[i]->data; \
-            vt_extradata_size += 2 + lps->data_size; \
+            vt_extradata_size += 2 + escape_ps(NULL, lps->data, lps->data_size); \
             num_##t##ps++; \
         } \
     }
@@ -369,11 +369,11 @@ CFDataRef ff_videotoolbox_hvcc_extradata_create(AVCodecContext *avctx)
     for (i = 0; i < HEVC_MAX_##T##PS_COUNT; i++) { \
         if (h->ps.t##ps_list[i]) { \
             const HEVC##T##PS *lps = (const HEVC##T##PS *)h->ps.t##ps_list[i]->data; \
+            int size = escape_ps(p + 2, lps->data, lps->data_size); \
             /* unsigned int(16) nalUnitLength; */ \
-            AV_WB16(p, lps->data_size); \
+            AV_WB16(p, size); \
             /* bit(8*nalUnitLength) nalUnit; */ \
-            memcpy(p + 2, lps->data, lps->data_size); \
-            p += 2 + lps->data_size; \
+            p += 2 + size; \
         } \
     }
 
-- 
ffmpeg-codebot

_______________________________________________
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".

[FFmpeg-devel] [PATCH 6/6] lavc/h264_ps: always include the stop bit in [s|p]ps->data

[rcombs]

From: rcombs <rcombs@rcombs.me>

The VideoToolbox hwaccel needs the entire NAL (including the stop bit),
but ff_h2645_packet_split may remove it. Detect this case by looking for
bit counts divisible by 8 and insert a stop-bit-only 0x80 byte.

Signed-off-by: rcombs <rcombs@rcombs.me>
---
 libavcodec/h264_ps.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/libavcodec/h264_ps.c b/libavcodec/h264_ps.c
index 051f06692c..e16da68dec 100644
--- a/libavcodec/h264_ps.c
+++ b/libavcodec/h264_ps.c
@@ -351,6 +351,10 @@ int ff_h264_decode_seq_parameter_set(GetBitContext *gb, AVCodecContext *avctx,
     }
     memcpy(sps->data, gb->buffer, sps->data_size);
 
+    // Re-add the removed stop bit (may be used by hwaccels).
+    if (!(gb->size_in_bits & 7) && sps->data_size < sizeof(sps->data))
+        sps->data[sps->data_size++] = 0x80;
+
     profile_idc           = get_bits(gb, 8);
     constraint_set_flags |= get_bits1(gb) << 0;   // constraint_set0_flag
     constraint_set_flags |= get_bits1(gb) << 1;   // constraint_set1_flag
@@ -775,6 +779,10 @@ int ff_h264_decode_picture_parameter_set(GetBitContext *gb, AVCodecContext *avct
     }
     memcpy(pps->data, gb->buffer, pps->data_size);
 
+    // Re-add the removed stop bit (may be used by hwaccels).
+    if (!(bit_length & 7) && pps->data_size < sizeof(pps->data))
+        pps->data[pps->data_size++] = 0x80;
+
     pps->sps_id = get_ue_golomb_31(gb);
     if ((unsigned)pps->sps_id >= MAX_SPS_COUNT ||
         !ps->sps_list[pps->sps_id]) {
-- 
ffmpeg-codebot
_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH 6/6] lavc/h264_ps: always include the stop bit in [s|p]ps->data

[Andreas Rheinhardt]

rcombs:
> From: rcombs <rcombs@rcombs.me>
> 
> The VideoToolbox hwaccel needs the entire NAL (including the stop bit),
> but ff_h2645_packet_split may remove it. Detect this case by looking for
> bit counts divisible by 8 and insert a stop-bit-only 0x80 byte.
> 
> Signed-off-by: rcombs <rcombs@rcombs.me>
> ---
>  libavcodec/h264_ps.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/libavcodec/h264_ps.c b/libavcodec/h264_ps.c
> index 051f06692c..e16da68dec 100644
> --- a/libavcodec/h264_ps.c
> +++ b/libavcodec/h264_ps.c
> @@ -351,6 +351,10 @@ int ff_h264_decode_seq_parameter_set(GetBitContext *gb, AVCodecContext *avctx,
>      }
>      memcpy(sps->data, gb->buffer, sps->data_size);
>  
> +    // Re-add the removed stop bit (may be used by hwaccels).
> +    if (!(gb->size_in_bits & 7) && sps->data_size < sizeof(sps->data))
> +        sps->data[sps->data_size++] = 0x80;
> +
>      profile_idc           = get_bits(gb, 8);
>      constraint_set_flags |= get_bits1(gb) << 0;   // constraint_set0_flag
>      constraint_set_flags |= get_bits1(gb) << 1;   // constraint_set1_flag
> @@ -775,6 +779,10 @@ int ff_h264_decode_picture_parameter_set(GetBitContext *gb, AVCodecContext *avct
>      }
>      memcpy(pps->data, gb->buffer, pps->data_size);
>  
> +    // Re-add the removed stop bit (may be used by hwaccels).
> +    if (!(bit_length & 7) && pps->data_size < sizeof(pps->data))
> +        pps->data[pps->data_size++] = 0x80;
> +
>      pps->sps_id = get_ue_golomb_31(gb);
>      if ((unsigned)pps->sps_id >= MAX_SPS_COUNT ||
>          !ps->sps_list[pps->sps_id]) {

Modifying these functions affects more than just VideoToolbox. Are you
sure that the other users of this file are ok with this change?

- Andreas
_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH 6/6] lavc/h264_ps: always include the stop bit in [s|p]ps->data

[Ridley Combs]

> On May 9, 2022, at 23:26, Andreas Rheinhardt <andreas.rheinhardt@outlook.com> wrote:
> 
> rcombs:
>> From: rcombs <rcombs@rcombs.me>
>> 
>> The VideoToolbox hwaccel needs the entire NAL (including the stop bit),
>> but ff_h2645_packet_split may remove it. Detect this case by looking for
>> bit counts divisible by 8 and insert a stop-bit-only 0x80 byte.
>> 
>> Signed-off-by: rcombs <rcombs@rcombs.me>
>> ---
>> libavcodec/h264_ps.c | 8 ++++++++
>> 1 file changed, 8 insertions(+)
>> 
>> diff --git a/libavcodec/h264_ps.c b/libavcodec/h264_ps.c
>> index 051f06692c..e16da68dec 100644
>> --- a/libavcodec/h264_ps.c
>> +++ b/libavcodec/h264_ps.c
>> @@ -351,6 +351,10 @@ int ff_h264_decode_seq_parameter_set(GetBitContext *gb, AVCodecContext *avctx,
>> }
>> memcpy(sps->data, gb->buffer, sps->data_size);
>> 
>> + // Re-add the removed stop bit (may be used by hwaccels).
>> + if (!(gb->size_in_bits & 7) && sps->data_size < sizeof(sps->data))
>> + sps->data[sps->data_size++] = 0x80;
>> +
>> profile_idc = get_bits(gb, 8);
>> constraint_set_flags |= get_bits1(gb) << 0; // constraint_set0_flag
>> constraint_set_flags |= get_bits1(gb) << 1; // constraint_set1_flag
>> @@ -775,6 +779,10 @@ int ff_h264_decode_picture_parameter_set(GetBitContext *gb, AVCodecContext *avct
>> }
>> memcpy(pps->data, gb->buffer, pps->data_size);
>> 
>> + // Re-add the removed stop bit (may be used by hwaccels).
>> + if (!(bit_length & 7) && pps->data_size < sizeof(pps->data))
>> + pps->data[pps->data_size++] = 0x80;
>> +
>> pps->sps_id = get_ue_golomb_31(gb);
>> if ((unsigned)pps->sps_id >= MAX_SPS_COUNT ||
>> !ps->sps_list[pps->sps_id]) {
> 
> Modifying these functions affects more than just VideoToolbox. Are you
> sure that the other users of this file are ok with this change?
> 
> - Andreas

It passes FATE, and only adds a byte containing the stop bit (which would be present prior to this commit in 7 of 8 possible cases), so I believe it should be fine (and conceptually including the stop bit makes more sense IMO; being inconsistent about it like this is wacky).

> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org <mailto:ffmpeg-devel@ffmpeg.org>
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel <https://ffmpeg.org/mailman/listinfo/ffmpeg-devel>
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org <mailto:ffmpeg-devel-request@ffmpeg.org> with subject "unsubscribe".

_______________________________________________
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".