[FFmpeg-devel] a64 encoder 6th round
Michael Niedermayer
michaelni
Sun Jan 25 17:38:54 CET 2009
On Sat, Jan 24, 2009 at 09:53:40PM +0100, Bitbreaker/METALVOTZE wrote:
> Fixed all alignment issues found by Diego, updates Changelog and
> documentation.
>> these could be set to FFMIN(width, C64XRES) ... i think
>>
>>
> done so.
>> *dest++ = ...
>>
>>
> I did forgo on that, as there was a bug anyway when scaling video below
> C64XRES. Fixed that.
>>
>>> + row1 <<= 2;
>>> + if (prep_dither_patterns[dither][y & 3][x & 3])
>>> + row1 |= vals[index2];
>>> + else
>>> + row1 |= vals[index1];
>>> + }
>>> + charset[y] = row1;
>>>
>>
>> could be simplified slightly with put_bits()
>>
> Unfortunatedly this didn't work out and trashes the charset when i do so. I
> guess because i redo certain chars in 5col mode. I could of course split
> things up into another loop for calculating the error first and adjusting
> pixels and a loop for rendering a charset line. But maybe that bloats up
> things more than it helps?
hmm forget put_bits then
[...]
> +/* own methods */
> +static void to_meta_with_crop(AVCodecContext *avctx, AVFrame *p, int *dest)
> +{
> + int blockx, blocky, x, y;
> + int luma = 0;
> + int height = FFMIN(avctx->height,C64YRES);
> + int width = FFMIN(avctx->width,C64XRES);
nitpick:
int height = FFMIN(avctx->height,C64YRES);
int width = FFMIN(avctx->width ,C64XRES);
> + uint8_t *src = p->data[0];
> +
> + for (blocky = 0; blocky<height; blocky += 8) {
> + for (blockx = 0; blockx<C64XRES; blockx += 8) {
> + for (y = blocky; y < blocky+8 && y<height; y++) {
> + for (x = blockx; x < blockx+8 && x<C64XRES; x += 2) {
> + if(x<width) {
> + /* build average over 2 pixels */
> + luma = (src[(x + 0 + y * p->linesize[0])] +
> + src[(x + 1 + y * p->linesize[0])]) / 2;
> + /* write blocks as linear data now so they are suitable for elbg */
> + dest[0] = luma;
the indention depth looks odd
> + }
> + dest++;
> + }
> + }
> + }
> + }
> +}
> +
> +static void render_charset(AVCodecContext *avctx, uint8_t *charset, uint8_t *colrammap)
> +{
> + A64Context *c = avctx->priv_data;
> + uint8_t row1;
> + int charpos, x, y;
> + int pix;
> + int dither;
> + int index1, index2;
> + int lowdiff, highdiff;
> + int dsteps = c->mc_dithersteps;
> + int maxindex = c->mc_use_5col + 3;
> + int maxsteps = c->mc_dithersteps * maxindex + 1;
> + int *best_cb = c->mc_best_cb;
> + static const uint8_t vals[] = { 3, 2, 1, 0, 3 };
could be replaced by (3-x)&3
(no iam not suggesting that it should be replaced, iam just mentioning it
could)
> +
> + /* now reduce colors first */
> + for (x = 0; x < 256 * 32; x++) {
> + pix = best_cb[x] * maxsteps / 255;
> + if (pix > maxsteps)
> + pix = maxsteps;
does this if() do anything at all?
if not, the rounding might also not be ideal, i mean
0..(255/maxsteps) will be 0
but only
255 will be maxsteps
that is, if 255 is the largest best_cb[]
> + best_cb[x] = pix;
> + }
> + /* and render charset */
> + for (charpos = 0; charpos < 256; charpos++) {
> + lowdiff = 0;
> + highdiff = 0;
> + for (y = 0; y < 8; y++) {
> + row1 = 0;
> + for (x = 0; x < 4; x++) {
> + pix = best_cb[y * 4 + x];
> + dither = pix % dsteps;
> + index1 = pix / dsteps;
> + if (index1 == maxindex)
> + index2 = maxindex;
> + else
> + index2 = index1 + 1;
index2= FFMIN(index1 + 1, maxindex);
> +
> + if (pix > 3 * dsteps)
> + highdiff += pix - 3 * dsteps;
> + if (pix < dsteps)
> + lowdiff += dsteps - pix;
> +
> + row1 <<= 2;
> + if (prep_dither_patterns[dither][y & 3][x & 3])
> + row1 |= vals[index2];
> + else
> + row1 |= vals[index1];
> + }
> + charset[y] = row1;
> + }
> +
> + /* are we in 5col mode and need to adjust pixels? */
> + if (c->mc_use_5col && highdiff > 0 && lowdiff > 0) {
> + if (lowdiff > highdiff) {
> + for (x = 0; x < 32; x++) {
> + if (best_cb[x] > 3 * dsteps)
> + best_cb[x] = 3 * dsteps;
> + }
> + } else {
> + for (x = 0; x < 32; x++) {
> + if (best_cb[x] < dsteps)
> + best_cb[x] = dsteps;
> + }
FFMAX/FFMIN can simplify these
[...]
> +static av_cold int a64multi_init_encoder(AVCodecContext *avctx)
> +{
> + A64Context *c = avctx->priv_data;
> + av_init_random(1, &c->randctx);
> +
> + if (avctx->global_quality < 1)
> + c->mc_lifetime = 4;
> + else
> + c->mc_lifetime = avctx->global_quality /= FF_QP2LAMBDA;
> +
> + av_log(avctx, AV_LOG_INFO, "charset lifetime set to %d frame(s)\n", c->mc_lifetime);
> +
> + c->mc_frame_counter = 0;
> + c->mc_dithersteps = 8;
as this seems to never be anything else it could as well be a #define
simpifying the code ...
> + c->mc_use_5col = avctx->codec->id == CODEC_ID_A64_MULTI5;
> +
> + c->mc_meta_charset = av_malloc(32000 * c->mc_lifetime * sizeof(int));
> + c->mc_best_cb = av_malloc(256 * 32 * sizeof(int));
> + c->mc_charmap = av_malloc(1000 * c->mc_lifetime * sizeof(int));
> +
> + avcodec_get_frame_defaults(&c->picture);
> + avctx->coded_frame = &c->picture;
> + avctx->coded_frame->pict_type = FF_I_TYPE;
> + avctx->coded_frame->key_frame = 1;
> + if (!avctx->codec_tag)
> + avctx->codec_tag = avcodec_pix_fmt_to_codec_tag(avctx->pix_fmt);
> +
> + return 0;
> +}
> +
> +static av_cold int a64multi_encode_frame(AVCodecContext *avctx, unsigned char *buf, int buf_size, void *data)
i dont think this should be av_cold but then i must admit its not clear what
gcc exactly does with the cold attribute. Fact is other encoders dont mark
their encode_frame cold though ...
[...]
> + if (data == NULL)
> + c->mc_lifetime = c->mc_frame_counter;
if(!data)
[...]
> Index: libavcodec/avcodec.h
> ===================================================================
> --- libavcodec/avcodec.h (Revision 16763)
> +++ libavcodec/avcodec.h (Arbeitskopie)
> @@ -190,6 +190,8 @@
> CODEC_ID_MOTIONPIXELS,
> CODEC_ID_TGV,
> CODEC_ID_TGQ,
> + CODEC_ID_A64_MULTI,
> + CODEC_ID_A64_MULTI5,
What differnce is there between the 2 codecs?
Either way i dont think it justifies 2 coedec ids.
[...]
> +/* dither patterns */
> +static const uint8_t prep_dither_patterns[9][4][4] = {
comment is not doxygen compatible
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
Thouse who are best at talking, realize last or never when they are wrong.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090125/98424ea5/attachment.pgp>
More information about the ffmpeg-devel
mailing list