[Ffmpeg-devel] [PATCH] fix error croping

[Limin Wang]

Hi,

> >>I do not think this is ok... At this point, the image is still in the 
> >>decoder pixel format. Telling av_picture_crop() that it is in a 
> >>different format might lead to a wrong result.
> >>
> >>I think the correct solution is to implement support for the desired 
> >>pixel format in av_picture_crop().
> >
> >So the next picture pading should use enc->pix_fmt instead of dec->pix_fmt?
> As far as I remember, the "filtering order" is:
> decoder ---> crop ---> rescale / convert pixfmt ---> pad ---> ....
> 
> So, cropping happens with the decoder pixfmt, and padding operates on 
> the encoder pixfmt. This is why I believe the current code is ok.

Thanks for your explanation, it helps a lot.

After investigating, I found out packed format can crop without more
change. That's why I change to enc->pix_fmt, it can crop the picture without
problem. Please review the patch. 

I didn't test other packed format, only yuyv422 is tested. However I think
other packed formats should be work also. For yuyv422, the crop left must
be divided by 4, or the picture color will be problem.

In addition, I still think ffmpeg.c line 733, enc->pix_fmt should be dec->pix_fmt,
for it's done before rescale processing.


Thanks,
Limin

-------------- next part --------------
Index: ffmpeg.c
===================================================================
--- ffmpeg.c	(revision 8622)
+++ ffmpeg.c	(working copy)
@@ -730,7 +730,7 @@
     if (ost->video_pad) {
         final_picture = &ost->pict_tmp;
         if (ost->video_resample) {
-            if (av_picture_crop((AVPicture *)&picture_pad_temp, (AVPicture *)final_picture, enc->pix_fmt, ost->padtop, ost->padleft) < 0) {
+            if (av_picture_crop((AVPicture *)&picture_pad_temp, (AVPicture *)final_picture, dec->pix_fmt, ost->padtop, ost->padleft) < 0) {
                 av_log(NULL, AV_LOG_ERROR, "error padding picture\n");
                 return;
             }
Index: libavcodec/imgconvert.c
===================================================================
--- libavcodec/imgconvert.c	(revision 8622)
+++ libavcodec/imgconvert.c	(working copy)
@@ -2220,19 +2220,22 @@
     int y_shift;
     int x_shift;
 
-    if (pix_fmt < 0 || pix_fmt >= PIX_FMT_NB || !is_yuv_planar(&pix_fmt_info[pix_fmt]))
+    if (pix_fmt < 0 || pix_fmt >= PIX_FMT_NB)
         return -1;
 
-    y_shift = pix_fmt_info[pix_fmt].y_chroma_shift;
-    x_shift = pix_fmt_info[pix_fmt].x_chroma_shift;
-
     dst->data[0] = src->data[0] + (top_band * src->linesize[0]) + left_band;
-    dst->data[1] = src->data[1] + ((top_band >> y_shift) * src->linesize[1]) + (left_band >> x_shift);
-    dst->data[2] = src->data[2] + ((top_band >> y_shift) * src->linesize[2]) + (left_band >> x_shift);
+    dst->linesize[0] = src->linesize[0];
+    if( is_yuv_planar(&pix_fmt_info[pix_fmt])) {
+        y_shift = pix_fmt_info[pix_fmt].y_chroma_shift;
+        x_shift = pix_fmt_info[pix_fmt].x_chroma_shift;
 
-    dst->linesize[0] = src->linesize[0];
-    dst->linesize[1] = src->linesize[1];
-    dst->linesize[2] = src->linesize[2];
+        dst->data[1] = src->data[1] + ((top_band >> y_shift) * src->linesize[1]) + (left_band >> x_shift);
+        dst->data[2] = src->data[2] + ((top_band >> y_shift) * src->linesize[2]) + (left_band >> x_shift);
+
+        dst->linesize[1] = src->linesize[1];
+        dst->linesize[2] = src->linesize[2];
+    }
+
     return 0;
 }
 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 481 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20070404/f348254a/attachment.pgp>