On Fri, 7 Dec 2012, Janne Grunau wrote:

On 2012-12-06 23:51:03 +0200, Martin Storsjö wrote:
When called from the v4l2 input device, pix_fmt can be
AV_PIX_FMT_NONE (for jpeg formats). Before 50ba57e0, this wasn't
an issue for avpicture_get_size, but after that commit, this
lead to crashes.
---
 libavcodec/avpicture.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavcodec/avpicture.c b/libavcodec/avpicture.c
index ec4e748..3a7b7fa 100644
--- a/libavcodec/avpicture.c
+++ b/libavcodec/avpicture.c
@@ -90,7 +90,7 @@ int avpicture_get_size(enum AVPixelFormat pix_fmt, int width, 
int height)

     if ((ret = av_image_check_size(width, height, 0, NULL)) < 0)
         return ret;
-    if (desc->flags & PIX_FMT_PSEUDOPAL)
+    if (desc && desc->flags & PIX_FMT_PSEUDOPAL)
         // do not include palette for these pseudo-paletted formats
         return width * height;
     return avpicture_fill(&dummy_pict, NULL, pix_fmt, width, height);

Not really a good fix for a hack in v4l2.c. I would prefer "fixing" it
at the source like:

avpicture_get_size(st->codec->pix_fmt != AV_PIX_FMT_NONE ?
                     st->codec->pix_fmt : AV_PIX_FMT_PAL8 , s->width, 
s->height);

or find another approximation for the max jpeg frame size.

I'm not even sure that this patch works except that it prevents a
segfault.

Well, after this patch, the function behaves exactly as it did before the refactoring in 50ba57e0, so it "works" as well as it did before at least. Whether that is a good design or not is a different issue of course.

From a quick glance it looks like avpicture_fill() would
return an error if pix_fmt is AV_PIX_FMT_NONE. If that's the case we
should just error out in avpicture_get_size() on AV_PIX_FMT_NONE.

Indeed, avpicture_fill just returns an error code in this case - we can add a check here for AV_PIX_FMT_NONE here if you think that is better, but IMO checking whether desc is NULL and just not touching it in that case is less disruptive (and fewer lines of code).

// Martin
_______________________________________________
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Reply via email to