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

[Peter Ross <pross@xvid.org>]

[-- Attachment #1.1: Type: text/plain, Size: 2665 bytes --]

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)

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

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