On Wed, Dec 07, 2011 at 11:01:22PM +0000, Paul B. Mahol wrote:
> On 12/7/11, Diego Biurrun <[email protected]> wrote:
> > On Tue, Dec 06, 2011 at 03:26:37AM +0000, Paul B. Mahol wrote:
> >> This patch implements CLJR encoder.
> >
> > Nice to see somebody finish that vestigial encoder.
> >
> >> --- a/libavformat/riff.c
> >> +++ b/libavformat/riff.c
> >> @@ -266,7 +266,7 @@ const AVCodecTag ff_codec_bmp_tags[] = {
> >> { CODEC_ID_PNG, MKTAG('M', 'P', 'N', 'G') },
> >> { CODEC_ID_PNG, MKTAG('P', 'N', 'G', '1') },
> >> - { CODEC_ID_CLJR, MKTAG('c', 'l', 'j', 'r') },
> >> + { CODEC_ID_CLJR, MKTAG('C', 'L', 'J', 'R') },
> >> { CODEC_ID_DIRAC, MKTAG('d', 'r', 'a', 'c') },
> >
> > Won't this have unwanted sideeffects when decoding files with that
> > fourcc?
>
> Nope, that is incorrect fourcc (according to multimedia.cx wiki).
> That line with that fourcc is apparently only used when encoding.
> With this change mplayer can play avconv encoded stuff.
OK
> --- a/libavcodec/cljr.c
> +++ b/libavcodec/cljr.c
> @@ -157,6 +164,8 @@ AVCodec ff_cljr_encoder = {
> .priv_data_size = sizeof(CLJRContext),
> .init = encode_init,
> .encode = encode_frame,
> - .long_name = NULL_IF_CONFIG_SMALL("Cirrus Logic AccuPak"),
> + .pix_fmts = (const enum PixelFormat[]) { PIX_FMT_YUV411P,
> + PIX_FMT_NONE},
> + .long_name = NULL_IF_CONFIG_SMALL("Cirrus Logic AccuPak"),
nit: space before }
Overall, this is looking good, thank you.
All it's missing is a libavcodec minor version bump.
Have you tested that it compiles with
configure --disable-everything --enable-encoder=cljr
?
Another thing - could you send me a git-formatted patch or use
git-send-email? That makes it much easier to apply your patch
with correct author information etc...
Diego
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel
