[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