On Wed, Nov 30, 2011 at 03:29:49AM -0800, Thad Ward wrote:
> 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.
Not in a case below though (where output is converted from planar to packed).
And IMO condition looks more natural when expressed "if this variable value is
equal to X" than "if X is a value of this variable".
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel
