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

[Stefan Gehrer]

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?

Stefan