[FFmpeg-devel] [PATCH 1/2] avcodec/jpegtables: remove duplicate luma and chroma quantization tables

[Peter Ross]

On Fri, Oct 14, 2022 at 05:19:44PM +0200, Andreas Rheinhardt wrote:
> Peter Ross:
> > Duplicates of the standard JPEG quantization tables were found in the
> > AGM, MSS34(dsp), NUV and VP31 codecs. This patch elimates those duplicates,
> > placing a single copy in jpegtables.
> > ---

> 1. mss34dsp now uses jpegtables, yet it does not have a dependency on
> it. Instead you seem to rely on all the users of mss34dsp to have a
> dependency on jpegtables, yet this is not true for all of them. You will
> get linking failures with --disable-everything --enable-decoder=msa1.

there is a OBJS-$(CONFIG_JPEGTABLES) += line in avcodec/Makefile that
takes care of this. i tested this patch under few different configurations
and observed no build errors.

> 2. The fact that you need to add jpegtables to configure for almost all
> components that use the jpeg quant tables means that it is not really
> appropriate to put the jpeg quant tables into the same files as the jpeg
> huff tables.
> 3. The jpeg huff tables are duplicated into libavformat for shared
> builds (because the overhead of exporting them exceeds the size gains
> from not duplicating them); yet when one uses --enable-shared and
> --enable-static at the same time, it might be that libavformat.a is
> linked to libavcodec.so and therefore has no access to libavcodec's
> internal symbols like the jpegtables, so we have to duplicate the
> jpegtables into libavformat.a in this case. But if one links using
> libavformat.a and libavcodec.a with both containing the jpeg huffman
> tables, then one will get a linker error with this patch: The jpeg
> huffman tables in libavformat will be pulled in by the libavformat
> component needing them. With this patch libavcodec/jpegtables.o will
> also be pulled in. But it also contains the jpeg tables already provided
> by lavf/jpegtables.o, so you will get a multiple definition error.
> In contrast to this, before this patch, lavc/jpegtables.o would not be
> pulled in, because all dependencies to the mjpeg huffman tables will be
> satisfied by lavf/jpegtabes.o.
> So to summarize: If one duplicates stuff in multiple libraries, the
> object files must really provide the exact same symbols; not more, not less.

wasn't aware jpegtabs.h was duplicated into libavformat.

> The last two points imply that these tables should be moved to a file of
> their own. Btw: I don't think that a configure subsystem is appropriate
> for this (a single file with some tables is not really a subsystem);
> actually I don't think that the current jpegtables subsystem is
> appropriate at all.

will do

-- Peter
(A907 E02F A6E5 0CD2 34CD 20D2 6760 79C5 AC40 DD6B)
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 195 bytes
Desc: not available
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20221015/cce4ae03/attachment.sig>