[FFmpeg-devel] [PATCH 3/7] proresdec2: use VLC for level instead of EC switch

[Christophe Gisquet]

Hello,

Le ven. 8 sept. 2023 à 11:57, Andreas Rheinhardt
<andreas.rheinhardt at outlook.com> a écrit :
> >> +#define CACHED_BITSTREAM_READER 1
> >
> > This should be in the commit switching to the cached bitstream reader.
>
> Correction: This header is included in videotoolbox.c and there is other
> stuff that also includes get_bits.h included in said file (and currently
> gets included before proresdec.h). This means that proresdec2.c and
> videotoolbox.c will have different opinions on what a GetBitContext is:
> It will be the non-cached one in videotoolbox.c and the cached one in
> proresdec2.c. This will work in practice, because ProresContext does not
> need the complete GetBitContext type at all (it does not contain a
> GetBitContext at all), so offsets are not affected. But it is
> nevertheless undefined behaviour and could become dangerous when using LTO.
>
> So you should switch the type of the pointer to BitstreamContextBE* in
> proresdec2.h. Furthermore, you can either include bitstream.h in
> proresdec.h or (IMO better) use a forward declaration and struct
> BitstreamContextBE* in the function pointer without including get_bits.h
> in the header at all.

On that point only: I don't recall (yes that's 3+ years old) the issue
being videotoolbox, it didn't have that include back when I wrote this
code.

It's a very faint recollection, and I don't find proof in the ffmpeg
code today or of 3+ years ago, but the problem you mention was
happening with the encoder instead. So maybe the fix now is needed
only by videotoolbox then.

-- 
Christophe