sitespeedio
Rewrite visual metrics
One thing that's been around for a long time is Visual Metrics. It was originally built by Pat Meenan, I moved a version into Browsertime with some custom functionality and Mozilla helped added more functionality.
Number one at my wish list is to rewrite visual metrics, because its a lot of code that I don't know and code that isn't used and we are missing a proper testing of the code. In my dream world it would be separate module, written in NodeJS and preferable less install dependencies than today and structure in multiple files with more readable code. Also having tests and videos where we could add problematic videos over time (I have some problematic videos I can share) so that the library gets better over time.
As @gmierz suggested, this could be a pretty cool student project: it's isolated and cool functionality.
@soulgalore I'm wondering if you've considered any alternatives to this vismet script? I think at one point you mentioned a new method/tool that you wanted to try out but I can't remember the name of it.
There's a private project (that I can share) that Tobias started and I worked on for a while that uses npm modules image-ssim and jimp to get rid of the python dependencies + ImageMagick (right now we are dependent on an old ImageMagick version). But that version still use ffmpeg to split the video.
An update on this:
At Mozilla, we've been looking for a way to integrate the visual-metrics processing into where the tests run (rather than running the processing in a docker). The issue we have is that we to test on so many platforms that the Imagemagick dependency prevents us from doing this easily. So I've started removing the Imagemagick usage from the visualmetrics.py script. You can find the fully-functional script here: https://github.com/gmierz/browsertime/blob/old-vm-fix/browsertime/visualmetrics.py
Currently works on all platforms, and browsers and nearly all metrics are exactly the same before and after. It needs a little more validation for the ContentfulSpeedIndex though. That said, the numbers are extremely close to each other now and we see no major issues with it. We plan on performing a visual validation of the edges produced in the new method vs the old method then (as discussed with @soulgalore), we'll make a PR to create a new visualmetrics-portable.py script that has these changes.
One thing to note is that in the portable script, I will be removing the hero element method, and the JPEG saving methods. If someone is interested in implementing these it shouldn't be difficult, but we have no use cases for these two methods so I am considering it as out-of-scope for this work. The portable script will provide the bare minimum (an MVP) for obtaining all the standard visual metrics (First/LastVisualChange, *SpeedIndexes, Recording Start, and the progress metrics) without Imagemagick.
It would be nice to get rid of FFMPEG but we've found that it's very easy to install and maintain across multiple platforms so I didn't delve into that removal.
Cool @gmierz I sure want to try it out, but I'm gonna need to add hero element and JPEG too, let me explain why:
-
the "hero" (I hate that name by the way) is super useful to pickup when elements are actually rendered (for example knowing the logo and the largest image). It uses element timings (not implemented in Firefox), so what's extra cool about it is that if you annotations those elements it works for all brothers in Browsertime, even though that the browser do not support it.
-
JPEG is nice since we store a lot of thumbnails. we visualise it like this. That example is ok since we only have a couple of screenshots but for some sites that takes longer time, we generate many many images.
Then there are other visual metrics functionality that we do not use (combining image as a video and the check for a "grey" frame, that was for an old Chrome bug) and I guess there are more? Maybe we can remove those from the new file too. That BLOB visualmetric.py is something I really want to get rid off or make easier to understand. Either if we could split the functionality into multiple smaller files, but preferable I want to move it to NodeJS.
Ok, I should be able to easily add the JPEG method. @soulgalore can you provide a visualmetrics.py command and data to test the hero element? I'll spend a bit of time there to see if I can quickly implement it.
I agree that changing the structure of the file would be great but I think that's something we can leave for others to tackle.
Thank you @gmierz !
I added a video and a JSON with matching test data in https://github.com/sitespeedio/browsertime/tree/hero/browsertime/test_data/hero
./visualmetrics.py --video test_data/hero/1.mp4 --herodata test_data/hero/visualElements.json.gz --orange --json --force
@soulgalore would you know why the hero calculation is using the alpha channel of the PNG frames?
It seems like it's just a trick to make masking faster, but I want to make sure before I get rid of that. I don't see a reason for checking the alpha channel here when we don't take it into consideration in any of the other methods. I'm going to test it without the alpha handling, but I'm curious to know if there's an edge case I should be aware of here.
Also, just a note that I've already converted the JPEG methods to use python dependencies.
Ah great with JPEG :)
No I don't know, please do what is easiest and best for you. The story about the visual elements is interesting: It was contributed to WebPageTest by the Speedcurve people, I lifted it from visual metrics from WebPageTest but then when WebPageTest changed licensed (without asking all contributors) the functionality was removed, if I remember it correctly. Don't know the story behind it but it would be interesting to know.
Ok cool, thanks for the context! :)
I've the hero calculation working now. With the data you've given me, I am able to get the same numbers:
With existing script:
{"SpeedIndex": 815, "FirstVisualChange": 766, "LastVisualChange": 1000, "VisualProgress": "0=0, 766=69, 833=69, 900=92, 966=92, 1000=100", "videoRecordingStart": 900, "LargestImage": 1000, "LastMeaningfulPaint": 1000, "logo": 1000, "Heading": 900, "header-logo": 766}
With python dependencies:
{"FirstVisualChange": 766, "LastVisualChange": 1000, "SpeedIndex": 815, "LargestImage": 1000, "Heading": 900, "header-logo": 0, "logo": 1000, "LastMeaningfulPaint": 1000, "VisualProgress": "0=0, 766=69, 833=69, 900=92, 966=92, 1000=100", "videoRecordingStart": 900}
Note how the header-logo is missing a value. I was really confused about why it works for all the others and not this one. I dug into it a bit and figured out that the alpha channel usage in the existing script was actually interfering with the logo mask and th hero timing calculation. Here's the example I found this from.
Here's a target mask, as well as the current mask from the code using the python dependencies:
Target:
Current mask at time 0:
With my compare method I was getting 13 different pixels here. With the previous script using Imagemagick, I would get a difference of 30 from the following images (they are the same frames as above):
Target:
Current mask at time 0:
If you download those last two images and zoom in around the edges of the element that was cropped, you'll notice that there is a ton of blurring. This is where that difference of 17 pixels is coming from. It's clear that this is the case because for all intents and purposes, the target header logo is virtually equal to the current header logo at time 0 so the header-logo time should also be 0. In other words, the existing implementation of calculating a hero element is bugged and moving to python dependencies will fix it.
@soulgalore here's the new script with the JPEG, and hero timing methods converted to use python dependencies: https://github.com/gmierz/browsertime/commit/3ba27ec362876b5346569a9fbfe709bf82532d72
PR #1741 has landed and adds a new portable visualmetrics script (usable with --visualMetricsPortable) with ImageMagick dependencies converted into Python dependencies. I didn't want to link this issue with the PR directly since there are still some tasks to take care of:
- Better organization of the files.
- Removal of FFMPEG (if possible).
- Conversion to a Node.js script.
@gmierz many many thanks for your work! If its ok for you I'm gonna PR later this week to the portable version to remove all the functionality we do not use in Browsertime (and you can review that I don't remove something you use at Mozilla). I want shrink than 2756 lines of code to make it mora manageable as a first step.
A did quick check I think we can reduce the size by at least 700-800 lines of code :)
Idea from @soulgalore from PR #1780:
One thing though, I'm thinking in the future we should add a way of setting the filmstrip screenshot size, the default one is actually too small when you try to see those little changes. And the full screenshot is really large.