Re: [FFmpeg-devel] [PATCH] examples/demuxing_decoding: use correct size of video_dst_data[0]

2015-05-12 Thread Andreas Cadhalpun 
On 12.05.2015 21:12, Michael Niedermayer wrote:
 On Tue, May 12, 2015 at 04:02:44PM +0200, Andreas Cadhalpun wrote:
 On 12.05.2015 14:51, Michael Niedermayer wrote:
 On Tue, May 12, 2015 at 02:31:38PM +0200, Andreas Cadhalpun wrote:
 @@ -108,6 +109,14 @@ static int decode_packet(int *got_frame, int cached)
(const uint8_t **)(frame-data), 
 frame-linesize,
pix_fmt, width, height);
  
 +if ((desc-flags  AV_PIX_FMT_FLAG_PAL ||
 + desc-flags  AV_PIX_FMT_FLAG_PSEUDOPAL) 
 +video_dst_data[1] - video_dst_data[0]  
 video_dst_linesize[0] * height) {
 +/* zero-initialize the padding before the palette */
 +memset(video_dst_data[0] + video_dst_linesize[0] * 
 height, 0,
 +   video_dst_data[1] - video_dst_data[0] - 
 video_dst_linesize[0] * height);
 +}

 i wonder if this shouldnt be moved to av_image_alloc() ?

 It's a bit nicer to do this in av_image_fill_pointers.
 
 yes but thats not safe
 
 for example rawdec calls avpicture_fill() on the input buffer
 which uses av_image_fill_pointers()

OK, then let's do it in av_image_alloc.

Best regards,
Andreas

From c244cba6812bcb15f871b72e721fc3310f6c983f Mon Sep 17 00:00:00 2001
From: Andreas Cadhalpun andreas.cadhal...@googlemail.com
Date: Tue, 12 May 2015 21:45:42 +0200
Subject: [PATCH] imgutils: initialize palette padding bytes in av_image_alloc

av_image_fill_pointers always aligns the palette, but the padding
bytes don't (and can't) get initialized in av_image_copy.

Thus initialize them in av_image_alloc.

This fixes 'Syscall param write(buf) points to uninitialised byte(s)'
valgrind warnings.

Signed-off-by: Andreas Cadhalpun andreas.cadhal...@googlemail.com
---
 libavutil/imgutils.c | 8 
 1 file changed, 8 insertions(+)

diff --git a/libavutil/imgutils.c b/libavutil/imgutils.c
index a8bc18d..ef0e671 100644
--- a/libavutil/imgutils.c
+++ b/libavutil/imgutils.c
@@ -219,6 +219,14 @@ int av_image_alloc(uint8_t *pointers[4], int linesizes[4],
 if (desc-flags  AV_PIX_FMT_FLAG_PAL || desc-flags  AV_PIX_FMT_FLAG_PSEUDOPAL)
 avpriv_set_systematic_pal2((uint32_t*)pointers[1], pix_fmt);
 
+if ((desc-flags  AV_PIX_FMT_FLAG_PAL ||
+ desc-flags  AV_PIX_FMT_FLAG_PSEUDOPAL) 
+pointers[1] - pointers[0]  linesizes[0] * h) {
+/* zero-initialize the padding before the palette */
+memset(pointers[0] + linesizes[0] * h, 0,
+   pointers[1] - pointers[0] - linesizes[0] * h);
+}
+
 return ret;
 }
 
-- 
2.1.4

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] examples/demuxing_decoding: use correct size of video_dst_data[0]

2015-05-12 Thread Michael Niedermayer 
On Tue, May 12, 2015 at 04:02:44PM +0200, Andreas Cadhalpun wrote:
 On 12.05.2015 14:51, Michael Niedermayer wrote:
  On Tue, May 12, 2015 at 02:31:38PM +0200, Andreas Cadhalpun wrote:
  @@ -108,6 +109,14 @@ static int decode_packet(int *got_frame, int cached)
 (const uint8_t **)(frame-data), 
  frame-linesize,
 pix_fmt, width, height);
   
  +if ((desc-flags  AV_PIX_FMT_FLAG_PAL ||
  + desc-flags  AV_PIX_FMT_FLAG_PSEUDOPAL) 
  +video_dst_data[1] - video_dst_data[0]  
  video_dst_linesize[0] * height) {
  +/* zero-initialize the padding before the palette */
  +memset(video_dst_data[0] + video_dst_linesize[0] * 
  height, 0,
  +   video_dst_data[1] - video_dst_data[0] - 
  video_dst_linesize[0] * height);
  +}
  
  i wonder if this shouldnt be moved to av_image_alloc() ?
 
 It's a bit nicer to do this in av_image_fill_pointers.

