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

Stefan Gehrer stefan.gehrer
Sun Sep 7 10:25:50 CEST 2008


Stefan Gehrer wrote:
> 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?

Never mind, I found both code (libavformat/avs.c and libavcodec/avs.c)
and roundup issue 252.

Stefan




More information about the ffmpeg-devel mailing list