On Thu, Oct 18, 2012 at 04:00:02AM +0200, Luca Barbato wrote:
> Code mainly written by Carl Eugen Hoyos, Michael Niedermayer, Paul B
> Mahol.
> ---
>
> Here the initial patch, it is a tad monstruous and probably could be
> split in two chunks nicely sooner or later.
>
> libavcodec/ffv1.c | 1124
> ++++++++++++++++++++++++++++++++++++++---------------
> 1 file changed, 815 insertions(+), 309 deletions(-)
>
> diff --git a/libavcodec/ffv1.c b/libavcodec/ffv1.c
> index f290b94..0d5cfcd 100644
> --- a/libavcodec/ffv1.c
> +++ b/libavcodec/ffv1.c
[...]
> @@ -565,56 +582,81 @@ static void encode_plane(FFV1Context *s, uint8_t *src,
> int w, int h,
> sample[0][-1] = sample[1][0];
> sample[1][w] = sample[1][w - 1];
> // { START_TIMER
> - if (s->avctx->bits_per_raw_sample <= 8) {
> + if (s->bits_per_raw_sample <= 8) {
> for (x = 0; x < w; x++)
> sample[0][x] = src[x + stride * y];
> encode_line(s, w, sample, plane_index, 8);
> } else {
> - for (x = 0; x < w; x++)
> - sample[0][x] = ((uint16_t *)(src + stride * y))[x] >>
> - (16 - s->avctx->bits_per_raw_sample);
> - encode_line(s, w, sample, plane_index,
> s->avctx->bits_per_raw_sample);
> + if (s->packed_at_lsb) {
> + for (x = 0; x < w; x++)
> + sample[0][x] = ((uint16_t *)(src + stride * y))[x];
> + } else {
> + for (x = 0; x < w; x++)
> + sample[0][x] =
> + ((uint16_t *)(src + stride *
> + y))[x] >> (16 -
> s->bits_per_raw_sample);
Does Diego approve that formatting?
Also I'd merge these - the loops differ only by shift parameters, but I don't
care about minicycles obviously.
> + }
> + encode_line(s, w, sample, plane_index, s->bits_per_raw_sample);
> }
> // STOP_TIMER("encode line") }
> }
> }
>
> -static void encode_rgb_frame(FFV1Context *s, uint32_t *src, int w, int h,
> - int stride)
> +static void encode_rgb_frame(FFV1Context *s, uint8_t *src[3], int w, int h,
> + int stride[3])
> {
> int x, y, p, i;
> const int ring_size = s->avctx->context_model ? 3 : 2;
> - int16_t *sample[3][3];
> + int16_t *sample[MAX_PLANES][3];
> + int lbd = s->avctx->bits_per_raw_sample <= 8;
> + int bits = s->avctx->bits_per_raw_sample >
> + 0 ? s->avctx->bits_per_raw_sample : 8;
what would Diego say?
[...]
>
> + v = av_crc(av_crc_get_table(AV_CRC_32_IEEE), 0,
> + f->avctx->extradata,
> + f->avctx->extradata_size);
weird alignment
> + AV_WL32(f->avctx->extradata + f->avctx->extradata_size, v);
> + f->avctx->extradata_size += 4;
> +
> return 0;
> }
>
[...]
> @@ -896,21 +971,133 @@ static int sort_stt(FFV1Context *s, uint8_t stt[256])
> return print;
> }
>
> -static av_cold int encode_init(AVCodecContext *avctx)
> +static av_cold int ffv1_encode_init(AVCodecContext *avctx)
> {
> FFV1Context *s = avctx->priv_data;
> + const AVPixFmtDescriptor *desc = av_pix_fmt_desc_get(avctx->pix_fmt);
> int i, j, k, m;
>
> common_init(avctx);
>
> s->version = 0;
> - s->ac = avctx->coder_type ? 2 : 0;
> +
> + if ((avctx->flags & (CODEC_FLAG_PASS1 | CODEC_FLAG_PASS2)) ||
> + avctx->slices > 1)
> + s->version = FFMAX(s->version, 2);
> +
> + if (avctx->level == 3) {
> + s->version = 3;
> + }
> +
> + if (s->ec < 0) {
> + s->ec = (s->version >= 3);
> + }
> +
> + if (s->version >= 2 &&
> + avctx->strict_std_compliance > FF_COMPLIANCE_EXPERIMENTAL) {
> + av_log(avctx, AV_LOG_ERROR,
> + "Version %d requested, please set -strict experimental in "
> + "order to enable it\n",
> + s->version);
> + return AVERROR(ENOSYS);
> + }
> +
> + s->ac = avctx->coder_type > 0 ? 2 : 0;
> +
> + s->plane_count = 3;
> + switch (avctx->pix_fmt) {
> + case AV_PIX_FMT_YUV444P9:
> + case AV_PIX_FMT_YUV422P9:
> + case AV_PIX_FMT_YUV420P9:
> + if (!avctx->bits_per_raw_sample)
> + s->bits_per_raw_sample = 9;
> + case AV_PIX_FMT_YUV444P10:
> + case AV_PIX_FMT_YUV420P10:
> + case AV_PIX_FMT_YUV422P10:
> + s->packed_at_lsb = 1;
> + if (!avctx->bits_per_raw_sample && !s->bits_per_raw_sample)
> + s->bits_per_raw_sample = 10;
> + case AV_PIX_FMT_GRAY16:
> + case AV_PIX_FMT_YUV444P16:
> + case AV_PIX_FMT_YUV422P16:
> + case AV_PIX_FMT_YUV420P16:
> + if (!avctx->bits_per_raw_sample && !s->bits_per_raw_sample) {
> + s->bits_per_raw_sample = 16;
> + } else if (!s->bits_per_raw_sample) {
> + s->bits_per_raw_sample = avctx->bits_per_raw_sample;
> + }
> + if (s->bits_per_raw_sample <= 8) {
> + av_log(avctx, AV_LOG_ERROR, "bits_per_raw_sample invalid\n");
> + return AVERROR_INVALIDDATA;
> + }
> + if (!s->ac && avctx->coder_type == -1) {
> + av_log(avctx, AV_LOG_INFO,
> + "bits_per_raw_sample > 8, forcing coder 1\n");
> + s->ac = 2;
> + }
> + if (!s->ac) {
> + av_log(
> + avctx, AV_LOG_ERROR,
> + "bits_per_raw_sample of more than 8 needs -coder 1
> currently\n");
> + return AVERROR_INVALIDDATA;
> + }
> + s->version = FFMAX(s->version, 1);
> + case AV_PIX_FMT_GRAY8:
> + case AV_PIX_FMT_YUV444P:
> + case AV_PIX_FMT_YUV440P:
> + case AV_PIX_FMT_YUV422P:
> + case AV_PIX_FMT_YUV420P:
> + case AV_PIX_FMT_YUV411P:
> + case AV_PIX_FMT_YUV410P:
> + s->chroma_planes = desc->nb_components < 3 ? 0 : 1;
> + s->colorspace = 0;
> + break;
> + case AV_PIX_FMT_YUVA444P:
> + case AV_PIX_FMT_YUVA422P:
> + case AV_PIX_FMT_YUVA420P:
> + s->chroma_planes = 1;
> + s->colorspace = 0;
> + s->transparency = 1;
> + break;
> + case AV_PIX_FMT_RGB32:
> + s->colorspace = 1;
> + s->transparency = 1;
> + break;
> + case AV_PIX_FMT_GBRP9:
> + if (!avctx->bits_per_raw_sample)
> + s->bits_per_raw_sample = 9;
> + case AV_PIX_FMT_GBRP10:
> + if (!avctx->bits_per_raw_sample && !s->bits_per_raw_sample)
> + s->bits_per_raw_sample = 10;
> + case AV_PIX_FMT_GBRP16:
> + if (!avctx->bits_per_raw_sample && !s->bits_per_raw_sample)
> + s->bits_per_raw_sample = 16;
> + else if (!s->bits_per_raw_sample)
> + s->bits_per_raw_sample = avctx->bits_per_raw_sample;
> + s->colorspace = 1;
> + s->chroma_planes = 1;
> + s->version = FFMAX(s->version, 1);
> + break;
> + default:
> + av_log(avctx, AV_LOG_ERROR, "format not supported\n");
> + return AVERROR_INVALIDDATA;
> + }
> + if (s->transparency) {
> + av_log(
> + avctx, AV_LOG_WARNING,
> + "Storing alpha plane, this will require a recent FFV1 decoder to
> playback!\n");
> + }
> + if (avctx->context_model > 1U) {
> + av_log(avctx, AV_LOG_ERROR,
> + "Invalid context model %d, valid values are 0 and 1\n",
> + avctx->context_model);
> + return AVERROR(EINVAL);
> + }
This colourspace selection looks unnecessarily overcomplicated.
> if (s->ac > 1)
> for (i = 1; i < 256; i++)
> s->state_transition[i] = ver2_state[i];
>
> - s->plane_count = 2;
> for (i = 0; i < 256; i++) {
> s->quant_table_count = 2;
> if (avctx->bits_per_raw_sample <= 8) {
[...]
> @@ -1421,86 +1638,210 @@ static void decode_plane(FFV1Context *s, uint8_t
> *src,
> } else {
> decode_line(s, w, sample, plane_index,
> s->avctx->bits_per_raw_sample);
> - for (x = 0; x < w; x++)
> - ((uint16_t *)(src + stride * y))[x] =
> - sample[1][x] << (16 - s->avctx->bits_per_raw_sample);
> + if (s->packed_at_lsb) {
> + for (x = 0; x < w; x++)
> + ((uint16_t *)(src + stride * y))[x] = sample[1][x];
> + } else {
> + for (x = 0; x < w; x++)
> + ((uint16_t *)(src + stride *
> + y))[x] = sample[1][x] <<
> + (16 -
> s->avctx->bits_per_raw_sample);
> + }
see above
[...]
> + } else if (f->avctx->bits_per_raw_sample <= 8 && !f->transparency) {
> switch (16 * f->chroma_h_shift + f->chroma_v_shift) {
> - case 0x00:
> - f->avctx->pix_fmt = AV_PIX_FMT_YUV444P;
> + case 0x00: f->avctx->pix_fmt = AV_PIX_FMT_YUV444P;
> + break;
> + case 0x01: f->avctx->pix_fmt = AV_PIX_FMT_YUV440P;
> break;
> - case 0x10:
> - f->avctx->pix_fmt = AV_PIX_FMT_YUV422P;
> + case 0x10: f->avctx->pix_fmt = AV_PIX_FMT_YUV422P;
> break;
> - case 0x11:
> - f->avctx->pix_fmt = AV_PIX_FMT_YUV420P;
> + case 0x11: f->avctx->pix_fmt = AV_PIX_FMT_YUV420P;
> break;
> - case 0x20:
> - f->avctx->pix_fmt = AV_PIX_FMT_YUV411P;
> + case 0x20: f->avctx->pix_fmt = AV_PIX_FMT_YUV411P;
> break;
> - case 0x22:
> - f->avctx->pix_fmt = AV_PIX_FMT_YUV410P;
> + case 0x22: f->avctx->pix_fmt = AV_PIX_FMT_YUV410P;
> break;
either leave it as as is or do it in one line (including break)
[...]
in general it doesn't look that bad
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel
