[FFmpeg-devel] [PATCH] Add DXV encoder with support for DXT1 texture format

[Connor Worley]

Thanks for the feedback! For the next revision, is it preferred to reply to this thread or create a new one?

On 1/19/24 08:23, Vittorio Giovara wrote:
>> diff --git a/libavcodec/allcodecs.c b/libavcodec/allcodecs.c
>> index 93ce8e3224..ef8c3a6d7d 100644
>> --- a/libavcodec/allcodecs.c
>> +++ b/libavcodec/allcodecs.c
>> @@ -106,6 +106,7 @@ extern const FFCodec ff_dvvideo_encoder;
>>  extern const FFCodec ff_dvvideo_decoder;
>>  extern const FFCodec ff_dxa_decoder;
>>  extern const FFCodec ff_dxtory_decoder;
>> +extern const FFCodec ff_dxv_encoder;
>>  extern const FFCodec ff_dxv_decoder;
>>
> nit: keep list in order


Not sure what you mean, the present order seems to be encoder followed by decoder for codecs that have both.

>>  extern const FFCodec ff_eacmv_decoder;
>>  extern const FFCodec ff_eamad_decoder;
>> diff --git a/libavcodec/dxvenc.c b/libavcodec/dxvenc.c
>> new file mode 100644
>> index 0000000000..33080fa1c9
>> --- /dev/null
>> +++ b/libavcodec/dxvenc.c
>> @@ -0,0 +1,358 @@
>> +/*
>> + * Resolume DXV encoder
>> + * Copyright (C) 2015 Vittorio Giovara <vittorio.giovara at gmail.com>
>> + * Copyright (C) 2015 Tom Butterworth <bangnoise at gmail.com>
>> + * Copyright (C) 2018 Paul B Mahol
>> + * Copyright (C) 2024 Connor Worley <connorbworley at gmail.com>
>>
> Idk about tom or paul, but I haven't done anything for this encoder :)
> I think you can prune the list of copyright quite a bit here


Got it. I copied some code verbatim from dxv.c and hapenc.c and erred on the side of overcrediting.


>> +#define LOOKBACK_HT_ELEMS 0x40000
>>
> What does the HT stand for?


Hash table --  this change implements a simple linear probing approach.

>> +#define LOOKBACK_WORDS    0x20202
>> +
>> +enum DXVTextureFormat {
>> +    DXV_FMT_DXT1 = MKBETAG('D', 'X', 'T', '1'),
>> +};
>>
> Why would you go for an enum here? Just for future expansion and the switch
> case below?


Exactly, that's the plan.