[FFmpeg-devel] [PATCH] Export large crop values via AVCodecContext

[Michael Niedermayer]

On Sat, Mar 13, 2010 at 06:31:03AM -0500, David Conrad wrote:
> On Jun 3, 2009, at 6:48 PM, Michael Niedermayer wrote:
> 
> > On Wed, Jun 03, 2009 at 06:43:33PM -0400, David Conrad wrote:
> >> On Jun 3, 2009, at 11:34 AM, Michael Niedermayer wrote:
> >> 
> >>> On Thu, May 28, 2009 at 09:10:45PM -0400, David Conrad wrote:
> >>>> Hi,
> >>>> 
> >>>> As far as I can tell, it's impractical to crop large values internally,
> >>>> since buffers returned from decode_frame must have the same dimensions as
> >>>> AVCodecContext, which also must be the coded dimensions (or up to 15 
> >>>> less)
> >>>> in order for get_buffer to allocate buffers of the needed size.
> >>>> 
> >>>> Thus, the only solution I see is to export these values and leave it to
> >>>> individual applications that care about such offsets to deal with it
> >>>> themselves.
> >>> 
> >>> I think these values can be exported through AVCodecContext.pan_scan
> >> 
> >> From my understanding of mpeg-2 (the only place AVPanScan is currently 
> >> used), the area(s) in a AVPanScan are the more important areas in a frame 
> >> that shouldn't be cropped e.g. when changing aspect ratio, while these 
> >> cropping offsets aren't intended to be displayed ever.
> > 
> > My interpretation of mpeg2 is that AVPanScan is exactly the rectangle that
> > is intended for display of a given aspect ratio.
> > So i would guess that a 16:9 video could contain a single 4:3 panscan
> > rectangle
> > 
> > 
> >> 
> >> Also, H.264 has separate pan-scan fields than what are exported here, 
> >> relative to this cropping (D.2.3 Pan-scan rectangle SEI message semantics)
> > 
> > I know, thats why i hesitated with my reply
> > but the crop values really are easier to use if they use the same API,
> > the crop rectangle maybe should be somehow marked as overriding the
> > main rectangle by default.
> 
> Seems I forgot about this.
> 

> commit 69c96e8da510a2758b66b89138931844d09c3e99
> Author: David Conrad <lessen42 at gmail.com>
> Date:   Sat Mar 13 06:27:10 2010 -0500
> 
>     Export cropping larger than than 15 pixels and/or on the left/top
> 
> diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
> index bc7c0a0..366af95 100644
> --- a/libavcodec/avcodec.h
> +++ b/libavcodec/avcodec.h
> @@ -30,7 +30,7 @@
>  #include "libavutil/avutil.h"
>  
>  #define LIBAVCODEC_VERSION_MAJOR 52
> -#define LIBAVCODEC_VERSION_MINOR 58
> +#define LIBAVCODEC_VERSION_MINOR 59
>  #define LIBAVCODEC_VERSION_MICRO  0
>  
>  #define LIBAVCODEC_VERSION_INT  AV_VERSION_INT(LIBAVCODEC_VERSION_MAJOR, \
> @@ -2596,6 +2596,14 @@ typedef struct AVCodecContext {
>       * - decoding: unused
>       */
>      int weighted_p_pred;
> +
> +    /**
> +     * If id is nonzero, this is the region of the frame that should be displayed.
> +     * Only position[0] is used for the offset.
> +     * - encoding: unused
> +     * - decoding: Set by libavcodec
> +     */
> +    AVPanScan clean_area;
>  } AVCodecContext;

i think pan_scan should be used, rather id could be set to a special value
to indicate that the position is important and it could be the first
area specified

also including it as you do makes extending AVPanScan impossible it would
break ABI

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

Let us carefully observe those good qualities wherein our enemies excel us
and endeavor to excel them, by avoiding what is faulty, and imitating what
is excellent in them. -- Plutarch
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20100313/9c962d89/attachment.pgp>