On Tue, Nov 29, 2011 at 10:49 PM, Nathan Caldwell <[email protected]> wrote:
>On Mon, Nov 28, 2011 at 12:56 AM, Thad Ward <[email protected]> wrote: >> Also, how does one get samples added to the fate test sample set? I've been >> creating tests for lagarith's various frame types. > >That's good to hear. I have almost no samples it will be nice to have >some regression tests. Do you have any 4:2:2 or 4:4:4 samples? Not currently. I've only got two samples currently, One with 48 frames of RGB24 and the other with 48 frames of RGBA, and those are ones I made myself with virtualdub and the reference lagarith codec. I plan on making a couple that test the various solid-color modes, and changing the RGBA clip to have various alpha values, rather than all opaque. > >> diff --git a/libavcodec/lagarith.c b/libavcodec/lagarith.c >> index 1aa9ec3..0595d57 100644 >> --- a/libavcodec/lagarith.c >> +++ b/libavcodec/lagarith.c >> @@ -447,7 +447,7 @@ static int lag_decode_frame(AVCodecContext *avctx, >> uint32_t offset_gu = 0, offset_bv = 0, offset_ry = 9; >> int offs[4]; >> uint8_t *srcs[4], *dst; >> - int i, j; >> + int i, j, planes; > >Initialize planes to 3. > >> AVFrame *picture = data; >> >> @@ -479,7 +479,17 @@ static int lag_decode_frame(AVCodecContext *avctx, >> } >> break; >> case FRAME_ARITH_RGBA: >> - avctx->pix_fmt = PIX_FMT_RGB32; >> + case FRAME_ARITH_RGB24: >> + if (FRAME_ARITH_RGB24 == frametype) { >> + avctx->pix_fmt = PIX_FMT_RGB24; >> + planes = 3; >> + offs[2] = 9; >> + } else if(FRAME_ARITH_RGBA == frametype) { >> + avctx->pix_fmt = PIX_FMT_RGB32; >> + planes = 4; >> + offs[2] = 13; >> + offs[3] = AV_RL32(buf + 9); >> + } > >Once you start with planes = 3, you can simplify these branches by >setting planes = 4, incrementing offset_ry by 4 and correctly setting >offs[3] only in the RGBA case. Also, what Kostya said about ordering >of conditions and spacing here, and below. I've generally gotten in the habit of putting the constant on the left, as any C compiler will complain loudly if I accidentally leave out one of the equals signs. I've swapped it already in a second submission of this patch after Kostya's comments. > >> if (avctx->get_buffer(avctx, p) < 0) { >> av_log(avctx, AV_LOG_ERROR, "get_buffer() failed\n"); >> @@ -487,26 +497,24 @@ static int lag_decode_frame(AVCodecContext *avctx, >> } >> offs[0] = offset_bv; >> offs[1] = offset_gu; >> - offs[2] = 13; > >You will also need to set offs[2] equal to offset_ry here. > >The rest looks good. I'll make the suggested changes and resubmit. I hadn't even noticed that unused offset_ry variable. Thad Ward _______________________________________________ libav-devel mailing list [email protected] https://lists.libav.org/mailman/listinfo/libav-devel
