[Cin] Real fix for 16-bit pngs

Andrew Randrianasulu randrianasulu at gmail.com
Tue Mar 24 19:39:16 CET 2020


В сообщении от Tuesday 24 March 2020 21:27:35 Phyllis Smith написал(а):
> I did try it on my x86 machine and it did not change the result per ffprobe.
> This may be misleading since ffmpeg-4.2 does not seem to even look for le.
> That is the disturbing feature.  The probes and header data do not include
> be.
> This means that all that can happen is that it can create bad data that is
> not
> decodable by most machines.  If it works on your machine, it is an outlier,
> not
> the normal condition.  You should test the data your machine generates with:
> [root at keystone Downloads]# ls -ltr | tail -1
> -rw-r--r--  1 root    root        179184 Mar 24 12:09 sixteen_bits_test2.png
> [root at keystone Downloads]# cd /path/cinelerra-5.1/thirdparty/ffmpeg-4.2
> [root at keystone Downloads]# ./ffprobe /path/sixteen_bits_test2.png
> Input #0, png_pipe, from 'sixteen_bits_test2.png':
>   Duration: N/A, bitrate: N/A
>     Stream #0:0: Video: png, rgba64be(pc), 720x576, ixt25 tbr, 25 tbn, 25
> tbc
> [root at keystone Downloads]# xv seen_bits_test2.png
> [root at keystone Downloads]# gimp seen_bits_test2.png
> I can't tell if the png seen_bits_test2.png is be or le.


ffprobe tells you?

 rgba64be(pc)

> On my machine it is dim and see thru (probably incorrect).
> Is it supposed to be right or wrong?


Yes, I set alpha fade to some non-100% value in Cinelerra before saving  this
specific png.


> 
> This is what I am using:
> What is your os? fc31?
No, Slackware, between long past and -current.

> what is your arch?  x86_64 ?
Only kernel run in 64-bit mode, userspace is in 32-bit mode.


> what version of png is in use? libpng16 ?

Yes, at lest this is what GIMP 2.10.x demand and use.
I have some older shared libraries (libpng14, libpng12)
for older applications.

> Since most decoders have a header that records the format (png_info
> in this case) it would be good to know how the "endianess" is encoded.  I
> do not see any definitions that can support it.  That means it always fails
> unless user supplied transformation data is included in the decoder open.

sorry, I'm also not expert in this. It seem 16-bit (64 bit per pixel)
WRITING is poorly documented :/

man libpng gives some advice on using this function for reading PNGs but not writing?

=====
PNG files store 16-bit pixels in network byte order (big-endian, ie. most significant bits first).  This code changes the storage to the other way (little-endian, i.e.  least
       significant bits first, the way PCs store them):

           if (bit_depth == 16)
              png_set_swap(png_ptr);

=====


But my png viewer in Konqureor (3.5.10) decodes file, and so does gimp.

Make your own PNG (set Cin to 16 bit in png options you/we added) 
and try to view it?

so i far I found few confusing references:
https://libspng.org/comparison/ - see very end...

"with libpng the output format depends on whether you set alpha filler bits, 
it also has weird defaults such as not converting 16-bit PNG’s to host endianness 
unless png_set_swap() is called."

https://lists.osgeo.org/pipermail/gdal-dev/2004-January/001822.html
and follow discussion .... (its short, just few msgs)


http://downloads.xara.com/opensource/doxygen/html/outptpng_8cpp-source.html

00728         // swap bytes of 16 bit files to most significant bit first
00729         png_set_swap(png_ptr);

I think sometimes 16 bits per pixel confused with 16 bits per *color channel* ....

https://sourceforge.net/p/libpng/bugs/279/
apparently user (developer) forgot to call this function on reading 16 bit per channel PNG.....


