[FFmpeg-devel] [PATCH] V210 decoder and encoder

[Baptiste Coudurier]

On 5/11/2009 12:27 PM, Reimar D?ffinger wrote:
> On Sat, May 09, 2009 at 01:30:40PM -0700, Baptiste Coudurier wrote:
>> +static av_cold int decode_init(AVCodecContext *avctx)
>> +{
>> +    if (avctx->width & 1) {
>> +        av_log(avctx, AV_LOG_ERROR, "v210 needs even width\n");
>> +        return -1;
>> +    }
> 
> Why, does something actually break when width is odd? Doesn't look like it to me...
> On a second look, the last Y value probably wouldn't be read...

Specs are this way. Code doesn't claim to support something else.

>> +    int aligned_width = ((avctx->width + 47) / 48) * 48;
>> +    int stride = aligned_width * 8 / 3;
>> +
>> +    if (avpkt->size < aligned_width * avctx->height * 8 / 3) {
> 
> Uh, why not stride * ... like almost all other raw decoders do i think?

Changed.

>> +        av_log(avctx, AV_LOG_ERROR, "packet too small\n");
>> +        return -1;
>> +    }
>> +
>> +    pic->reference = 0;
>> +    if (avctx->get_buffer(avctx, pic) < 0)
>> +        return -1;
>> +
>> +    ydst = pic->data[0];
>> +    udst = pic->data[1];
>> +    vdst = pic->data[2];
>> +    yend = ydst + avctx->width;
>> +    pic->pict_type = FF_I_TYPE;
>> +    pic->key_frame = 1;
>> +
>> +    for (y = 0; y < avctx->height; y++) {
>> +        const uint32_t *src = psrc;
>> +        uint32_t v;
>> +        for (x = 0; x < avctx->width - 5; x += 6) {
>> +            v= le2me_32(*src++);
>> +            *udst++ = (v & 0x3FF)      <<  6;
>> +            *ydst++ = (v & 0xFFC00)    >>  4;
>> +            *vdst++ = (v & 0x3FF00000) >> 14;
> 
> Both
> 
>> +            *udst++ = (v & 0x000003FF) <<  6;
>> +            *ydst++ = (v & 0x000FFC00) >>  4;
>> +            *vdst++ = (v & 0x3FF00000) >> 14;
> 
> and
> 
>> +            *udst++ = (v <<  6) & 0xFFC0;
>> +            *ydst++ = (v >>  4) & 0xFFC0;
>> +            *vdst++ = (v >> 14) & 0xFFC0;
> 
> Seem nicer to me.

I don't get what you mean.

> I think the later one might have a speed advantage due to needing only one
> constant.
> Also like in the encoder you don't really need one of the ands.

You mean ydst >> 14 ? What if the 2 bits are not zero ? Doesn't the bit
gets replicated ?

[...]

-- 
Baptiste COUDURIER                              GnuPG Key Id: 0x5C1ABAAA
Key fingerprint                 8D77134D20CC9220201FC5DB0AC9325C5C1ABAAA
FFmpeg maintainer                                  http://www.ffmpeg.org