yes but thats not safe

for example rawdec calls avpicture_fill() on the input buffer
which uses av_image_fill_pointers()

[...]

-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

During times of universal deceit, telling the truth becomes a
revolutionary act. -- George Orwell


signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] examples/demuxing_decoding: use correct size of video_dst_data[0]

2015-05-12 Thread Michael Niedermayer 
On Tue, May 12, 2015 at 09:48:45PM +0200, Andreas Cadhalpun wrote:
 On 12.05.2015 21:12, Michael Niedermayer wrote:
  On Tue, May 12, 2015 at 04:02:44PM +0200, Andreas Cadhalpun wrote:
  On 12.05.2015 14:51, Michael Niedermayer wrote:
  On Tue, May 12, 2015 at 02:31:38PM +0200, Andreas Cadhalpun wrote:
  @@ -108,6 +109,14 @@ static int decode_packet(int *got_frame, int cached)
 (const uint8_t **)(frame-data), 
  frame-linesize,
 pix_fmt, width, height);
   
  +if ((desc-flags  AV_PIX_FMT_FLAG_PAL ||
  + desc-flags  AV_PIX_FMT_FLAG_PSEUDOPAL) 
  +video_dst_data[1] - video_dst_data[0]  
  video_dst_linesize[0] * height) {
  +/* zero-initialize the padding before the palette */
  +memset(video_dst_data[0] + video_dst_linesize[0] * 
  height, 0,
  +   video_dst_data[1] - video_dst_data[0] - 
  video_dst_linesize[0] * height);
  +}
 
  i wonder if this shouldnt be moved to av_image_alloc() ?
 
  It's a bit nicer to do this in av_image_fill_pointers.
  
  yes but thats not safe
  
  for example rawdec calls avpicture_fill() on the input buffer
  which uses av_image_fill_pointers()
 
 OK, then let's do it in av_image_alloc.
 
 Best regards,
 Andreas
 

  imgutils.c |8 
  1 file changed, 8 insertions(+)
 d5adef96014b1b086bdebacf6301d2f8ed104625  
 0001-imgutils-initialize-palette-padding-bytes-in-av_imag.patch
 From c244cba6812bcb15f871b72e721fc3310f6c983f Mon Sep 17 00:00:00 2001
 From: Andreas Cadhalpun andreas.cadhal...@googlemail.com
 Date: Tue, 12 May 2015 21:45:42 +0200
 Subject: [PATCH] imgutils: initialize palette padding bytes in av_image_alloc

applied

thanks

[...]

-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Into a blind darkness they enter who follow after the Ignorance,
they as if into a greater darkness enter who devote themselves
to the Knowledge alone. -- Isha Upanishad


signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] examples/demuxing_decoding: use correct size of video_dst_data[0]

2015-05-12 Thread Andreas Cadhalpun 
On 12.05.2015 00:28, Michael Niedermayer wrote:
 this breaks demuxing_decoding with pixel formats that use more than
 1 plane
 
 for example:
 doc/examples/demuxing_decoding lena255.jpg out.raw /dev/null
 Could not find audio stream in input file 'lena255.jpg'
 Input #0, image2, from 'lena255.jpg':
 Duration: 00:00:00.04, start: 0.00, bitrate: 3124 kb/s
 Stream #0:0: Video: mjpeg, yuvj444p(pc, bt470bg/unknown/unknown), 
 255x255, 25 tbr, 25 tbn, 25 tbc
 Demuxing video from file 'lena255.jpg' into 'out.raw'
 video_frame n:0 coded_n:0 pts:NOPTS
 Demuxing succeeded.
 Play the output video file with the command:
 ffplay -f rawvideo -pix_fmt yuvj444p -video_size 255x255 out.raw
 
 but the printed command line for ffplay does not work as the stored
 image is not containing teh chroma planes

I see. Attached is a patch that should fix the warnings without breaking
anything. (It's not exactly beautiful, but it works.)

Best regards,
Andreas

From a52993c013dc2d64cd2f099fe472704fd2a75d6d Mon Sep 17 00:00:00 2001
From: Andreas Cadhalpun andreas.cadhal...@googlemail.com
Date: Tue, 12 May 2015 14:15:52 +0200
Subject: [PATCH] examples/demuxing_decoding: fully initialize the
 video_dst_data buffer

av_image_fill_pointers always alignes the palette, but the padding bytes
don't (and can't) get initialized in av_image_copy.

Thus initialize them explicitly.

This fixes 'Syscall param write(buf) points to uninitialised byte(s)'
valgrind warnings.

Signed-off-by: Andreas Cadhalpun andreas.cadhal...@googlemail.com
---
 doc/examples/demuxing_decoding.c | 9 +
 1 file changed, 9 insertions(+)

diff --git a/doc/examples/demuxing_decoding.c b/doc/examples/demuxing_decoding.c
index feeeb96..4678604 100644
--- a/doc/examples/demuxing_decoding.c
+++ b/doc/examples/demuxing_decoding.c
@@ -71,6 +71,7 @@ static int decode_packet(int *got_frame, int cached)
 {
 int ret = 0;
 int decoded = pkt.size;
+const AVPixFmtDescriptor *desc = av_pix_fmt_desc_get(pix_fmt);
 
 *got_frame = 0;
 
@@ -108,6 +109,14 @@ static int decode_packet(int *got_frame, int cached)
   (const uint8_t **)(frame-data), frame-linesize,
   pix_fmt, width, height);
 
+if ((desc-flags  AV_PIX_FMT_FLAG_PAL ||
+ desc-flags  AV_PIX_FMT_FLAG_PSEUDOPAL) 
+video_dst_data[1] - video_dst_data[0]  video_dst_linesize[0] * height) {
+/* zero-initialize the padding before the palette */
+memset(video_dst_data[0] + video_dst_linesize[0] * height, 0,
+   video_dst_data[1] - video_dst_data[0] - video_dst_linesize[0] * height);
+}
+
 /* write to rawvideo file */
 fwrite(video_dst_data[0], 1, video_dst_bufsize, video_dst_file);
 }
-- 
2.1.4

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] examples/demuxing_decoding: use correct size of video_dst_data[0]

2015-05-12 Thread Michael Niedermayer 
On Tue, May 12, 2015 at 02:31:38PM +0200, Andreas Cadhalpun wrote:
 On 12.05.2015 00:28, Michael Niedermayer wrote:
  this breaks demuxing_decoding with pixel formats that use more than
  1 plane
  
  for example:
  doc/examples/demuxing_decoding lena255.jpg out.raw /dev/null
  Could not find audio stream in input file 'lena255.jpg'
  Input #0, image2, from 'lena255.jpg':
  Duration: 00:00:00.04, start: 0.00, bitrate: 3124 kb/s
  Stream #0:0: Video: mjpeg, yuvj444p(pc, bt470bg/unknown/unknown), 
  255x255, 25 tbr, 25 tbn, 25 tbc
  Demuxing video from file 'lena255.jpg' into 'out.raw'
  video_frame n:0 coded_n:0 pts:NOPTS
  Demuxing succeeded.
  Play the output video file with the command:
  ffplay -f rawvideo -pix_fmt yuvj444p -video_size 255x255 out.raw
  
  but the printed command line for ffplay does not work as the stored
  image is not containing teh chroma planes
 
 I see. Attached is a patch that should fix the warnings without breaking
 anything. (It's not exactly beautiful, but it works.)
 
 Best regards,
 Andreas
 

  demuxing_decoding.c |9 +
  1 file changed, 9 insertions(+)
 ee1725417c3d550a65ba846e9195ded4ebdf4a3d  
 0001-examples-demuxing_decoding-fully-initialize-the-vide.patch
 From a52993c013dc2d64cd2f099fe472704fd2a75d6d Mon Sep 17 00:00:00 2001
 From: Andreas Cadhalpun andreas.cadhal...@googlemail.com
 Date: Tue, 12 May 2015 14:15:52 +0200
 Subject: [PATCH] examples/demuxing_decoding: fully initialize the
  video_dst_data buffer
 
 av_image_fill_pointers always alignes the palette, but the padding bytes
 don't (and can't) get initialized in av_image_copy.
 
 Thus initialize them explicitly.
 
 This fixes 'Syscall param write(buf) points to uninitialised byte(s)'
 valgrind warnings.
 
 Signed-off-by: Andreas Cadhalpun andreas.cadhal...@googlemail.com
 ---
  doc/examples/demuxing_decoding.c | 9 +
  1 file changed, 9 insertions(+)
 
 diff --git a/doc/examples/demuxing_decoding.c 
 b/doc/examples/demuxing_decoding.c
 index feeeb96..4678604 100644
 --- a/doc/examples/demuxing_decoding.c
 +++ b/doc/examples/demuxing_decoding.c
 @@ -71,6 +71,7 @@ static int decode_packet(int *got_frame, int cached)
  {
  int ret = 0;
  int decoded = pkt.size;
 +const AVPixFmtDescriptor *desc = av_pix_fmt_desc_get(pix_fmt);
  
  *got_frame = 0;
  
 @@ -108,6 +109,14 @@ static int decode_packet(int *got_frame, int cached)
(const uint8_t **)(frame-data), frame-linesize,
pix_fmt, width, height);
  
 +if ((desc-flags  AV_PIX_FMT_FLAG_PAL ||
 + desc-flags  AV_PIX_FMT_FLAG_PSEUDOPAL) 
 +video_dst_data[1] - video_dst_data[0]  
 video_dst_linesize[0] * height) {
 +/* zero-initialize the padding before the palette */
 +memset(video_dst_data[0] + video_dst_linesize[0] * height, 0,
 +   video_dst_data[1] - video_dst_data[0] - 
 video_dst_linesize[0] * height);
 +}

