[FFmpeg-devel] libavcodec: r12b decoder

Andreas Rheinhardt andreas.rheinhardt at outlook.com
Mon Jun 7 23:32:52 EEST 2021


Dennis Fleurbaaij:
> Thanks for the review Andreas!
> 
> I've addressed all your concerns besides the "& in the define", I didn't
> know that the binary AND is implicit in this situation, any link for this?
> Even if it is, I just find it much more pleasing to see the & there
> somehow, is there some leniency for personal preference?
> 

The lower nibble doesn't survive the right shift; and given that
GET_FF() returns an uint8_t, there are no bits higher than 0xFF anyway.

I have no objection to keeping the "&".

> 
> +    for (h = 0; h < avctx->height; h++) {
> +        g_dst = (uint16_t *)g_line;
> +        b_dst = (uint16_t *)b_line;
> +        r_dst = (uint16_t *)r_line;
> +

C90 disallows declarations in the middle of blocks, but it allows them
at the beginning of *any* block, not only the outermost block of a
function. You can declare these variables here.

And we also allow for loops with variable declarations, i.e. you may
declare h (and w later) via "for (int h = 0;". (This is illegal in C90.)


Your code currently does not check the size of the input packet: It
needs to be at least height * width / PIXELS_PER_BLOCK *
BYTES_PER_BLOCK, but it doesn't check that (and I guess that too big
packets are also invalid?).

- Andreas


More information about the ffmpeg-devel mailing list