Histogram issues
Reel-Deal opened this issue · 24 comments
Hi there,
I noticed 2 issues with Histogram("color2") - the first image is from avs 2.6, the second is avs+. The top and bottom lines of the square are missing. Not sure when it happened but I believe it used to work correctly.
Also looking at it closely, the markers are not uniform. In avs 2.6 you can see that most of the dots are on the inside edge (a few in the inside the red line), while in avs+ the dots at the bottom are on the outside and the dots on the right are inside the blue line.
2nd issue is that there is no error checking for planar RGB input (only packed) so using it with a planar rgb clip results in:
Edit: similar result when using planar rgb with Histogram("color").
Fixed these two bugs and another one (crash) I have found in "color". See readme change log.
test6:
https://drive.google.com/uc?export=download&id=1cl-NMsjadi2RRdNOAecNbz8ZrP20n2GL
test6
Thanks! That fixed the mentioned issues.
Moving onto the levels mode. First issue I noticed is that it crashes when factor is set to 0.
Blankclip(pixel_type="Y8").Histogram("levels", 0.0)
Avisynth read error:
CAVIStreamSynth: System exception - Access Violation at 0x000007FECB030816
Is there a good reason why factor is an unnamed parameter? It seems strange that the preceding parameter is named. I think is the only filter in avs that is like that, all of the others that have unnamed parameters always come before the named parameters. Naming it would not break anything and makes the usage more consistent. Unless I'm missing something...
2nd issue, the markers parameter. The name and docs suggest that when it is set to false, the markers on the histogram won't show up. But more goes on than that:
Is that the intended behavior when markers=false ? I had never tried this option before but just reading the name I automatically assumed that the dashed midpoint line and the limited range markers for yuv is what wouldn't show up when set to false. I did not expect the color legend to disappear.
3rd issue, the histogram is off by one pixel in regards to the position of the color legend (you can actually see it clearly in the V histogram of the previous pic):
In subsampled formats is not as clear to see as in yuv444 or rgb since the colors are shared. I checked avs26 and it's the same result.
Because I was curious to see whether the CAVIStreamSynth Access Violation would only be true on Windows (since AFAIK, I believe that denotation is in main.cpp, where it interacts with VfW). Turns out, no, it's not: the same script causes a segfault on Linux.
This is the backtrace:
(gdb) r -i test.avs
Starting program: /usr/bin/ffmpeg -i test.avs
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1".
ffmpeg version N-105655-ge4ad38d0f7-6ubuntu5 Copyright (c) 2000-2022 the FFmpeg developers
built with gcc 11 (Ubuntu 11.2.0-7ubuntu2)
configuration: --prefix=/usr --extra-version=6ubuntu5 --toolchain=hardened --libdir=/usr/lib/x86_64-linux-gnu --incdir=/usr/include/x86_64-linux-gnu --arch=amd64 --enable-gpl --enable-debug --disable-stripping --enable-version3 --enable-mbedtls --enable-ladspa --enable-libaom --enable-libass --enable-libbluray --enable-libbs2b --enable-libcaca --enable-libcdio --enable-libcodec2 --enable-libdav1d --enable-libflite --enable-libfontconfig --enable-libfreetype --enable-libfribidi --enable-libgme --enable-libgsm --enable-libjack --enable-libmp3lame --enable-libmysofa --enable-libopenjpeg --enable-libopenmpt --enable-libopus --enable-libplacebo --enable-libpulse --enable-librabbitmq --enable-librubberband --enable-libshine --enable-libsnappy --enable-libsoxr --enable-libspeex --enable-libsrt --enable-libssh --enable-libtheora --enable-libtwolame --enable-libvidstab --enable-libvorbis --enable-libvpx --enable-libwebp --enable-libx265 --enable-libxml2 --enable-libxvid --enable-libzimg --enable-libzvbi --enable-lv2 --enable-omx --enable-openal --enable-opencl --enable-opengl --enable-sdl2 --enable-librsvg --enable-libmfx --enable-libdc1394 --enable-libdrm --enable-libiec61883 --enable-nvenc --enable-chromaprint --enable-frei0r --enable-libx264 --enable-avisynth --enable-vapoursynth --cpu=skylake --extra-cflags='-march=skylake' --extra-ldflags=-pthread --pkg-config-flags=--static
libavutil 57. 21.100 / 57. 21.100
libavcodec 59. 21.100 / 59. 21.100
libavformat 59. 17.102 / 59. 17.102
libavdevice 59. 5.100 / 59. 5.100
libavfilter 8. 27.100 / 8. 27.100
libswscale 6. 5.100 / 6. 5.100
libswresample 4. 4.100 / 4. 4.100
libpostproc 56. 4.100 / 56. 4.100
[New Thread 0x7fffeaf53640 (LWP 24154)]
[New Thread 0x7fffea752640 (LWP 24155)]
[New Thread 0x7fffe9f51640 (LWP 24156)]
[New Thread 0x7fffe9750640 (LWP 24157)]
[New Thread 0x7fffe8f4f640 (LWP 24158)]
[New Thread 0x7fffd3fff640 (LWP 24159)]
Thread 1 "ffmpeg" received signal SIGSEGV, Segmentation fault.
0x00007fffeb523ba8 in Histogram::DrawModeLevels (this=0x5555592f0b50, n=0, env=0x5555592dee10) at ../avs_core/filters/histogram.cpp:1879
1879 if (pixelsize == 1) ptr[x] = color_i;
(gdb) bt
#0 0x00007fffeb523ba8 in Histogram::DrawModeLevels(int, IScriptEnvironment*)
(this=0x5555592f0b50, n=0, env=0x5555592dee10)
at ../avs_core/filters/histogram.cpp:1879
#1 0x00007fffeb51b994 in Histogram::GetFrame(int, IScriptEnvironment*)
(this=0x5555592f0b50, n=0, env=0x5555592dee10)
at ../avs_core/filters/histogram.cpp:238
#2 0x00007fffeb34f0e2 in Cache::GetFrame(int, IScriptEnvironment*)
(this=0x5555592f1e30, n=0, env_=0x5555592dee10)
at ../avs_core/core/cache.cpp:235
#3 0x00007fffeb35003a in CacheGuard::GetFrame(int, IScriptEnvironment*)
(this=0x5555592f0cc0, n=0, env=0x5555592dee10)
at ../avs_core/core/cache.cpp:522
#4 0x00007fffeb34c785 in avs_get_frame(AVS_Clip*, int) (p=0x5555592f1d30, n=0)
at ../avs_core/core/avisynth_c.cpp:832
#5 0x000055555575a141 in ()
#6 0x0000555555b0bc31 in ()
#7 0x0000555555803162 in ()
#8 0x0000555555806fb8 in ()
#9 0x00005555557f9e30 in main ()
(gdb)
Histogram("levels", 0.0)
That second 0.0 parameter is weird, it is a nameless named parameter: it has name but when it is provided in the list of parameters, it can be of type 'anything'.
Instead of [options]. there is []. in the function signature.
I don't know that trick in language construction needed this one, or just the fact that this parameter would hold differently named options.
Presently only "levels" is using it and it means a floating point parameter 'factor'.
Probably it was planned to be able to hold other types as well, string or even a comma delimited array or any other option used for other Histogram modes.
We can easily give a name e.g. "option" for that parameter, it does no harm.
The other bugs are probably straighforward to resolve, I'm gonna build then a test again, unless other small issues emerge meanwhile.
I think for consistency's sake it would be great if it could be used like Histogram(mode="levels", factor=.5), if possible. And yes I wont break any old scripts since it forced nameless parameters when using factor. I went back are read the old changelog and nothing is mentioned in that regard other than a factor option was added.
Correct me if I'm wrong but aren't all of the pixels in the histogram supposed to be white? Just the outer pixels have a mixture of black/gray.
Yep, when factor is 0, then an internal scaling results in infinity after a non-catched div-by-zero.
float scale = float(64.0 / maxval);
And this floating point infinity in then converted back to -max_int and drawing a vertical line into the -2147483583th Y position results in access violation of course.
Top color is really different, and is a greyscale shade of the fractional height of the bar, e.g. it visually distincts a height of 63.1, (top is almost black), 63.5 (middle grey top), and 63.99 (white)
EDIT: it would be better to have shades of the real background color instead of white-to-black: e.g. shades of green in your example above.
EDIT: it would be better to have shades of the real background color instead of white-to-black: e.g. shades of green in your example above.
I agree, the black pixels are really obvious in RGB (mainly the red and green channel) and does not look too good.
I remember a long time ago trying to make my own custom RGB histogram and used a binary mask and it would loose those black pixels.
Edit:
Histogram("StereoOverlay") also does not check for Planar RGB, only packed (it does check for bitdepth). It results in some interesting but wrong colors. Stereo and StereoY8 allow any 8bit format but it always return YV12 or Y8 respectively (nothing wrong here).
preliminary info (no build yet),
What's new is the second one: for lowish 'factor' values the U and V was differently scaled for 4:2:0 and 4:4:4
- Fix: Histogram "Levels": prevent crash when factor=0
- Fix: Histogram "Levels": fix incorrect "factor" applied for U/V part drawing when format was subsampled (non-444)
- Histogram "Audiolevels" and StereoOverlay to deny planar RGB
- Histogram "Luma": support 10-16 and 32 bits
- Histogram: give parameter name "factor" and type 'float' for Histogram's unnamed optional parameter used in "Level" mode.
Other modes just ignore this parameter if given.
Very nice!
I was thinking a bit more about the markers parameter. I'm sure you probably have something in mind but maybe having separate 'markers' and 'background' parameters makes more sense. This allows to keep either or ... just a thought.
As it turned out the top of histogram spikes are shaded proportionally with the fractional height of the bars, from black to white.
Total height is always at most 64 pixels. This extra color legend of the top pixel may indicate something but I'm not convinced it is helping anything. Deep grey = +0.1 in height, grey = +0.5 in height, whitish top can mean +0.9.
This visual enhancement works nice when the background is black (limited part of the Y graph).
But even then it makes more question than help when interpreting it. Black top on colored background is not nice, even seems to be a visual garbage. O.K, let's replace black with R, G or B. But the background is uneven, it is a color gradient.
If I'd make it not using black to white shades but we used color shade of the actual background, it has no sense, since U,V,R,G and B are already a gradient shaded from left to right. So we would shade into a shaded endpoint just to have an antialiase-like effect on the graph. I'd better remove this coloring effect or just leave it for Y channel.
To be honest with you I've never seen scattered black pixels on histograms in other software. That is why I asked, usually they a solid color or some have a color edge at the top, but never what it seems like random black/gray pixels. And I don't even have the slightest clue of what it can possibly mean. The easiest thing would be to make the entire histogram white :). FFmpeg's histogram is all black and white and they use a gradient for the color at the bottom, kinda like a divider between planes. Resolve is one that I've seen that uses colored edges and the rest of the histogram is a lighter color.
Experimenting with something else I found an oddity with Histogram("classic"). The docs say that the mid-point line represents Y=128 but according to the script below, that is not the case. To make the lines line up colors needs to be set to 125, which seems like an odd number considering that 128 is exactly half of 256 (the default width).
Blankclip(height=16, width=256, pixel_type="Y8", colors=[128])
Histogram(keepsource=false)
Actually Y's midpoint is not 128 indeed.
For U and V yes: 128 is the middle of 16-240. But Y range goes from 16 to 235 so the middle is 125.5
I see, but since the graph represents the full [0,255] scale it seems a bit counter intuitive. Thanks for the info, I will add a note about that to the docs. I guess I will have to make my own custom midpoint line for my rgb parade script I'm working on :).
Middle point for usual Y makes not much sense, it just attracts the eyes, makes the interpretation a bit easier. For me it's all the same if it's really 50% of the limited luma range or not.
U and V really needs it because it represents the real 0-point of chroma.
Yeah I understand about chroma midpoint, it's the first time I hear 125 for Y. I made my own "classic" mode with the 128 midpoint line and a gain option that can be quite helpful at making the waveform more visible:
Qiuck snapshot - 3.7.2 test7, x64 only
https://drive.google.com/uc?export=download&id=1K8nW5dBHwILh4TNbfui0XJL6a_xwzikU
No midpoint correction there - could we put it back to 128?
Thanks for the updated build.
No midpoint correction there - could we put it back to 128?
I'm not sure if I understand the question. But on the same topic I noticed that "levels" midpoint line is at 128 for luma (I'm guessing because of chroma it was easier to draw one straight line than having luma be 125 and chroma 128).
Yeah I understand about chroma midpoint, it's the first time I hear 125 for Y. I made my own "classic" mode with the 128 midpoint line and a gain option that can be quite helpful at making the waveform more visible:
I guess it depends on what you want to show, 128 is range half, it coincides with (Y plane) PC range middle-grey (127.5 actually), while TV range middle-grey is 125.5.
I was making new pictures for the Histogram docs and noticed that on the wiki it says that audio for Audiolevels is converted to 16-bit. Which is true, I see that it does that for both Audiolevels and the Stereo* filters (line 212 & 224). But it seems that the original audio is returned and the 16-bit intermediate is only used internally. That's true right?
Yes, it's only converted to a temporary audio clip and used internally.
Thanks, I will clarify that.
Closing this issue. Thanks again for the support and patience :)