i wonder if this shouldnt be moved to av_image_alloc() ?

[...]

-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

If you think the mosad wants you dead since a long time then you are either
wrong or dead since a long time.


signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] examples/demuxing_decoding: use correct size of video_dst_data[0]

2015-05-12 Thread Andreas Cadhalpun 
On 12.05.2015 11:17, wm4 wrote:
 On Mon, 11 May 2015 23:34:32 +0200
 Andreas Cadhalpun andreas.cadhal...@googlemail.com wrote:
 @@ -109,7 +108,7 @@ static int decode_packet(int *got_frame, int cached)
pix_fmt, width, height);
  
  /* write to rawvideo file */
 -fwrite(video_dst_data[0], 1, video_dst_bufsize, video_dst_file);
 +fwrite(video_dst_data[0], 1, video_dst_linesize[0] * height, 
 video_dst_file);
 
 This would still write the line padding to the file, wouldn't it?

It wouldn't, because the video_dst_data buffer has no line padding.
(That's the only purpose of this buffer, see the comment above av_image_copy.)

 This
 means it wouldn't write the full frame, _and_ each line would have
 garbage at the end of it.
 
 (Also I can't make sense of these examples...)

This one is quite useful for fuzzing, because it has much less overhead than 
ffmpeg.

Best regards,
Andreas
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] examples/demuxing_decoding: use correct size of video_dst_data[0]

2015-05-12 Thread Andreas Cadhalpun 
On 12.05.2015 14:51, Michael Niedermayer wrote:
 On Tue, May 12, 2015 at 02:31:38PM +0200, Andreas Cadhalpun wrote:
 @@ -108,6 +109,14 @@ static int decode_packet(int *got_frame, int cached)
(const uint8_t **)(frame-data), frame-linesize,
pix_fmt, width, height);
  
 +if ((desc-flags  AV_PIX_FMT_FLAG_PAL ||
 + desc-flags  AV_PIX_FMT_FLAG_PSEUDOPAL) 
 +video_dst_data[1] - video_dst_data[0]  
 video_dst_linesize[0] * height) {
 +/* zero-initialize the padding before the palette */
 +memset(video_dst_data[0] + video_dst_linesize[0] * height, 
 0,
 +   video_dst_data[1] - video_dst_data[0] - 
 video_dst_linesize[0] * height);
 +}
 
 i wonder if this shouldnt be moved to av_image_alloc() ?

It's a bit nicer to do this in av_image_fill_pointers.
New patch attached.

Best regards,
Andreas

From 636d367e35363cb3388897aeded7d836d381cb11 Mon Sep 17 00:00:00 2001
From: Andreas Cadhalpun andreas.cadhal...@googlemail.com
Date: Tue, 12 May 2015 15:51:21 +0200
Subject: [PATCH] imgutils: initialize palette padding bytes in
 av_image_fill_pointers

av_image_fill_pointers always aligns the palette, but the padding
bytes don't (and can't) get initialized in av_image_copy.

Thus initialize them in av_image_fill_pointers.

This fixes 'Syscall param write(buf) points to uninitialised byte(s)'
valgrind warnings.

Signed-off-by: Andreas Cadhalpun andreas.cadhal...@googlemail.com
---
 libavutil/imgutils.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/libavutil/imgutils.c b/libavutil/imgutils.c
