[FFmpeg-devel] [PATCH] Fix crop filter for paletted formats != PAL8

[Michael Niedermayer]

On Mon, Nov 09, 2009 at 09:08:10PM +0100, Stefano Sabatini wrote:
> On date Monday 2009-11-09 03:06:04 +0100, Michael Niedermayer encoded:
> > On Mon, Nov 09, 2009 at 12:13:51AM +0100, Stefano Sabatini wrote:
> > > Hi,
> > > 
> > > this is along the line of how paletted formats are treated in the
> > > scale filter, note that with my tests I didn't noticed the issue as
> > > data[1], when used with a paletted format different than pal8, only
> > > contains a long sequence of zeros, and seems unused.
> > > 
> > > Anyway with the patch applied it looks more correct, also would be
> > > useful some pointer towards a collection of paletted samples (I only
> > > found few examples in the Mplayer archive).
> > > 
> > > Regards.
> > > -- 
> > > FFmpeg = Friendly Frenzy Mythic Patchable Empowered God
> > 
> > >  vf_crop.c |    8 +++++++-
> > >  1 file changed, 7 insertions(+), 1 deletion(-)
> > > 5f43c9d456054bd95b6e713d2f11745261839ae3  fix-crop-for-pal.patch
> > > Index: libavfilter/vf_crop.c
> > > ===================================================================
> > > --- libavfilter/vf_crop.c	(revision 20479)
> > > +++ libavfilter/vf_crop.c	(working copy)
> > > @@ -33,6 +33,7 @@
> > >  
> > >      int bpp;            ///< bits per pixel
> > >      int hsub, vsub;     ///< chroma subsampling
> > > +    int input_is_pal;   ///< set a value different from 0 if the input format is paletted
> > >  } CropContext;
> > >  
> > >  static int query_formats(AVFilterContext *ctx)
> > > @@ -145,6 +146,11 @@
> > >          return -1;
> > >      }
> > >  
> > > +    crop->input_is_pal = link->format == PIX_FMT_PAL8      ||
> > > +                         link->format == PIX_FMT_BGR4_BYTE ||
> > > +                         link->format == PIX_FMT_RGB4_BYTE ||
> > > +                         link->format == PIX_FMT_BGR8      ||
> > > +                         link->format == PIX_FMT_RGB8;
> > >      return 0;
> > >  }
> > >  
> > > @@ -176,7 +182,7 @@
> > >      ref2->data[0] += crop->y * ref2->linesize[0];
> > >      ref2->data[0] += (crop->x * crop->bpp) >> 3;
> > >  
> > > -    if (link->format != PIX_FMT_PAL8) {
> > > +    if (!crop->input_is_pal) {
> > >          for (i = 1; i < 3; i ++) {
> > >              if (ref2->data[i]) {
> > >                  ref2->data[i] += (crop->y >> crop->vsub) * ref2->linesize[i];
> > 
> > why the temporary variable in the context ?
> 
> Oh well, I really don't care this or the other, I thought that it was
> better to cache the value rather than recompute it each time in

i dont think the 30 times per second calculation is an issue


> start_frame(), I also find the first version of the patch slightly
> more readable, also vf_scale.c is doing like in the first version.

hmm


> 
> Regards.
> -- 
> FFmpeg = Faboulous & Faboulous Mean Pacific Enlightening Guide

>  vf_crop.c |    6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 77273eb95f68a6f36c0da6099c4d6cca0caddf0b  fix-crop-for-pal.patch

ok

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

I wish the Xiph folks would stop pretending they've got something they
do not.  Somehow I fear this will remain a wish. -- M?ns Rullg?rd
-------------- 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/20091109/1cf5cdc6/attachment.pgp>