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

Stefan Gehrer stefan.gehrer
Sun Sep 7 09:42:14 CEST 2008


Michael Niedermayer wrote:
> 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

Applied.

I also looked at replacing the deprecated AVPalette stuff. At its
definition there is a note to use AVPacket instead. Is there actually
any code that does handle palettes in the new way? If not, is there
any mail thread or roundup issue that explains the necessary changes
better?

Stefan




More information about the ffmpeg-devel mailing list