> 
> gg
> 
> On Tue, Mar 24, 2020 at 11:54 AM Andrew Randrianasulu <
> randrianasulu at gmail.com> wrote:
> 
> > В сообщении от Tuesday 24 March 2020 20:35:55 Phyllis Smith написал(а):
> > > Andrew,
> > > GG writes here:  This is weird.  I have looked into this and Little
> > Endian
> > > / png / swapping in ffmpeg, libpng.
> > > see libpng16-src pnginfo.h, no endian or swap or relevant order params.
> > > see ffmpeg-4.2 libavcodec/pngdec.c:618 decode_idat_chunk, only 16be
> > formats.
> >
> > Well, _on BE machine_, it seems, there will be no need to call this.
> > But I'm on little x86.
> >
> > > Little Endian format is not supported in ffmpg.  It is not in any png.h
> > > header data.  It only seems to be a user specified transform option.   In
> > > other words, you have to tell it not to use the default of big endian
> > > (network byte order).  Since this only generates the possibility of a
> > fail,
> > > I think that the best idea is to remove it as a possibility.   Perhaps
> > you
> > > should investigate why it is the default on the machine you are using.  I
> > > do not see any way it could be normally generated using libpng16 source
> > > (the current version used by all os vendors).
> >
> > I take this code from gimp-2.10 git sources, this works for me.
> > ORDER of calls is important.
> >
> >
> > https://github.com/GNOME/gimp/blob/mainline/plug-ins/common/file-png.c
> >
> > around lines 1823-1828:
> >
> >
> >     if (text)
> >       png_set_text (pp, info, text, 1);
> >
> >
> >     png_write_info (pp, info);
> >     if (G_BYTE_ORDER == G_LITTLE_ENDIAN)
> >       png_set_swap (pp);
> >
> > Try to save 16 bpp PNG with current git, and with this fix. See the
> > difference :/
> >
> >
> >
> > >
> > >
> > > On Tue, Mar 24, 2020 at 6:13 AM Andrew Randrianasulu <
> > > randrianasulu at gmail.com> wrote:
> > >
> > > > Sorry, I apparently send wrong patch - 16 bit pngs were still broken.
> > > >
> > > > I scratched my head a lot (because it was working!)
> > > > and realized I swapped order of calls!
> > > >
> > > > diff relative to commit 8c43913b9f18a3856bbff826aef96587a32f0470
> > > >
> > > > diff --git a/cinelerra-5.1/cinelerra/filepng.C
> > > > b/cinelerra-5.1/cinelerra/filepng.C
> > > > index 4df95f1e..c2e1cf6c 100644
> > > > --- a/cinelerra-5.1/cinelerra/filepng.C
> > > > +++ b/cinelerra-5.1/cinelerra/filepng.C
> > > > @@ -229,9 +229,9 @@ int FilePNG::write_frame(VFrame *frame, VFrame
> > *data,
> > > > FrameWriterUnit *unit)
> > > >                                 asset->png_use_alpha ?
> > > > PNG_COLOR_TYPE_RGB_ALPHA : PNG_COLOR_TYPE_RGB,
> > > >                                 PNG_INTERLACE_NONE,
> > > > PNG_COMPRESSION_TYPE_DEFAULT, PNG_FILTER_TYPE_DEFAULT);
> > > >  // does not work (gg 2020/03/17 libpng16 fc31)
> > > > +                       png_write_info(png_ptr, info_ptr);
> > > >                         if( asset->png_depth == 16 &&
> > > > BC_Resources::little_endian )
> > > >                                 png_set_swap(png_ptr);
> > > > -                       png_write_info(png_ptr, info_ptr);
> > > >                         png_write_image(png_ptr,
> > output_frame->get_rows());
> > > >                         png_write_end(png_ptr, info_ptr);
> > > >                         result = 0;
> > > >
> > > > this way it SHOULD work .... Tested!
> > > >
> > > > Also, hint for ccache:
> > > > in .ccache/ccache.conf set for maximum compression
> > > >
> > > > compression = true
> > > > compression_level = 8
> > > > --
> > > > Cin mailing list
> > > > Cin at lists.cinelerra-gg.org
> > > > https://lists.cinelerra-gg.org/mailman/listinfo/cin
> > > >
> > >
> >
> >
> > --
> > Cin mailing list
> > Cin at lists.cinelerra-gg.org
> > https://lists.cinelerra-gg.org/mailman/listinfo/cin
> >
> 




More information about the Cin mailing list