[FFmpeg-devel] [PATCH 3/3] Revert "avcodec/h264_mp4toannexb_bsf: fix missing PS before IDR frames"
Michael Niedermayer
michael at niedermayer.cc
Wed Mar 20 04:19:26 EET 2024
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
Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
Signed-off-by: Michael Niedermayer <michael at niedermayer.cc>
---
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
More information about the ffmpeg-devel
mailing list