[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