diff mbox series

[FFmpeg-devel,05/15] lavc/videotoolbox: escape 0x00000[0-3]s in avcC PSs

Message ID 20211113210916.49167-5-rcombs@rcombs.me
State New
Headers show
Series [FFmpeg-devel,01/15] lavu/pixfmt: add high-bit-depth semi-planar 4:2:2/4:4:4 formats | expand

Checks

Context Check Description
andriy/make_x86 success Make finished
andriy/make_fate_x86 fail Make fate failed
andriy/make_ppc success Make finished
andriy/make_fate_ppc fail Make fate failed

Commit Message

rcombs Nov. 13, 2021, 9:09 p.m. UTC 
---
 libavcodec/videotoolbox.c | 51 +++++++++++++++++++++++++++++++++------
 1 file changed, 43 insertions(+), 8 deletions(-)

Comments

Derek Buitenhuis Nov. 15, 2021, 3:25 p.m. UTC | #1 
On 11/13/2021 9:09 PM, rcombs wrote:
> ---
>  libavcodec/videotoolbox.c | 51 +++++++++++++++++++++++++++++++++------
>  1 file changed, 43 insertions(+), 8 deletions(-)

Why reimplement NAL emulation bytes for the Nth time in the codebase?

- Derek
rcombs Nov. 16, 2021, 3:30 a.m. UTC | #2 
> On Nov 15, 2021, at 09:25, Derek Buitenhuis <derek.buitenhuis@gmail.com> wrote:
> 
> On 11/13/2021 9:09 PM, rcombs wrote:
>> ---
>> libavcodec/videotoolbox.c | 51 +++++++++++++++++++++++++++++++++------
>> 1 file changed, 43 insertions(+), 8 deletions(-)
> 
> Why reimplement NAL emulation bytes for the Nth time in the codebase?

Because it was faster to write a trivial implementation than to track down an existing one. Even now that I've found a couple, one's a static in videotoolboxenc and is somewhat heavier than is warranted here, and one's in cbs_h2645 and is way heavier than is warranted. Any particular one in mind that I could use?

> 
> - Derek
> _______________________________________________
> 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".
Derek Buitenhuis Nov. 16, 2021, 2:54 p.m. UTC | #3 
On 11/16/2021 3:30 AM, Ridley Combs wrote:
> Because it was faster to write a trivial implementation than to track down an existing one.

Not a good reason...

>  Even now that I've found a couple, one's a static in videotoolboxenc and is somewhat heavier than is warranted here, and one's in cbs_h2645 and is way heavier than is warranted. Any particular one in mind that I could use?

I suspect Andreas or James have an opinion on this.

- Derek
diff mbox series

Patch

diff --git a/libavcodec/videotoolbox.c b/libavcodec/videotoolbox.c
index 49e726a75f..510b1852ba 100644
--- a/libavcodec/videotoolbox.c
+++ b/libavcodec/videotoolbox.c
@@ -140,14 +140,49 @@  int ff_videotoolbox_alloc_frame(AVCodecContext *avctx, AVFrame *frame)
 
 #define AV_W8(p, v) *(p) = (v)
 
+static int escape_ps(uint8_t* dst, const uint8_t* src, int src_size)
+{
+    int i;
+    int size = src_size;
+    uint8_t* p = dst;
+
+    for (i = 0; i < src_size; i++) {
+        if (i + 2 < src_size &&
+            src[i]     == 0x00 &&
+            src[i + 1] == 0x00 &&
+            src[i + 2] <= 0x03) {
+            if (dst) {
+                *p++ = src[i++];
+                *p++ = src[i++];
+                *p++ = 0x03;
+            }
+            size++;
+        }
+        if (dst)
+            *p++ = src[i];
+    }
+
+    if (dst)
+        av_assert0((p - dst) == size);
+
+    return size;
+}
+
 CFDataRef ff_videotoolbox_avcc_extradata_create(AVCodecContext *avctx)
 {
     VTContext *vtctx = avctx->internal->hwaccel_priv_data;
     H264Context *h = avctx->priv_data;
     CFDataRef data = NULL;
     uint8_t *p;
-    int vt_extradata_size = 6 + 2 + h->ps.sps->data_size + 3 + h->ps.pps->data_size;
-    uint8_t *vt_extradata = av_malloc(vt_extradata_size);
+    int sps_size = escape_ps(NULL, h->ps.sps->data, h->ps.sps->data_size);
+    int pps_size = escape_ps(NULL, h->ps.pps->data, h->ps.pps->data_size);
+    int vt_extradata_size;
+    uint8_t *vt_extradata;
+    int i;
+
+    vt_extradata_size = 6 + 2 + sps_size + 3 + pps_size;
+    vt_extradata = av_malloc(vt_extradata_size);
+
     if (!vt_extradata)
         return NULL;
 
@@ -159,14 +194,14 @@  CFDataRef ff_videotoolbox_avcc_extradata_create(AVCodecContext *avctx)
     AV_W8(p + 3, h->ps.sps->data[3]); /* level */
     AV_W8(p + 4, 0xff); /* 6 bits reserved (111111) + 2 bits nal size length - 3 (11) */
     AV_W8(p + 5, 0xe1); /* 3 bits reserved (111) + 5 bits number of sps (00001) */
-    AV_WB16(p + 6, h->ps.sps->data_size);
-    memcpy(p + 8, h->ps.sps->data, h->ps.sps->data_size);
-    p += 8 + h->ps.sps->data_size;
+    AV_WB16(p + 6, sps_size);
+    p += 8;
+    p += escape_ps(p, h->ps.sps->data, h->ps.sps->data_size);
     AV_W8(p + 0, 1); /* number of pps */
-    AV_WB16(p + 1, h->ps.pps->data_size);
-    memcpy(p + 3, h->ps.pps->data, h->ps.pps->data_size);
+    AV_WB16(p + 1, pps_size);
+    p += 3;
+    p += escape_ps(p, h->ps.pps->data, h->ps.pps->data_size);
 
-    p += 3 + h->ps.pps->data_size;
     av_assert0(p - vt_extradata == vt_extradata_size);
 
     // save sps header (profile/level) used to create decoder session,