[Ffmpeg-devel] [PATCH] fix error croping

[Limin Wang]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Hi,

> [...]
> >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;
> >             }
> As I said yesterday, I believe the code is ok as it is, and this change 
> is an error. Note that here av_picture_crop() is used on the final 
> picture, to identify the area in which resampling must output.
> And the final picture is in enc->pix_fmt pixel format.

it's OK, now I understand your meaning now.

 
> >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;
> This looks wrong (regarding the left_band) for a packed format. Is it 
> really working for you?
> For example, when using yuyv422 I have to skip 4 bytes for cropping 2 
> pixels, no?
What's your meaning? for yuyv422, the left_band must be divided by 4, 
that's 4, 8, 16, 32, etc. The reason is the data is stored as:
yuyv yuyv ..., so we can't cut them. Yes, it works for me. I have tested
with rgb24, it works also, that's limitation is left_band must be divided 
by 3.

Below is reproduced step if you want, assume you have yuv420 source:

lmwang at lmwang_pc:~/open/h264/ffmpeg$ ./ffmpeg -s 352x288 -i /home/lmwang/test_sequences/cif/foreman_cif.yuv
- -pix_fmt rgb24 /tmp/rgb.yuv


lmwang at lmwang_pc:~/open/h264/ffmpeg$ ./ffmpeg -y -s 352x288 -pix_fmt
rgb24 -i /tmp/rgb.yuv -croptop 0 -cropleft 6 -vcodec h264
/tmp/foreman_cif.h264
FFmpeg version SVN-r8622, Copyright (c) 2000-2007 Fabrice Bellard, et al.
  configuration: --enable-gpl --enable-x264 --enable-libfaac --enable-libfaad
  libavutil version: 49.4.0
  libavcodec version: 51.40.2
  libavformat version: 51.11.0
  built on Apr  4 2007 15:04:20, gcc: 4.1.2 20061115 (prerelease) (Debian
  4.1.1-21)
Input #0, rawvideo, from '/tmp/rgb.yuv':
  Duration: N/A, bitrate: N/A
  Stream #0.0: Video: rawvideo, rgb24, 352x288, 25.00 fps(r)
Output #0, h264, to '/tmp/foreman_cif.h264':
  Stream #0.0: Video: h264, yuv420p, 346x288, q=2-31, 200 kb/s, 25.00 fps(c)
Stream mapping:
  Stream #0.0 -> #0.0
[h264 @ 0x8489b30]width or height not divisible by 16 (346x288), compression
will suffer.
[h264 @ 0x8489b30]using cpu capabilities MMX MMXEXT SSE SSE2
Press [q] to stop encoding
frame=  300 fps= 43 q=-1.0 Lsize=     576kB time=12.0 bitrate= 392.9kbits/s
video:576kB audio:0kB global headers:0kB muxing overhead 0.000000%
[h264 @ 0x8489b30]slice I:25    Avg QP:29.08  size:  8685
[h264 @ 0x8489b30]slice P:275   Avg QP:31.00  size:  1349
[h264 @ 0x8489b30]mb I  I16..4: 24.5%  0.0% 75.5%
[h264 @ 0x8489b30]mb P  I16..4:  4.3%  0.0%  0.0%  P16..4: 50.8%  0.0%  0.0%
0.0%  0.0%    skip:44.9%
[h264 @ 0x8489b30]final ratefactor: 30.76
[h264 @ 0x8489b30]SSIM Mean Y:0.9203689
[h264 @ 0x8489b30]kb/s:392.0

lmwang at lmwang_pc:~/open/h264/ffmpeg$ mplayer /tmp/foreman_cif.h264


lmwang at lmwang_pc:~/open/h264/ffmpeg$ ./ffmpeg -s 352x288 -i
/home/lmwang/test_sequences/cif/foreman_cif.yuv -pix_fmt yuyv422
/tmp/yuyv422.yuv

lmwang at lmwang_pc:~/open/h264/ffmpeg$ ./ffmpeg -y -s 352x288 -pix_fmt
yuyv422 -i /tmp/yuyv422.yuv -croptop 0 -cropleft 4 -vcodec h264
/tmp/foreman_cif.h264
FFmpeg version SVN-r8622, Copyright (c) 2000-2007 Fabrice Bellard, et al.
  configuration: --enable-gpl --enable-x264 --enable-libfaac --enable-libfaad
  libavutil version: 49.4.0
  libavcodec version: 51.40.2
  libavformat version: 51.11.0
  built on Apr  4 2007 15:04:20, gcc: 4.1.2 20061115 (prerelease) (Debian
  4.1.1-21)
Input #0, rawvideo, from '/tmp/yuyv422.yuv':
  Duration: N/A, bitrate: N/A
  Stream #0.0: Video: rawvideo, yuyv422, 352x288, 25.00 fps(r)
Output #0, h264, to '/tmp/foreman_cif.h264':
  Stream #0.0: Video: h264, yuv420p, 348x288, q=2-31, 200 kb/s, 25.00 fps(c)
Stream mapping:
  Stream #0.0 -> #0.0
[h264 @ 0x8489b30]width or height not divisible by 16 (348x288), compression
will suffer.
[h264 @ 0x8489b30]using cpu capabilities MMX MMXEXT SSE SSE2
Press [q] to stop encoding
frame=  300 fps= 44 q=-1.0 Lsize=     577kB time=12.0 bitrate= 393.9kbits/s
video:577kB audio:0kB global headers:0kB muxing overhead 0.000000%

lmwang at lmwang_pc:~/open/h264/ffmpeg$ mplayer /tmp/foreman_cif.h264

> 
> Anyway, I might be confused about this... Let's see other people's comments.


Thanks,
Limin
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.6 (GNU/Linux)

iQEVAwUBRhNT0kztbf7dKiuoAQL3Dwf+PfDnDXQ4OQQffAqKMhRWYhk0kAdx7tpO
pgW9oHLnMhFEjqLD/+oXTCGP1JxSuiAFM1+WdDEYsYLtucxLCHTdndHDxP3NVOys
29oFIMH0pFAe8rlwq1qvmpsZOXtrVf1VdbLdu4eYk8UrheWTWXHgfnrkRFfoYFOY
taH3AyHj9U36rh3tL+j8Cx3VmaXrcJ9ILN4HRoYDd6Htnr0RqhM9gRizWZPdWUC6
mH4gFn1wazFQ1JdEZvKNBeBgeuPisonHPuujuVsJa3IKLFU9rsxv3PYzLDrhgm2v
0ymjdSYs4G3Ph/EtOFwP/xqwDCecErfjpJ4UympugEafHtf+kUDE7g==
=sE/s
-----END PGP SIGNATURE-----