[FFmpeg-devel] cleanup mjpeg encoding

[Michael Niedermayer]

Hi

On Mon, May 14, 2007 at 02:11:17AM +0200, Aurelien Jacobs wrote:
> On Mon, 14 May 2007 02:02:26 +0200
> Michael Niedermayer <michaelni at gmx.at> wrote:
> 
> > Hi
> > 
> > On Mon, May 14, 2007 at 02:01:06AM +0200, Aurelien Jacobs wrote:
> > > Hi,
> > > 
> > > The mjpeg_write_tables and mjpeg_data_only_frames vars are always set
> > > to the same value.
> > > Is the attached patch ok ?
> > 
> > no, because it will break LJPEG and JPEG-LS,
> 
> JPEG-LS encoder don't even need mjpeg.c to be compiled so it for sure
> won't be affected. LJPEG call MPV_encode_init() just the same way as
> MJPEG, and that's this function which set the constant value for those
> 2 variables. So LJPEG shouldn't be affected either.

after your patch jpeg_table_header() is called unconditionally, this function
stores quantization tables amongth other things, quantization tables
have nothing to do with LJPEG ...

and a few lines after the call to jpeg_table_header() is a check for LJPEG
so this doesnt look correct at all to me though it doesnt seem like your
patch changed the behavior it was already wrong before thats also why
the regression tests pass ...

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Good people do not need laws to tell them to act responsibly, while bad
people will find a way around the laws. -- Plato
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20070514/17f71d6e/attachment.pgp>