| Message ID | DBAPR03MB6664A6CD42377B64E1DD794E8F909@DBAPR03MB6664.eurprd03.prod.outlook.com |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel] avcodec/libx264: Use av_memdup() where appropriate | expand |
| Context | Check | Description |
|---|---|---|
| andriy/make_x86 | success | Make finished |
| andriy/make_fate_x86 | success | Make fate finished |
| andriy/make_ppc | success | Make finished |
| andriy/make_fate_ppc | success | Make fate finished |
On 11/7/2021 8:33 AM, Andreas Rheinhardt wrote: > Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com> > --- > Will apply tonight unless there are objections. > > libavcodec/libx264.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/libavcodec/libx264.c b/libavcodec/libx264.c > index 0766b4a950..29546ebf06 100644 > --- a/libavcodec/libx264.c > +++ b/libavcodec/libx264.c > @@ -985,10 +985,9 @@ static av_cold int X264_init(AVCodecContext *avctx) > if (nal[i].i_type == NAL_SEI) { > av_log(avctx, AV_LOG_INFO, "%s\n", nal[i].p_payload+25); > x4->sei_size = nal[i].i_payload; > - x4->sei = av_malloc(x4->sei_size); > + x4->sei = av_memdup(nal[i].p_payload, x4->sei_size); > if (!x4->sei) > return AVERROR(ENOMEM); > - memcpy(x4->sei, nal[i].p_payload, nal[i].i_payload); > continue; This loop will leak x4->sei if libx264 returns a x264_nal_t array with more than one SEI NALU. I assume x264_encoder_headers() is expected to only return one such NALU with the unregistered user data SEI containing the encode settings string, and i can see looking at the source code that currently it does as much, but it's not explicit in the documentation at all (It does not even mention that it returns a SEI NALU, only SPS/PPS), so in theory it could also at any point in the future start including global metadata like mastering display or content light and not be considered an API break. So maybe the av_malloc should be replaced with av_realloc and the memcpy kept around with an offset to merge two or more SEI NALUs if present, to be safe. Also, that av_log() line printing the payload is making the same risky assumption (And the offset is wrong, it should be 24). > } > memcpy(p, nal[i].p_payload, nal[i].i_payload); >
diff --git a/libavcodec/libx264.c b/libavcodec/libx264.c index 0766b4a950..29546ebf06 100644 --- a/libavcodec/libx264.c +++ b/libavcodec/libx264.c @@ -985,10 +985,9 @@ static av_cold int X264_init(AVCodecContext *avctx) if (nal[i].i_type == NAL_SEI) { av_log(avctx, AV_LOG_INFO, "%s\n", nal[i].p_payload+25); x4->sei_size = nal[i].i_payload; - x4->sei = av_malloc(x4->sei_size); + x4->sei = av_memdup(nal[i].p_payload, x4->sei_size); if (!x4->sei) return AVERROR(ENOMEM); - memcpy(x4->sei, nal[i].p_payload, nal[i].i_payload); continue; } memcpy(p, nal[i].p_payload, nal[i].i_payload);
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com> --- Will apply tonight unless there are objections. libavcodec/libx264.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)