[FFmpeg-devel] [PATCH] Adding Cinepak encoder (attached)

[Michael Niedermayer]

On Mon, Jan 13, 2014 at 10:19:14AM +0000, u-owvm at aetey.se wrote:
> Hello,
> 
> This is the Cinepac encoder with the tweak allowing windows players to
> handle the resulting files: the number of strips per frame is restricted
> to below 3 which in my limited testing allowed playing by all of
> the following:
> 
> - mplayer on Linux with the binary (windows dll) decoder
> - windows media player on Windows XP
> - windows media player on Windows 98
> - windows media player on Windows 3.1
> 
> Raising the number of strips to 4 makes these programs crash immediately.
> 
> The former max strip number (the actual number is dynamically chosen
> by the encoder) was 32, which allows for better quality, especially on
> higher resolutions, and is accepted by the decoder in ffmpeg.
> 
> Now it is hardcoded to 3.
> 
> The files in my test did not play properly under Quicktime on MacOSX.
> It seems that the description of the format (multimedia.cx wiki) is
> insufficient/inconsistent with the assumptions of the Quicktime decoder.
> 
> The 3 strip limit seems to be present in Quicktime as well, up to 3
> strips produce a distorted image while 4 strips make the player show
> nothing at all.
> 
> Regards,
> Rl
[...]
> +typedef struct {
> +    AVCodecContext *avctx;
> +    unsigned char *pict_bufs[4], *strip_buf, *frame_buf;

> +    AVFrame last_frame;
> +    AVFrame best_frame;
> +    AVFrame scratch_frame;
> +    AVFrame input_frame;

sizeof(AVFrame) is not part of the ABI anymore so these must be changed
to AVFrame *... or something else

[...]
> +enomem:
> +    av_free(s->codebook_input);
> +    av_free(s->codebook_closest);
> +    av_free(s->strip_buf);
> +    av_free(s->frame_buf);
> +    av_free(s->mb);
> +#ifdef CINEPAKENC_DEBUG
> +    av_free(s->best_mb);
> +#endif
> +
> +    for(x = 0; x < (avctx->pix_fmt == AV_PIX_FMT_RGB24 ? 4 : 3); x++)
> +        av_free(s->pict_bufs[x]);

these should all be av_freep() to prevent stale pointers from being
left in the struct and potentially causing double frees


[...]

> +                    rr = 0.2857*rr + 0.5714*gg + 0.1429*bb;
> +                    if(      rr <   0) rr =   0;
> +                    else if (rr > 255) rr = 255;
> +                    scratch_pict.data[0][i1 + i2*scratch_pict.linesize[0]] = rr;
> +                }
> +                r /= 4; g /= 4; b /= 4;
> +// "U"
> +                rr = -0.1429*r - 0.2857*g + 0.4286*b;
> +                if(      rr < -128) rr = -128;
> +                else if (rr >  127) rr =  127;
> +                scratch_pict.data[1][0] = rr + 128; // quantize needs unsigned
> +// "V"
> +                rr = 0.3571*r - 0.2857*g - 0.0714*b;

with float operations there may be slight differences in the output
which would break a fate test

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Avoid a single point of failure, be that a person or equipment.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20140114/6f8b6222/attachment.asc>