Re: [FFmpeg-devel] [PATCH] avformat/bit: return early from write_packet if pkt-size is 0

2015-02-26 Thread Nicolas George
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

2015-02-26 Thread Nicolas George
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

2015-02-26 Thread Andreas Cadhalpun

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

2015-02-26 Thread Andreas Cadhalpun

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

2015-02-26 Thread Michael Niedermayer
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