[FFmpeg-devel] [PATCH] 1bpp and 2bpp support in QTRLE

[Michael Niedermayer]

On Fri, Sep 05, 2008 at 09:57:19AM +0200, Stefan Gehrer wrote:
> Michael Niedermayer wrote:
> > On Wed, Sep 03, 2008 at 05:48:25PM +0200, Stefan Gehrer wrote:
> >> Michael Niedermayer wrote:
> >>> On Sun, Aug 31, 2008 at 12:13:13PM +0200, Stefan Gehrer wrote:
> >>>> Hi,
> >>>>
> >>>> patch is based on Roberto's patch here
> >>>> (http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/2007-January/022046.html)
> >>>> and adds support for 1bpp and 2bpp decoding in qtrle.c.
> >>>> The samples from http://samples.mplayerhq.hu/V-codecs/QTRLE/
> >>>> play visually ok, but the 1bpp decoding produces warnings
> >>>> about trying to write pixels past the end of the image.
> >>>>
> >>>> Stefan
> >>>> Index: qtrle.c
> >>>> ===================================================================
> >>>> --- qtrle.c	(revision 15121)
> >>>> +++ qtrle.c	(working copy)
> >>> [...]
> >>>>  static void qtrle_decode_2bpp(QtrleContext *s, int stream_ptr, int 
> >>>> row_ptr, int lines_to_change)
> >>>>  {
> >> [snip]
> >>
> >>>>  }
> >>> this looks like it can be in a common function handling 4bpp as well.
> >>> it that is marked as inline gcc should optimize the bpp out.
> >> Done, new patch attached.
> >>
> >>>> @@ -387,6 +485,7 @@
> >>>>      QtrleContext *s = avctx->priv_data;
> >>>>      int header, start_line;
> >>>>      int stream_ptr, height, row_ptr;
> >>>> +    int has_palette = 0;
> >>>>       s->buf = buf;
> >>>>      s->size = buf_size;
> >>>> @@ -433,28 +532,19 @@
> >>>>      case 2:
> >>>>      case 34:
> >>>>          qtrle_decode_2bpp(s, stream_ptr, row_ptr, height);
> >>>> +        has_palette = 1;
> >>>>          break;
> >>>>       case 4:
> >>>>      case 36:
> >>>>          qtrle_decode_4bpp(s, stream_ptr, row_ptr, height);
> >>>> -        /* make the palette available on the way out */
> >>>> -        memcpy(s->frame.data[1], s->avctx->palctrl->palette, 
> >>>> AVPALETTE_SIZE);
> >>>> -        if (s->avctx->palctrl->palette_changed) {
> >>>> -            s->frame.palette_has_changed = 1;
> >>>> -            s->avctx->palctrl->palette_changed = 0;
> >>>> -        }
> >>>> +        has_palette = 1;
> >>>>          break;
> >>>>       case 8:
> >>>>      case 40:
> >>>>          qtrle_decode_8bpp(s, stream_ptr, row_ptr, height);
> >>>> -        /* make the palette available on the way out */
> >>>> -        memcpy(s->frame.data[1], s->avctx->palctrl->palette, 
> >>>> AVPALETTE_SIZE);
> >>>> -        if (s->avctx->palctrl->palette_changed) {
> >>>> -            s->frame.palette_has_changed = 1;
> >>>> -            s->avctx->palctrl->palette_changed = 0;
> >>>> -        }
> >>>> +        has_palette = 1;
> >>>>          break;
> >>>>       case 16:
> >>>> @@ -474,6 +564,16 @@
> >>>>              avctx->bits_per_sample);
> >>>>          break;
> >>>>      }
> >>>> +
> >>>> +    if(has_palette) {
> >>>> +        /* make the palette available on the way out */
> >>>> +        memcpy(s->frame.data[1], s->avctx->palctrl->palette, 
> >>>> AVPALETTE_SIZE);
> >>>> +        if (s->avctx->palctrl->palette_changed) {
> >>>> +            s->frame.palette_has_changed = 1;
> >>>> +            s->avctx->palctrl->palette_changed = 0;
> >>>> +        }
> >>>> +    }
> >>>> +
> >>>>  done:
> >>>>      *data_size = sizeof(AVFrame);
> >>>>      *(AVFrame*)data = s->frame;
> >>> The avctx->palctrl->palette_changed stuff is deprecated due to race
> >>> conditions caused by the design.
> >> I see, though I consider this patch independent of the issue as it only
> >> moves the code to a common place.
> >> As far as I can see the deprecated way of handling palettes must be
> >> fixed in the MOV demuxer first.
> > 
> > fine
> > 
> > but maybe the 1bpp case can also be merged into the 2+4 code?
> > It is a little more different so iam not sure ...
> > 
> 
> A fair part of the 1bpp-code is about line skipping which does not
> exist in the 2/4bpp code. Of the remaining 16 lines of code only
> four are an exact match with the 2/4bpp code, the rest would be
> special-cased. I tried merging but did not like it.
> Can I apply as is then?

ok


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

No human being will ever know the Truth, for even if they happen to say it
by chance, they would not even known they had done so. -- Xenophanes
-------------- 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/20080906/277269d0/attachment.pgp>