Real fix for 16-bit pngs
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
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. 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). On Tue, Mar 24, 2020 at 6:13 AM Andrew Randrianasulu < [email protected]> 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 [email protected] https://lists.cinelerra-gg.org/mailman/listinfo/cin
В сообщении от 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 < [email protected]> 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 [email protected] https://lists.cinelerra-gg.org/mailman/listinfo/cin
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@keystone Downloads]# ls -ltr | tail -1 -rw-r--r-- 1 root root 179184 Mar 24 12:09 sixteen_bits_test2.png [root@keystone Downloads]# cd /path/cinelerra-5.1/thirdparty/ffmpeg-4.2 [root@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@keystone Downloads]# xv seen_bits_test2.png [root@keystone Downloads]# gimp seen_bits_test2.png I can't tell if the png seen_bits_test2.png is be or le. On my machine it is dim and see thru (probably incorrect). Is it supposed to be right or wrong? This is what I am using: What is your os? fc31? what is your arch? x86_64 ? what version of png is in use? libpng16 ? 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. gg On Tue, Mar 24, 2020 at 11:54 AM Andrew Randrianasulu < [email protected]> 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 < [email protected]> 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 [email protected] https://lists.cinelerra-gg.org/mailman/listinfo/cin
-- Cin mailing list [email protected] https://lists.cinelerra-gg.org/mailman/listinfo/cin
В сообщении от 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@keystone Downloads]# ls -ltr | tail -1 -rw-r--r-- 1 root root 179184 Mar 24 12:09 sixteen_bits_test2.png [root@keystone Downloads]# cd /path/cinelerra-5.1/thirdparty/ffmpeg-4.2 [root@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@keystone Downloads]# xv seen_bits_test2.png [root@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 < [email protected]> 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 < [email protected]> 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 [email protected] https://lists.cinelerra-gg.org/mailman/listinfo/cin
-- Cin mailing list [email protected] https://lists.cinelerra-gg.org/mailman/listinfo/cin
participants (2)
-
Andrew Randrianasulu -
Phyllis Smith