[FFmpeg-devel] [PATCH] libavcodec/zmbvenc: Add support for RGB formats

[Matthew Fearnley]

On Fri, 8 Mar 2019 at 18:07, Carl Eugen Hoyos <ceffmpeg at gmail.com> wrote:

> 2019-03-08 15:04 GMT+01:00, Tomas Härdin <tjoppen at acc.umu.se>:
> > tor 2019-03-07 klockan 14:42 +0000 skrev Matthew Fearnley:
> >> This consists mostly of the following changes:
> >> - add newly supported pixel formats (RGB555LE, RGB565LE, BGR0)
> >> - select the ZMBV format (c->fmt) and bytes per pixel (c->bypp) based on
> >>   avctx->pix_fmt
> >> - multiply widths/x-values by c->bypp, in places where bytes, not
> pixels,
> >> are
> >>   expected
> >> - disable palette-writing code for non-palette pix_fmts
> >> - make a note about histogram[]'s datatype (it could need increasing if
> >>   ZMBV_BLOCK is increased)
> >> - adjust the c->score_tab length to take up to (and including) 4 times
> the
> >>   number of pixels in a block
> >> - initialise c->score_tab up to c->bypp * the number of pixels
> >>
> >> Note: the ZmbvFormat enum allows for additional bit depths:
> >> - 1,2,4-bit (palette)
> >> - 24-bit (RGB)
> >>
> >> At time of writing the specifics of these (e.g. channel order, bit
> >> alignment)
> >> are not currently defined, and DOSBox only implements support for
> >> 8/15/16/32
> >> bpp.
> >> One might expect the 24-bit format - if implemented - to be BGR24, to
> have
> >> the
> >> same channel order as BGR0.
> >> However, the decoder in zmbv.c has been guessed to use RGB24, so I have
> >> chosen
> >> to not contradict this, and omitted specific support for this format.
> >
> > Sounds good.
>
> Yes.
>
> > Maybe we could coordinate 1/2/4/24-bit support with the
>
> I believe FFmpeg cannot support 1/2/4 bit for encoding.
>
As far as I can see, FFmpeg has very limited support for bit depths less
than 8.  I think there are basically two formats (plus variants), with
fixed "palettes":
1bpp: MONO_BLACK / MONO_WHITE (black/white only)
4bpp: RGB4[_BYTE], BGR4[_BYTE] (RGB 1:2:1, 16 colours - I presume red/blue
would be 0,255; green would be 0,85,170,255)

If the ZMBV formats were defined, these might be worth encoder adding
support for.
(Practically speaking though, it would be a slight pain, because the
encoder would do the work in 8bpp and pack/unpack as needed.)
But with PAL8 being the only format allowing a free palette, all sub-8bpp
formats would have to decode to that, so they wouldn't round-trip.

(It should be possible to implement decoding to pal8 if
> that doesn't work yet and if samples exist.)
>
No samples or specifications exist that I know of, so I don't plan to
submit any patches to the decoder unless/until there is something to work
with there.

> dosbox devs? And maybe we should do something about
> > the RGB24 thing in the decoder..
>
Yeah, I think talking with the DOSBox devs sounds like a potentially good
idea.

>
> Do I understand correctly that no existing implementation
> supports 24bit rgb? If that is correct, I believe FFmpeg
> shouldn't add it (but this may only be me).
>
I agree that FFmpeg shouldn't add support for any formats that haven't been
defined.
As far as I know, the specifics of the 24-bit RGB format havn't been
discussed anywhere, and there are no samples I know of.

A likely specification of 24-bit is trivial enough to add support for, that
I was originally planning to add it with an #ifdef (like in the decoder).
But it wouldn't do to have contradictory channel orders proposed in the
decoder and encoder, so I will leave that for now unless DOSBox will commit
to one.

I presume that FFmpeg generally doesn't like to set standards in media
formats, only to implement existing ones.
My personal feelings in this case would be to provide support that's
disabled at compile-time if an official specification can be agreed, and to
have support included by default if an independent implementation - or at
least independent samples - are available that agree with the specification.
,
Matthew