[FFmpeg-devel] [PATCH]v308 and yuv4 encoders and decoders
Derek Buitenhuis
derek.buitenhuis at gmail.com
Fri Dec 30 01:38:54 CET 2011
> Hi!
>
> Attached implements v308 as defined in QuickTime and yuv4 which only exists in
> libquicktime afaict, see ticket #470 for samples.
>
> To be committed in two parts.
>
> Please comment, Carl Eugen
>
Given I reviewed part of this in private, there shouldn't be
too much in that part to review :P.
> @item v210 QuickTime uncompressed 4:2:2 10-bit @tab X @tab X
> + at item v308 QuickTime uncompressed 4:4:4 @tab X @tab X
> @item v410 QuickTime uncompressed 4:4:4 10-bit @tab X @tab X
Perhaps Align the @tab and friends? I have no idea if this makes any
difference to texi2html.
> diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
> index 690ea38..d2915ad 100644
> --- a/libavcodec/avcodec.h
> +++ b/libavcodec/avcodec.h
> @@ -259,6 +259,8 @@ enum CodecID {
> CODEC_ID_ESCAPE130 = MKBETAG('E','1','3','0'),
>
> CODEC_ID_G2M = MKBETAG( 0 ,'G','2','M'),
> + CODEC_ID_V308 = MKBETAG('V','3','0','8'),
> + CODEC_ID_YUV4 = MKBETAG('Y','U','V','4'),
This should be added as:
CODEC_ID_V308,
CODEC_ID_YUV4,
directly above:
CODEC_ID_UTVIDEO = 0x800,
> +static av_cold int v308_decode_init(AVCodecContext *avctx)
> +{
> + avctx->pix_fmt = PIX_FMT_YUV444P;
> +
> + avctx->coded_frame = avcodec_alloc_frame();
> +
> + if (!avctx->coded_frame) {
> + av_log(avctx, AV_LOG_ERROR, "Could not allocate frame.\n");
> + return AVERROR(ENOMEM);
> + }
> +
> + return 0;
> +}
The spec requires it to have an even width, so I think at
the very least, you should add a warning if it doesn't
meet this.
[...]
> + if (avpkt->size < 3 * (avctx->width + 1 >> 1) * (avctx->height + 1 >> 1)) {
> + av_log(avctx, AV_LOG_ERROR, "Insufficient input data.\n");
> + return AVERROR(EINVAL);
> + }
Given that 4:2:0 must always have an even width, I'm not sure what the
+ 1 accomplishes?
> + for (i = 0; i < (avctx->height + 1) >> 1; i++) {
> + for (j = 0; j < (avctx->width + 1) >> 1; j++) {
Ditto.
> + u[j] = *src++ ^ 0x80;
> + v[j] = *src++ ^ 0x80;
Where does the xor with 0x80 come from? Something to do with
signed-ness?
[...]
> --- a/libavformat/riff.c
> +++ b/libavformat/riff.c
> @@ -290,6 +290,8 @@ const AVCodecTag ff_codec_bmp_tags[] = {
> { CODEC_ID_VBLE, MKTAG('V', 'B', 'L', 'E') },
> { CODEC_ID_ESCAPE130, MKTAG('E', '1', '3', '0') },
> { CODEC_ID_DXTORY, MKTAG('x', 't', 'o', 'r') },
> + { CODEC_ID_V308, MKTAG('v', '3', '0', '8') },
> + { CODEC_ID_YUV4, MKTAG('y', 'u', 'v', '4') },
> { CODEC_ID_NONE, 0 }
> };
It should be possible to add these up near where v210 and v410 are,
without breaking ABI, no?
Overall looks good. :) Are you interested in adding tests to FATE
after?
- Derek
More information about the ffmpeg-devel
mailing list