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

[Matthew Fearnley]

> On 11 Mar 2019, at 10:37, Tomas Härdin <tjoppen at acc.umu.se> wrote:
> 
> fre 2019-03-08 klockan 21:39 +0000 skrev 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>:
>>>> 
>>>> 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.
> 
> There's some justification for adding sub-8bpp, like BMP. bmp.c
> converts all of them except GRAY8 to PAL8. Bitdepths besides 1, 4 and 8
> don't work at all.
> 
> One way to at least allow both the bmp and zmbv encoders to do sub-8bpp 
> from PAL8 would be to keep track of the maximum number of colors in
> some appropriate struct.
> 
> Adding proper sub-8bpp support would involve a lot of libsws headache I
> suspect.
It occurs to me that adding sub-8bpp has some implications:

My current understanding (I could be wrong) is that FFmpeg tends to detect the pix_fmt based on the first frame. If FFmpeg detects the first frame as e.g. PAL4, and chooses that as its output, that means the rest of the video will have to be encodable as PAL4, otherwise it (obviously) won’t be encoded properly.

So adding a PAL4 format puts a new constraint on encoders (inside and outside FFmpeg) to not encode frames in a way that looks like PAL4, unless the whole video will be encodable that way.

If FFmpeg supports PAL8 only, then it can be tempting to optimise videos to encode as sub-8bpp whenever possible, knowing they will always (in FFmpeg at least) decode to PAL8. But this could break format detection for tools outside FFmpeg, if they choose to add sub-8bpp support.

The safest thing FFmpeg can do is to always decode sub-8bpp to PAL8, and to emit PAL8 frames as exactly 8bpp (where applicable). It could still offer encoding formats for PAL1/2/4, but these formats could only be detected by scanning the whole video.

The suggestion of bits_per_raw_sample sounds interesting. What would that look like in practice?

>> (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.
> 
> Just a small thing to be clear: ZMBV_ENABLE_24BPP is not defined
> anywhere, so we're free to do however we want with it. It's not going
> to break anyone's workflow unless they were foolish enough to encode
> 24-bit ZMBVs outside of the non-existing spec.
True. But they might be understandably puzzled if they encode as 24-bit, and then find the channels swapped when they decode.

Thanks for writing the email to the DOSBox crew.
If they choose a channel order, then we have good grounds for fixing the encoder (if need be), and implementing the decoder in the same way.
It occurs to me that they might (in theory) also want to specify 2/4 byte alignment on RGB, like with the MVs.  My gut says there’d be very little benefit though, and it would only be seen with strange video / block widths.

It also occurs to me this will may warrant a version bump in the format, to give an easy error case for decoders that don’t expect it. Particularly if our decoder has to redefine its channel order.

Matthew