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

[Michael Niedermayer]

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 ...

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

The worst form of inequality is to try to make unequal things equal.
-- Aristotle
-------------- 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/20080904/88bfa73c/attachment.pgp>