index a8bc18d..bef3390 100644
--- a/libavutil/imgutils.c
+++ b/libavutil/imgutils.c
@@ -125,7 +125,11 @@ int av_image_fill_pointers(uint8_t *data[4], enum AVPixelFormat pix_fmt, int hei
 
 if (desc-flags  AV_PIX_FMT_FLAG_PAL ||
 desc-flags  AV_PIX_FMT_FLAG_PSEUDOPAL) {
+i = size[0];
 size[0] = (size[0] + 3)  ~3;
+/* zero-initialize the padding before the palette */
+if (data[0]  size[0] - i  0)
+memset(data[0] + i, 0, size[0] - i);
 data[1] = ptr + size[0]; /* palette is stored here as 256 32 bits words */
 return size[0] + 256 * 4;
 }
-- 
2.1.4

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

[FFmpeg-devel] [PATCH] examples/demuxing_decoding: use correct size of video_dst_data[0]

2015-05-11 Thread Andreas Cadhalpun 
Currently video_dst_bufsize is used, which is set to the return value
of av_image_alloc, but this is the combined size of all planes.
Only the first plane is written in fwrite.

The size of the first plane is the product of video_dst_linesize[0] and
height.

This fixes 'Syscall param write(buf) points to uninitialised byte(s)'
valgrind warnings.

Signed-off-by: Andreas Cadhalpun andreas.cadhal...@googlemail.com
---
 doc/examples/demuxing_decoding.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/doc/examples/demuxing_decoding.c b/doc/examples/demuxing_decoding.c
index feeeb96..81c8a92 100644
--- a/doc/examples/demuxing_decoding.c
+++ b/doc/examples/demuxing_decoding.c
@@ -47,7 +47,6 @@ static FILE *audio_dst_file = NULL;
 
 static uint8_t *video_dst_data[4] = {NULL};
 static int  video_dst_linesize[4];
-static int video_dst_bufsize;
 
 static int video_stream_idx = -1, audio_stream_idx = -1;
 static AVFrame *frame = NULL;
@@ -109,7 +108,7 @@ static int decode_packet(int *got_frame, int cached)
   pix_fmt, width, height);
 
 /* write to rawvideo file */
-fwrite(video_dst_data[0], 1, video_dst_bufsize, video_dst_file);
+fwrite(video_dst_data[0], 1, video_dst_linesize[0] * height, 
video_dst_file);
 }
 } else if (pkt.stream_index == audio_stream_idx) {
 /* decode audio frame */
@@ -290,7 +289,6 @@ int main (int argc, char **argv)
 fprintf(stderr, Could not allocate raw video buffer\n);
 goto end;
 }
-video_dst_bufsize = ret;
 }
 
 if (open_codec_context(audio_stream_idx, fmt_ctx, AVMEDIA_TYPE_AUDIO) = 
0) {
-- 
2.1.4

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] examples/demuxing_decoding: use correct size of video_dst_data[0]

2015-05-11 Thread Michael Niedermayer 
On Mon, May 11, 2015 at 11:34:32PM +0200, Andreas Cadhalpun wrote:
 Currently video_dst_bufsize is used, which is set to the return value
 of av_image_alloc, but this is the combined size of all planes.
 Only the first plane is written in fwrite.
 
 The size of the first plane is the product of video_dst_linesize[0] and
 height.
 
 This fixes 'Syscall param write(buf) points to uninitialised byte(s)'
 valgrind warnings.
 
 Signed-off-by: Andreas Cadhalpun andreas.cadhal...@googlemail.com
 ---
  doc/examples/demuxing_decoding.c | 4 +---
  1 file changed, 1 insertion(+), 3 deletions(-)

this breaks demuxing_decoding with pixel formats that use more than
1 plane

for example:
doc/examples/demuxing_decoding lena255.jpg out.raw /dev/null
Could not find audio stream in input file 'lena255.jpg'
Input #0, image2, from 'lena255.jpg':
Duration: 00:00:00.04, start: 0.00, bitrate: 3124 kb/s
Stream #0:0: Video: mjpeg, yuvj444p(pc, bt470bg/unknown/unknown), 
255x255, 25 tbr, 25 tbn, 25 tbc
Demuxing video from file 'lena255.jpg' into 'out.raw'
video_frame n:0 coded_n:0 pts:NOPTS
Demuxing succeeded.
Play the output video file with the command:
ffplay -f rawvideo -pix_fmt yuvj444p -video_size 255x255 out.raw

but the printed command line for ffplay does not work as the stored
image is not containing teh chroma planes


[...]
-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

It is what and why we do it that matters, not just one of them.


signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel