flypig.co.uk

I've been enjoying some good progress over the last few days, first fixing a crash bug during video playback and then fixing the discolouration that's been plaguing video playback. That's boosted my confidence and given me the momentum I need to try to tackle the WebRTC video discolouration issue.

The problem with video decoding turned out to the data frames that were being initialised without a valid colour space value. Having set this value the colours returned to their expected state. So I'm thinking maybe there's something similar going on with the WebRTC video decoding? After all, both are making use of the gecko-camera code.

In the gecko code, however, they take different routes. The video decoding is handled by the GeckoCameraVideoDecoder.cpp source file in the dom/media/platforms/gecko-camera/ directory. The WebRTC decoding is handled by code in device_info_sfos.cc and video_capture_sfos.cc in the third_party/libwebrtc/webrtc/modules/video_capture/sfos/ directory.

On the face of it they don't share functionality and unfortunately there's no code similar to the video decoding pipeline in the WebRTC code. The closest I can find to the colour space fix in the WebRTC code is where the video type is set in the FillCapabilities() method in device_info_sfos.cc:

                VideoCaptureCapability vcaps;
                vcaps.width = cap.width;
                vcaps.height = cap.height;
                vcaps.maxFPS = cap.fps;
                vcaps.videoType = VideoType::kI420;
                _captureCapabilities.push_back(vcaps);

Could it be that the video type should be set to something different? The list of possible values is summarised in the Mozilla docs with the canonical list found in the common_types.h file looking like this:

enum class VideoType {
  kUnknown,
  kI420,
  kIYUV,
  kRGB24,
  kABGR,
  kARGB,
  kARGB4444,
  kRGB565,
  kARGB1555,
  kYUY2,
  kYV12,
  kUYVY,
  kMJPEG,
  kNV21,
  kNV12,
  kBGRA,
};

As we can see from the code snippet from inside FillCapabilities() earlier, currently the video type is being set to I420. Here's the description for this from the docs:
 
This looks to me to be essentially the same as the Y'CbCr format that the video decoder was using. This is interesting because it highlights that the three channels aren't all the same size: the two chroma channels are sub-sampled by a factor of two in both directions (so giving a quarter of the area). Given this, it's hard to imagine that the data isn't being correctly interpreted as I420 data at the other end. If it were interpreted as RGB data, say, all three channels would then need to be the same size and this would create the most almighty mess if it wasn't.

After looking more through the code I can't see anything wrong on the gecko side. As Frajo (krnlyng) explains on the GitHub ticket the issue is probably related to code in gecko-camera that's incorrectly initialising a DroidMediaBufferYCbCr structure. There are two instances of this structure used in the gecko-camera code and I can see one being correctly passed to droid_media_buffer_lock_ycbcr() before being used. The other, however, isn't.

It could be that passing the other instance through the same method would give positive results. But right now, just as Frajo hypothesised, this very much looks like it's a problem in gecko-camera rather than in the gecko code itself. And that means I'm still going to put it to one side for the time being. Maybe I'll come back to this later.

That means I need to move on to the next task in my list, which is figuring out why some sites aren't correctly fitting to the screen width. There are two examples of sites where I've seen this happen. The most obvious is DuckDuckGo. This happens to be the site I use most frequently for searching the Web, so from my perspective it'd be really nice if it were working correctly with the browser.

Unfortunately that's not currently the case. It renders, but extends over roughly three times the width of the screen, as you can see in these screenshots.
 
We see something similar happening with websites that use Discourse for their forum backend. The header bar appears to extend across two screen widths, even though the topic items correctly extend across just a single screen. It's usable but looks like a bit of a mess.
 
We go to some lengths in the browser code to set the correct dimensions, dots-per-inch values and scaling so that pages render correctly, especially if they're designed to be responsive. But there are a myriad ways that a browser can query the dimensions of a page, so my working hypothesis is that one or more of the values the browser is returning to the JavaScript on the page are incorrect. That might explain why, for example, the DuckDuckGo input field is the correct width across a single page, but the search results are not. Maybe the first is using one API to determine the width and the other using something different?

To try to test this I've put together a test page that displays all of the ways I can think of to query relevant dimensions. It turns out there are quite a few! Unfortunately, comparing the results from the ESR 78 build and the ESR 91 build I'm getting identical results. I've even worked through all of the tests on QuirksMode with no apparent difference between the two versions of the browser. Either I'm missing a critical JavaScript API from the ones I'm testing, or there's something else going on here.
 
But when displaying the test page using Firefox on my laptop in Responsive Design Mode (which allows you to change the dimensions of the page), I accidentally discover something significant: when rendering DockDuckGo on desktop Firefox with the screen width reduced, it actually spans three screen widths as well.
 
Playing around with the browser's user agent string shows that this is all intentional and happening server-side. By changing the user-agent to an Android version string suddenly things start working correctly, with the search results spanning just a single page. It seems a little odd, but presumably when DuckDuckGo thinks a desktop browser is in use, it intentionally uses a wider page size. So I've added the following two lines to the ua-update.json:

  "duckduckgo.com": "Mozilla/5.0 (Mobile; rv:91.0) Gecko/91.0 
    Firefox/91.0",
  "openai.com": "Mozilla/5.0 (Mobile; rv:91.0) Gecko/91.0 
    Firefox/91.0"

With this applied, now both DuckDuckGo and the OpenAI forum that uses Discourse are working correctly, filling just a single width of the screen:
 
So that's resolved one of the last few things on my list to fix. The next is getting the browser to run on Sailfish OS 4.6. My concern is that getting this working is going to be challenging, but that's something for tomorrow.

If you'd like to read any of my other gecko diary entries, they're all available on my Gecko-dev Diary page.