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

[Vittorio Giovara <vittorio.giovara@gmail.com>]

On Fri, Jan 19, 2024 at 5:56 PM Connor Worley <connorbworley@gmail.com>
wrote:

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

here is fine


> 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.
>

disregard, I assumed it was in alphabetical order


> > What does the HT stand for?
>
>
> Hash table --  this change implements a simple linear probing approach.
>

got it, would be nice to have a small comment on why it's needed, as
documentation


> >> +#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.
>

very cool
-- 
Vittorio
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".