[FFmpeg-devel] [PATCH] simplify crop/pad handling in ffmpeg.c

[Hervé]

Le 16 avr. 08 ? 13:38, Luca Abeni a ?crit :
> Hi,
>
> Diego Biurrun wrote:
>> This is a patch from Herv? Flores to simplify crop/pad handling in
>> ffmpeg.c.  For your consideration.
> [...]
>
>> Index: ffmpeg.c
>> ===================================================================
>> --- ffmpeg.c	(revision 12847)
>> +++ ffmpeg.c	(working copy)
>> @@ -2262,10 +2262,6 @@
>>         fprintf(stderr, "Incorrect top crop size\n");
>>         av_exit(1);
>>     }
>> -    if ((frame_topBand % 2) != 0) {
>> -        fprintf(stderr, "Top crop size must be a multiple of 2\n");
>> -        av_exit(1);
>> -    }
>
> Sorry; maybe I am missing something, but I see this check removed  
> from here
> and not re-introduced in other places... Do we support cropping and  
> padding
> with non-multiple-of-2 quantities?

the idea of this patch (not from me but from a guy from pixar, sent in  
the list a long time ago) was to permit an odd value for pad and crop.
If your file has an odd height, ffmpeg does not handle it

with this patch you can pad (or crop) only one pixel to have a final  
even value

usually, each parameter check for even value an exit if value is odd

with this patch, the only check is done with the resulting value :
-  if height - crop + pad  = even, so no problem
-  if height - crop + pad  = odd, exit

PS: I don't remember exactly what was the pb and how to reproduce it,  
but if I well remeber, this patch have a pb with chroma and interlaced  
streams (bad resample, no "swap"/reinterlacing of the chroma channel)
eg: file has an even height, if you -padbottom 1 and -padtop 1 => no  
good chroma resample

another bug with this patch:  file has an even height, if you - 
cropbottom 1 and -padbottom 1 => no change (no pad, no crop)

I think the easiest way to correct this patch will be: final check in  
the patch shouldn't permit odd pad/crop if the file have a native even  
value => exit

(PS: I'm not a C coder, I just manage to read it a little, that's all)
bye

Herv?