Re: [FFmpeg-devel] [PATCH] avformat/bit: return early from write_packet if pkt-size is 0
L'octidi 8 ventôse, an CCXXIII, Andreas Cadhalpun a écrit : the bit format muxer currently segfaults, when it is passed a packet of size 0. This can be triggered e.g. with: ffmpeg -y -f lavfi -i sine=duration=0.1 -c:a flac -f bit /dev/null Attached patch fixes this. This looks wrong. The bit muxer seems to assume packets have a constant size, apparently 10 octets. Someone knowing the format should check, but I suppose this would be more correct: if (pkt-size != 10) return AVERROR(EINVAL); Also, it seems you should not be able to mux FLAC into this format, the write header callback does not validate enough. It looks like this muxer is a quick-and-dirty implementation, emphasis on the dirty. Last: how come ffmpeg/lavf tries to mux an empty packet? Regards, -- Nicolas George signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avformat/bit: return early from write_packet if pkt-size is 0
L'octidi 8 ventôse, an CCXXIII, Nicolas George a écrit : Last: how come ffmpeg/lavf tries to mux an empty packet? It comes from that commit: commit 0957b274e312e985d69cb490bee2a7ff820acaa6 Author: Anton Khirnov an...@khirnov.net Date: 2014-04-29 15:06:45 +0200 lavc: add an option to enable side data-only packets during encoding Some encoders (e.g. flac) need to send side data when there is no more data to be output. This enables them to output a packet with no data in it, only side data. IMHO, the empty packet should not be reaching the demuxers that do not expect them, that would change the output. I suggest you check with Anton. Regards, -- Nicolas George signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avformat/bit: return early from write_packet if pkt-size is 0
On 26.02.2015 20:48, Nicolas George wrote: L'octidi 8 ventôse, an CCXXIII, Andreas Cadhalpun a écrit : the bit format muxer currently segfaults, when it is passed a packet of size 0. This can be triggered e.g. with: ffmpeg -y -f lavfi -i sine=duration=0.1 -c:a flac -f bit /dev/null Attached patch fixes this. This looks wrong. The bit muxer seems to assume packets have a constant size, apparently 10 octets. Someone knowing the format should check, but I suppose this would be more correct: if (pkt-size != 10) return AVERROR(EINVAL); You're right. New patch with this check attached. Also, it seems you should not be able to mux FLAC into this format, the write header callback does not validate enough. It looks like this muxer is a quick-and-dirty implementation, emphasis on the dirty. Indeed. I also attached a patch limiting the bit muxer to codec g729 with 1 channel, as this is apparently the only thing it supports. Last: how come ffmpeg/lavf tries to mux an empty packet? You already analyzed this. Best regards, Andreas From c882243d66af01cca9efbcb4a750c7ea60bd610b Mon Sep 17 00:00:00 2001 From: Andreas Cadhalpun andreas.cadhal...@googlemail.com Date: Thu, 26 Feb 2015 21:38:50 +0100 Subject: [PATCH 1/2] avformat/bit: check that pkt-size is 10 in write_packet Ohter packet sizes are not supported by this muxer. This avoids a null pointer dereference of pkt-data. Signed-off-by: Andreas Cadhalpun andreas.cadhal...@googlemail.com --- libavformat/bit.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/libavformat/bit.c b/libavformat/bit.c index 7b807b9..5d05da0 100644 --- a/libavformat/bit.c +++ b/libavformat/bit.c @@ -133,6 +133,9 @@ static int write_packet(AVFormatContext *s, AVPacket *pkt) GetBitContext gb; int i; +if (pkt-size != 10) +return AVERROR(EINVAL); + avio_wl16(pb, SYNC_WORD); avio_wl16(pb, 8 * 10); -- 2.1.4 From 369e4a57f9e52564f12f15fd31e1fdb0177499cd Mon Sep 17 00:00:00 2001 From: Andreas Cadhalpun andreas.cadhal...@googlemail.com Date: Thu, 26 Feb 2015 21:42:02 +0100 Subject: [PATCH 2/2] avformat/bit: only accept the g729 codec and 1 channel Other codecs/channel numbers are not supported by this muxer. Signed-off-by: Andreas Cadhalpun andreas.cadhal...@googlemail.com --- libavformat/bit.c | 8 ++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/libavformat/bit.c b/libavformat/bit.c index 5d05da0..138d2fe 100644 --- a/libavformat/bit.c +++ b/libavformat/bit.c @@ -119,8 +119,12 @@ static int write_header(AVFormatContext *s) { AVCodecContext *enc = s-streams[0]-codec; -enc-codec_id = AV_CODEC_ID_G729; -enc-channels = 1; +if ((enc-codec_id != AV_CODEC_ID_G729) || enc-channels != 1) { +av_log(s, AV_LOG_ERROR, + only codec g729 with 1 channel is supported by this format\n); +return AVERROR(EINVAL); +} + enc-bits_per_coded_sample = 16; enc-block_align = (enc-bits_per_coded_sample * enc-channels) 3; -- 2.1.4 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avformat/bit: return early from write_packet if pkt-size is 0
On 26.02.2015 21:47, Nicolas George wrote: L'octidi 8 ventôse, an CCXXIII, Nicolas George a écrit : Last: how come ffmpeg/lavf tries to mux an empty packet? It comes from that commit: commit 0957b274e312e985d69cb490bee2a7ff820acaa6 Author: Anton Khirnov an...@khirnov.net Date: 2014-04-29 15:06:45 +0200 lavc: add an option to enable side data-only packets during encoding Some encoders (e.g. flac) need to send side data when there is no more data to be output. This enables them to output a packet with no data in it, only side data. IMHO, the empty packet should not be reaching the demuxers that do not expect them, that would change the output. I suggest you check with Anton. The other muxers handle the empty packet gracefully, so I don't see a problem. Best regards, Andreas ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avformat/bit: return early from write_packet if pkt-size is 0
On Thu, Feb 26, 2015 at 10:11:37PM +0100, Andreas Cadhalpun wrote: On 26.02.2015 20:48, Nicolas George wrote: L'octidi 8 ventôse, an CCXXIII, Andreas Cadhalpun a écrit : the bit format muxer currently segfaults, when it is passed a packet of size 0. This can be triggered e.g. with: ffmpeg -y -f lavfi -i sine=duration=0.1 -c:a flac -f bit /dev/null Attached patch fixes this. This looks wrong. The bit muxer seems to assume packets have a constant size, apparently 10 octets. Someone knowing the format should check, but I suppose this would be more correct: if (pkt-size != 10) return AVERROR(EINVAL); You're right. New patch with this check attached. Also, it seems you should not be able to mux FLAC into this format, the write header callback does not validate enough. It looks like this muxer is a quick-and-dirty implementation, emphasis on the dirty. Indeed. I also attached a patch limiting the bit muxer to codec g729 with 1 channel, as this is apparently the only thing it supports. applied both patches thanks [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Observe your enemies, for they first find out your faults. -- Antisthenes signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel