From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org [79.124.17.100]) by master.gitmailbox.com (Postfix) with ESMTP id 5AF6249778 for ; Wed, 20 Mar 2024 06:41:32 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 90D7868D333; Wed, 20 Mar 2024 08:41:29 +0200 (EET) Received: from out203-205-221-245.mail.qq.com (out203-205-221-245.mail.qq.com [203.205.221.245]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 4F67268D2F1 for ; Wed, 20 Mar 2024 08:41:21 +0200 (EET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=foxmail.com; s=s201512; t=1710916876; bh=3Wzw60xrbBfgSWLkJhHBTWzFA3I4Az2fLo937oGEquY=; h=From:Subject:Date:References:To:In-Reply-To; b=aVoMxM/hyCjK8+lCJO03oHqp/dFVkauuUPzftvUcpb+ke4nAsadu50LwBj9gIQhb3 /f+J0Dz8ZZVJJkhxKWY4orFSoFP8orCBesjdexB29m6LO/Dw1r2czLjV/zJj06EdP2 63t+yRmBat1toesyBiCDMCWVS4wyO3sCeugUUg6U= Received: from smtpclient.apple ([119.147.10.205]) by newxmesmtplogicsvrsza10-0.qq.com (NewEsmtp) with SMTP id A4F27480; Wed, 20 Mar 2024 14:41:15 +0800 X-QQ-mid: xmsmtpt1710916875tukq261a7 Message-ID: X-QQ-XMAILINFO: MQ+wLuVvI2LQdUYbpBrgrk8r55sHkgJORZoQQ9qVI81UQsr4TegPWUgQd1tjrj WF7hNlbkXJqdlTj+T44FVSnY6Gc9SS0mXfVIo/Mko93XUM9U+iq1AXN5Xhnw/mhpZaUhvYwkRgAu r91l/W2M2HUCDxL77iH0Y4icL7L2tpPTC5EgOI9K7Ix1eBhBvOTtrTmgqZWpqABwNnrw/vVf2Kiq FjAzt0uq4F2G/EGphfY9kOIFzK+VfURt46eP8FIjM4WPQ7PlnxPsgWkaOo9io1Zu1lsI5h/T9hM4 y0Jxbc8cn5pd+oQ9fIwa+9+ZYCiFmLSgSvmkji68ov0bDbZdPIzdA9r3hXJVPP4p49PzgnxwY9Nv B3+NxLgoOAJtLPoIJZAwLjICROWvagCCK34y6BwlOIUz8zVnUJpPq8XnG8p5QtYBgIyOMKl5TCjU Cu6vkso02tasV112VBIqS0E+juf85lZq7DP3Z0PhYMK1sDzpDKR5dR7SAHIUBPaBonNVZ7p+tMzV Lx/GFOwu3QtPT7mgUVaGvJX3h85guSeLd1iWTP8UsdIdemV1Jn5lEPmOOejhifVAN4oG+x8y5HOK +YTROhkFo64Z0zMYxzUDSAiB7NLpoFwcUTH3ABquQ8EwpYIhE8atplp/pBryLMD6xasVjW7PH+Pj NXzxwX2ThL59Z9f7WMEPQVOglsj500BLYtDJea+WZX5BirwLBJYb3fgD60N/abBCVRCF+AmJfvao +UUAbzSuKCHowDzqpVOEHq205Tw//LsIV7nrnNEY+0RpZJnRaT35euAUtpc8ChJebsr9iyjVB0Vh 28Gut9DPotTHzuLOkMfkq4qbRbNlrJT1ONPAHn5MFq6O59pQ6z+JPOkgCjFTLeSBKsfLjbkam88m mJRn8luvNMpEHXfYqfNV9B6uWMhBQGboAMmBmUz84m7moDwiPexJ8NEmeQ5l3tAv5Pfb19jd7hv1 Dqyivnr5s6sgjiU7bPhUow7+IMicy4F73letoL6jTgIUcFtqOAO/72HWdaqSKVz2wGrcUD/+aBjz gbqtpeU/4SP7KDHpieY0Ew9VikqOo= X-QQ-XMRINFO: MPJ6Tf5t3I/ycC2BItcBVIA= From: Zhao Zhili Mime-Version: 1.0 (Mac OS X Mail 16.0 \(3774.200.91.1.1\)) Date: Wed, 20 Mar 2024 14:41:05 +0800 References: <20240320021926.3759-1-michael@niedermayer.cc> <20240320021926.3759-3-michael@niedermayer.cc> To: FFmpeg development discussions and patches In-Reply-To: <20240320021926.3759-3-michael@niedermayer.cc> X-OQ-MSGID: <9B7B3A38-62BF-4E84-8398-6F5CCF97F5A0@foxmail.com> X-Mailer: Apple Mail (2.3774.200.91.1.1) Subject: Re: [FFmpeg-devel] [PATCH 3/3] Revert "avcodec/h264_mp4toannexb_bsf: fix missing PS before IDR frames" X-BeenThere: ffmpeg-devel@ffmpeg.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: FFmpeg development discussions and patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: FFmpeg development discussions and patches Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" Archived-At: List-Archive: List-Post: > On Mar 20, 2024, at 10:19, Michael Niedermayer wrote: > > This reverts commit d3aa0cd16f5e952bc346b7c74b4dcba95151a63a. > > Fixes: out of array write > Fixes: 64407/clusterfuzz-testcase-minimized-ffmpeg_BSF_H264_MP4TOANNEXB_fuzzer-4966763443650560 > > The bsf code performs 2 iterations, the first counts how much space is needed > than allocates > and the 2nd pass copies into teh allocated space > > The reverted code reallocates sps/pps in the first pass in a data dependant way that leaves > the 2nd pass in a different state then the first Sorry for the break. How to access the fuzz report details? Without the patch, it generates broken files in those cases. I want to dig further to fix it. > > Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg > Signed-off-by: Michael Niedermayer > --- > libavcodec/bsf/h264_mp4toannexb.c | 83 +++---------------------------- > tests/fate/h264.mak | 5 -- > 2 files changed, 6 insertions(+), 82 deletions(-) > > diff --git a/libavcodec/bsf/h264_mp4toannexb.c b/libavcodec/bsf/h264_mp4toannexb.c > index 120241c892..b99de39ce9 100644 > --- a/libavcodec/bsf/h264_mp4toannexb.c > +++ b/libavcodec/bsf/h264_mp4toannexb.c > @@ -36,8 +36,6 @@ typedef struct H264BSFContext { > uint8_t *pps; > int sps_size; > int pps_size; > - unsigned sps_buf_size; > - unsigned pps_buf_size; > uint8_t length_size; > uint8_t new_idr; > uint8_t idr_sps_seen; > @@ -133,33 +131,16 @@ pps: > memset(out + total_size, 0, padding); > > if (pps_offset) { > - uint8_t *sps; > - > + s->sps = out; > s->sps_size = pps_offset; > - sps = av_fast_realloc(s->sps, &s->sps_buf_size, s->sps_size); > - if (!sps) { > - av_free(out); > - return AVERROR(ENOMEM); > - } > - s->sps = sps; > - memcpy(s->sps, out, s->sps_size); > } else { > av_log(ctx, AV_LOG_WARNING, > "Warning: SPS NALU missing or invalid. " > "The resulting stream may not play.\n"); > } > if (pps_offset < total_size) { > - uint8_t *pps; > - > + s->pps = out + pps_offset; > s->pps_size = total_size - pps_offset; > - pps = av_fast_realloc(s->pps, &s->pps_buf_size, s->pps_size); > - if (!pps) { > - av_freep(&s->sps); > - av_free(out); > - return AVERROR(ENOMEM); > - } > - s->pps = pps; > - memcpy(s->pps, out + pps_offset, s->pps_size); > } else { > av_log(ctx, AV_LOG_WARNING, > "Warning: PPS NALU missing or invalid. " > @@ -179,35 +160,6 @@ pps: > return 0; > } > > -static int h264_mp4toannexb_save_ps(uint8_t **dst, int *dst_size, > - unsigned *dst_buf_size, > - const uint8_t *nal, uint32_t nal_size, > - int first) > -{ > - static const uint8_t nalu_header[4] = { 0, 0, 0, 1 }; > - const int start_code_size = sizeof(nalu_header); > - uint8_t *ptr; > - uint32_t size; > - > - if (first) > - size = 0; > - else > - size = *dst_size; > - > - ptr = av_fast_realloc(*dst, dst_buf_size, size + nal_size + start_code_size); > - if (!ptr) > - return AVERROR(ENOMEM); > - > - memcpy(ptr + size, nalu_header, start_code_size); > - size += start_code_size; > - memcpy(ptr + size, nal, nal_size); > - size += nal_size; > - > - *dst = ptr; > - *dst_size = size; > - return 0; > -} > - > static int h264_mp4toannexb_init(AVBSFContext *ctx) > { > int extra_size = ctx->par_in->extradata_size; > @@ -268,9 +220,6 @@ static int h264_mp4toannexb_filter(AVBSFContext *ctx, AVPacket *opkt) > if (j) \ > av_log(__VA_ARGS__) > for (int j = 0; j < 2; j++) { > - int sps_count = 0; > - int pps_count = 0; > - > buf = in->data; > new_idr = s->new_idr; > sps_seen = s->idr_sps_seen; > @@ -301,18 +250,8 @@ static int h264_mp4toannexb_filter(AVBSFContext *ctx, AVPacket *opkt) > > if (unit_type == H264_NAL_SPS) { > sps_seen = new_idr = 1; > - if (!j) { > - h264_mp4toannexb_save_ps(&s->sps, &s->sps_size, &s->sps_buf_size, > - buf, nal_size, !sps_count); > - sps_count++; > - } > } else if (unit_type == H264_NAL_PPS) { > pps_seen = new_idr = 1; > - if (!j) { > - h264_mp4toannexb_save_ps(&s->pps, &s->pps_size, &s->pps_buf_size, > - buf, nal_size, !pps_count); > - pps_count++; > - } > /* if SPS has not been seen yet, prepend the AVCC one to PPS */ > if (!sps_seen) { > if (!s->sps_size) { > @@ -332,10 +271,9 @@ static int h264_mp4toannexb_filter(AVBSFContext *ctx, AVPacket *opkt) > > /* prepend only to the first type 5 NAL unit of an IDR picture, if no sps/pps are already present */ > if (new_idr && unit_type == H264_NAL_IDR_SLICE && !sps_seen && !pps_seen) { > - if (s->sps_size) > - count_or_copy(&out, &out_size, s->sps, s->sps_size, PS_OUT_OF_BAND, j); > - if (s->pps_size) > - count_or_copy(&out, &out_size, s->pps, s->pps_size, PS_OUT_OF_BAND, j); > + if (ctx->par_out->extradata) > + count_or_copy(&out, &out_size, ctx->par_out->extradata, > + ctx->par_out->extradata_size, PS_OUT_OF_BAND, j); > new_idr = 0; > /* if only SPS has been seen, also insert PPS */ > } else if (new_idr && unit_type == H264_NAL_IDR_SLICE && sps_seen && !pps_seen) { > @@ -351,7 +289,7 @@ static int h264_mp4toannexb_filter(AVBSFContext *ctx, AVPacket *opkt) > else > ps = PS_NONE; > count_or_copy(&out, &out_size, buf, nal_size, ps, j); > - if (unit_type == H264_NAL_SLICE) { > + if (!new_idr && unit_type == H264_NAL_SLICE) { > new_idr = 1; > sps_seen = 0; > pps_seen = 0; > @@ -391,14 +329,6 @@ fail: > return ret; > } > > -static void h264_mp4toannexb_close(AVBSFContext *ctx) > -{ > - H264BSFContext *s = ctx->priv_data; > - > - av_freep(&s->sps); > - av_freep(&s->pps); > -} > - > static void h264_mp4toannexb_flush(AVBSFContext *ctx) > { > H264BSFContext *s = ctx->priv_data; > @@ -418,6 +348,5 @@ const FFBitStreamFilter ff_h264_mp4toannexb_bsf = { > .priv_data_size = sizeof(H264BSFContext), > .init = h264_mp4toannexb_init, > .filter = h264_mp4toannexb_filter, > - .close = h264_mp4toannexb_close, > .flush = h264_mp4toannexb_flush, > }; > diff --git a/tests/fate/h264.mak b/tests/fate/h264.mak > index 674054560b..d0c57eabe9 100644 > --- a/tests/fate/h264.mak > +++ b/tests/fate/h264.mak > @@ -227,7 +227,6 @@ FATE_H264-$(call FRAMECRC, MOV, H264) += fate-h264-twofields-packet > FATE_H264-$(call DEMMUX, MOV, H264, H264_MP4TOANNEXB_BSF SCALE_FILTER) += fate-h264-bsf-mp4toannexb-new-extradata > > FATE_H264-$(call DEMMUX, MOV, H264, H264_MP4TOANNEXB_BSF) += fate-h264-bsf-mp4toannexb \ > - fate-h264-bsf-mp4toannexb-2 \ > fate-h264_mp4toannexb_ticket5927 \ > fate-h264_mp4toannexb_ticket5927_2 \ > > @@ -432,10 +431,6 @@ fate-h264-conformance-sva_nl1_b: CMD = framecrc -i $(TARGET_SAM > fate-h264-conformance-sva_nl2_e: CMD = framecrc -i $(TARGET_SAMPLES)/h264-conformance/SVA_NL2_E.264 > > fate-h264-bsf-mp4toannexb: CMD = md5 -i $(TARGET_SAMPLES)/h264/interlaced_crop.mp4 -c:v copy -f h264 > -# First IDR is prefixed by SPS/PPS > -fate-h264-bsf-mp4toannexb-2: CMD = md5 -i $(TARGET_SAMPLES)/h264/ps_prefix_first_idr.mp4 -c:v copy -f h264 > -fate-h264-bsf-mp4toannexb-2: CMP = oneline > -fate-h264-bsf-mp4toannexb-2: REF = cffcfa6a2d0b58c9de1f5785f099f41d > fate-h264-bsf-mp4toannexb-new-extradata: CMD = stream_remux mov $(TARGET_SAMPLES)/h264/extradata-reload-multi-stsd.mov "" h264 "-map 0:v" > fate-h264_mp4toannexb_ticket5927: CMD = transcode "mp4" $(TARGET_SAMPLES)/h264/thezerotheorem-cut.mp4 \ > h264 "-c:v copy -bsf:v h264_mp4toannexb -an" "-c:v copy" > -- > 2.17.1 > > _______________________________________________ > 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 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".