[FFmpeg-devel] a64 encoder 7th round
Bitbreaker/METALVOTZE
bitbreaker
Mon Jan 26 08:33:05 CET 2009
> nitpick:
> int height = FFMIN(avctx->height,C64YRES);
> int width = FFMIN(avctx->width ,C64XRES);
Fixed.
>> + 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
Fixed.
>> + 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)
I might do so. It is more or less a remain of testing and i don't think
the order will change anymore. It just was very handy in times i was not
sure about the final order of the colors.
>> + 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[]
I guess i wanted to be extra secure, (coz i am using that values for
building an index later on), nothing else :-)
> index2= FFMIN(index1 + 1, maxindex);
[...]
> FFMAX/FFMIN can simplify these
Made more use of FFMAX/FFMIN.
> as this seems to never be anything else it could as well be a #define
> simpifying the code ...
Did a define for that. I kept this so far, as i thought of maybe having
an option for that once, and enable the use to choose between different
ditherpatterns/depths. But can be inplemented easily again if needed.
> 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 ...
I was too eager and accidently also added it to the encode function.
However, i am also not sure what this does, i simply did how others did :-)
>> + 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.
The difference is not really big. The normal multicol mode works with a
fixed color in the colram, thus having a range of 4 colors (3 multicol
registers + colorram). That covers a luminance range from black up to
light gray.
The 5col mode extends the dynamic range, by displaying the brightest and
the darkest color with the color defined by the colorram. That means i
need to add the colorram for each frame and therefore i am able to
display a range from black to white (however black and white may not
occur at once in a 8x8 block, else i scale either white down or black up
if that happens).
I have chosen the way of two codecs, as i lacked an appropriate option
to set it. As for the charset lifetime i now use qscale, maybe there is
also a commandline option that suites that behaviour well? Then i can of
course just use a single codec ID.
>> +/* dither patterns */
>> +static const uint8_t prep_dither_patterns[9][4][4] = {
>
> comment is not doxygen compatible
Fixed.
Kindest regards,
Toby
--
style)KURVE - Fotografie mit Biss.
www.style-kurve.de - info at style-kurve.de
Tobias Bindhammer, Uhlandstrasse 8, 89278 Stra?
01577/1751761
-------------- next part --------------
A non-text attachment was scrubbed...
Name: a64_7.diff
Type: text/x-diff
Size: 14808 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090126/d7929589/attachment.diff>
More information about the ffmpeg-devel
mailing list