On Sun, 3 Jun 2012, Jindřich Makovička wrote:
On Sun, Jun 3, 2012 at 3:55 AM, Michael Niedermayer <[email protected]> wrote:
Hi Jindrich
can i convince you to crosspost patches to ffmpeg-dev in the future ?
either way, ive fixed some potential security issues in your patches
in ffmpeg. Just potential, maybe they cant be triggered ... iam not
sure but i think its safer ...
also i wonder why you didnt use avio_open_dyn_buf() ?
from a quick look it should be simpler and would also avoid the
somewhat ugly and arbitrary + 1024
Hi,
Thanks for suggestions, this should be much simpler. Patch attached.
From 78a84feb21786792b7233272ac02749fd56c69cb Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jind=C5=99ich=20Makovi=C4=8Dka?= <[email protected]>
Date: Sun, 3 Jun 2012 06:11:10 +0200
Subject: [PATCH] mpegtsenc: use avio_open_dyn_buf(), zero pointers after
freeing
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
per suggestion by Michael Niedermayer
Signed-off-by: Jindřich Makovička <[email protected]>
---
libavformat/mpegtsenc.c | 52 ++++++++---------------------------------------
1 file changed, 8 insertions(+), 44 deletions(-)
diff --git a/libavformat/mpegtsenc.c b/libavformat/mpegtsenc.c
index f3520c0..423b416 100644
--- a/libavformat/mpegtsenc.c
+++ b/libavformat/mpegtsenc.c
@@ -226,10 +226,6 @@ typedef struct MpegTSWriteStream {
int64_t payload_dts;
int payload_flags;
uint8_t *payload;
-
- uint8_t *adata;
- int adata_pos;
- int adata_size;
AVFormatContext *amux;
} MpegTSWriteStream;
@@ -464,19 +460,6 @@ static void section_write_packet(MpegTSSection *s, const
uint8_t *packet)
avio_write(ctx->pb, packet, TS_PACKET_SIZE);
}
-/* Write callback for audio packetizer */
-static int mpegts_audio_write(void *opaque, uint8_t *buf, int size)
-{
- MpegTSWriteStream *ts_st = (MpegTSWriteStream *)opaque;
- if (ts_st->adata_pos + size > ts_st->adata_size)
- return AVERROR(EIO);
-
- memcpy(ts_st->adata + ts_st->adata_pos, buf, size);
- ts_st->adata_pos += size;
-
- return 0;
-}
-
static int mpegts_write_header(AVFormatContext *s)
{
MpegTSWrite *ts = s->priv_data;
@@ -577,25 +560,11 @@ static int mpegts_write_header(AVFormatContext *s)
st->codec->extradata_size > 0)
{
AVStream *ast;
- uint8_t *buffer;
- int buffer_size = 32768;
ts_st->amux = avformat_alloc_context();
if (!ts_st->amux) {
ret = AVERROR(ENOMEM);
goto fail;
}
- buffer = av_malloc(buffer_size);
- if (!buffer) {
- ret = AVERROR(ENOMEM);
- goto fail;
- }
- ts_st->amux->pb = avio_alloc_context(buffer, buffer_size,
AVIO_FLAG_WRITE,
- ts_st, NULL,
mpegts_audio_write, NULL);
- if (!ts_st->amux->pb) {
- av_free(buffer);
- ret = AVERROR(ENOMEM);
- goto fail;
- }
ts_st->amux->oformat = av_guess_format((ts->flags & MPEGTS_FLAG_AAC_LATM) ?
"latm" : "adts", NULL, NULL);
if (!ts_st->amux->oformat) {
ret = AVERROR(EINVAL);
@@ -1082,24 +1051,20 @@ static int mpegts_write_packet_internal(AVFormatContext
*s, AVPacket *pkt)
av_init_packet(&pkt2);
pkt2.data = pkt->data;
pkt2.size = pkt->size;
- ts_st->adata_size = 1024 + pkt->size;
- ts_st->adata = data = av_malloc(ts_st->adata_size);
- ts_st->adata_pos = 0;
- if (!data)
+ ret = avio_open_dyn_buf(&ts_st->amux->pb);
+ if (ret < 0)
return AVERROR(ENOMEM);
ret = av_write_frame(ts_st->amux, &pkt2);
if (ret < 0) {
+ avio_close_dyn_buf(ts_st->amux->pb, &data);
+ ts_st->amux->pb = 0;
av_free(data);
return ret;
}
- avio_flush(ts_st->amux->pb);
- if (ts_st->amux->pb->error < 0) {
- av_free(data);
- return ts_st->amux->pb->error;
- }
- buf = ts_st->adata;
- size = ts_st->adata_pos;
+ size = avio_close_dyn_buf(ts_st->amux->pb, &data);
+ ts_st->amux->pb = 0;
+ buf = data;
}
}
@@ -1180,9 +1145,8 @@ static int mpegts_write_end(AVFormatContext *s)
MpegTSWriteStream *ts_st = st->priv_data;
av_freep(&ts_st->payload);
if (ts_st->amux) {
- av_free(ts_st->amux->pb->buffer);
- av_free(ts_st->amux->pb);
avformat_free_context(ts_st->amux);
+ ts_st->amux = NULL;
}
}
You're missing the same change in mpegts_write_header (where you currently
try to free both pb->buffer and pb on failure), but I can do the that
change when I apply it, so no need to resend the patch just for that.
Other than this, looks acceptable to me, I'll apply it later if there's no
further comments.
// Martin
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel
