WMS Driver Clobbers Transparency

[joelodom]

The WMS driver is apparently reformatting raster data and loosing alpha information in the process. Here is a WMS service description:

<GDAL_WMS>
  <Service name="WMS">
    <ServerURL>http://mesonet.agron.iastate.edu/cgi-bin/wms/nexrad/n1p.cgi?TRANSPARENT=TRUE</ServerURL>
    <ImageFormat>image/png</ImageFormat>
    <Layers>nexrad-n1p</Layers>
  </Service>
  <DataWindow>
    <UpperLeftX>-102.017258</UpperLeftX>
    <UpperLeftY>47.974853</UpperLeftY>
    <LowerRightX>-93.570578</LowerRightX>
    <LowerRightY>42.602123</LowerRightY>
    <SizeX>948</SizeX>
    <SizeY>603</SizeY>
  </DataWindow>
  <Timeout>45</Timeout>
</GDAL_WMS>

GDAL returns the raster as a 3 band PNG. If I put the following URL in my browser and load the PNG file with GDALOpen, I get a single band raster with a 4 byte color table.

​http://mesonet.agron.iastate.edu/cgi-bin/wms/nexrad/n1p.cgi?TRANSPARENT=TRUE&request=GetMap&version=1.1.0&layers=nexrad-n1p&styles=&srs=EPSG:4326&format=image/png&width=948&height=603&bbox=-106.24059800,39.92912300,-97.79391800,45.30185300

[nowakpl]

<BandsCount>4</BandsCount> missing from the config file, not a bug.

[warmerdam]

Cool, thanks Adam!

[ChrisABailey]

in version 1.6.2.

It looks like even when you do specify BandCount = 4, that there are still some issues:

1st the WMS Request does not have the transparent parameter set to true so the WMS does not return an image with the proper alpha channel (even if the request format is PNG).

2nd if the server does not return a format with 4 bands (for instance terraserver only returns JPEG images), then the wms driver reports an error when you try to retreive any of the bands because the requested number of bands does not equal the number of bands in the returned file.

I have made changes to address both of these issues and also to properly set the BandColorInterpratation on the !WMSRasterBands which were always set to unknown.

I am attaching the three files that I modified in the fmts/wms project.

In minidriver_wms.cpp I modified the reading of the XML WMS config file to support a new "Transparent" Tag. If the XML contains <Transparent> True <\transparent> then a new member variable m_transparent will be set to true. which is user in constructing the WMS request.

This is now line 112:

m_transparent = CPLGetXMLValue(config, "Transparent","");

then on line 154:

if (m_transparent.size()) URLAppendF(url, "&transparent=%s", m_transparent.c_str());

Similarly I added the member variable to minidriver_wms.h on line 158.

Finally in rasterband.cpp I made several modifications. I added calls to SetColorInterpretation() based on the ColorInterpretation returned when the results file is opened.

I also now create an all opaque alpha band and return it if the WMS returns a three band raster when the client requests a four band raster. The way I read the spec, the WMS does not need to honor requests for transparency so this is not an error condition.

                                if (!accepted_as_ct) {
                                        if (ds->GetRasterCount()==3 && m_parent_dataset->m_bands_count == 4 && (eDataType == GDT_Byte))
                                        { // WMS returned a file with no alpha / fill with opaque  (Is this too much of a hack?)
                                                accepted_as_no_alpha = true;
                                        }
                                        else
                                        {
                                                CPLError(CE_Failure, CPLE_AppDefined, "GDALWMS: Incorrect bands count %d in downloaded block, expected %d.",
                                                        ds->GetRasterCount(), m_parent_dataset->m_bands_count);
                                                ret = CE_Failure;
                                        }
                                }
            }
        }
        if (!advise_read) {
            for (int ib = 1; ib <= m_parent_dataset->nBands; ++ib) {
                if (ret == CE_None) {
                    void *p = NULL;
                    GDALRasterBlock *b = NULL;
                    if ((buffer != NULL) && (ib == to_buffer_band)) {
                        p = buffer;
                    } else {
                        GDALWMSRasterBand *band = static_cast<GDALWMSRasterBand *>(m_parent_dataset->GetRasterBand(ib));
                        if (m_overview >= 0) band = static_cast<GDALWMSRasterBand *>(band->GetOverview(m_overview));
                        if (!band->IsBlockInCache(x, y)) {
                            b = band->GetLockedBlockRef(x, y, true);
                            if (b != NULL) {
                                p = b->GetDataRef();
                                if (p == NULL) {
                                  CPLError(CE_Failure, CPLE_AppDefined, "GDALWMS: GetDataRef returned NULL.");
                                  ret = CE_Failure;
                                }
                            }
                        }
                        else
                        {
                            //CPLDebug("WMS", "Band %d, block (x,y)=(%d, %d) already in cache", band->GetBand(), x, y);
                        }
                    }
                    if (p != NULL) {
                        int pixel_space = GDALGetDataTypeSize(eDataType) / 8;
                        int line_space = pixel_space * nBlockXSize;
                                                if (color_table == NULL) {
                                                        if( ib <= ds->GetRasterCount()) {
                                                                if (ds->RasterIO(GF_Read, 0, 0, sx, sy, p, sx, sy, eDataType, 1, &ib, pixel_space, line_space, 0) != CE_None) {
                                                                        CPLError(CE_Failure, CPLE_AppDefined, "GDALWMS: RasterIO failed on downloaded block.");
                                                                        ret = CE_Failure;
                                                                }
                                                                else
                                                                {  // new code to set color intrepretation which was always GCI_Undefined
                                                                        GDALRasterBand *band = (m_parent_dataset->GetRasterBand(ib));
                                                                        band->SetColorInterpretation(ds->GetRasterBand(ib)->GetColorInterpretation());
                                                                }
                                                        }
                                                        else
                                                        {   
                                                                if (accepted_as_no_alpha)
                                                                {
                                                                        // the file had 3 bands and we are reading band 4 (Alpha) so fill with 255 (no alpha)
                                                                        GByte *byte_buffer = reinterpret_cast<GByte *>(p);
                                                                        for (int y = 0; y < sy; ++y) {
                                                                                for (int x = 0; x < sx; ++x) {
                                                                                        const int offset = x + y * line_space;
                                                                                        byte_buffer[offset] = 255;  // fill with opaque
                                                                                }
                                                                        }
                       // new code to set color intrepretation which was always GCI_Undefined
                                                                        GDALRasterBand *band = (m_parent_dataset->GetRasterBand(ib));
                                                                        band->SetColorInterpretation(GCI_AlphaBand);
                                                                }
                                                                else
                                                                {  // we should never get here because this case was caught above
                                                                        CPLError(CE_Failure, CPLE_AppDefined, "GDALWMS: Incorrect bands count %d in downloaded block, expected %d.",
                                                                                ds->GetRasterCount(), m_parent_dataset->m_bands_count);
                                                                        ret = CE_Failure;
                                                                }

                                                        }
                                                } else if (ib <= 4) {
                                                        if (ds->RasterIO(GF_Read, 0, 0, sx, sy, p, sx, sy, eDataType, 1, NULL, pixel_space, line_space, 0) != CE_None) {
                                                                CPLError(CE_Failure, CPLE_AppDefined, "GDALWMS: RasterIO failed on downloaded block.");
                                                                ret = CE_Failure;
                            }
                                                        else
                                                        { // new code to set color intrepretation which was always GCI_Undefined
                                                                GDALRasterBand *band = (m_parent_dataset->GetRasterBand(ib));
                                                                band->SetColorInterpretation((GDALColorInterp)((int)GCI_RedBand + ib-1));
                                                        }
                            if (ret == CE_None) {
                                GByte *band_color_table = color_table + 256 * (ib - 1);
                                GByte *byte_buffer = reinterpret_cast<GByte *>(p);
                                for (int y = 0; y < sy; ++y) {
                                    for (int x = 0; x < sx; ++x) {
                                        const int offset = x + y * line_space;
                                        byte_buffer[offset] = band_color_table[byte_buffer[offset]];
                                    }
                                }
                            }
                        } else {
                            CPLError(CE_Failure, CPLE_AppDefined, "GDALWMS: Color table supports at most 4 components.");
                            ret = CE_Failure;
                        }
                    }

If you choose to accept this as a patch, then the documentation on the WMSDriver needs to be updated to reflect the new XML keyword.

[nowakpl]

A few notes:
1) Please convert all tabs to spaces.
2) I don't think we should be setting color interpretation from within RasterIO call, this info should be available right after GDALOpen and should not change later. Maybe a <ColorInterpretation>...</ColorInterpretation> setting with each char indicating what color interpretation to set for given band. "R" - red, "G" - green, "B" - blue, "A" - alpha, "g" - gray, "u" - unknown. "RGB" would be set band 1 to red, band 2 to green, band 3 to blue, or "GBR" set band 1 to green, band 2 to blue, band 3 to red. Default for 1 band image would be "g", 3 band "RGB" and 4 band "RGBA". For other band counts default color interpretation would be unknown.

[ChrisABailey]

I agree it would be better if we could know the color interpretation right after the call to open but can we know the color interpretation that early? I believe the WMS request is not actually made until the call to RasterIO? (actually in ReadBlockFromFile)

I am new to GDAL so I don't know how other datasets handle this. Is the band interpretation normally the actual bands in the file or the desired interpretation. For instance, what if the server returns a TIF in CMYK?

I'll resend with tabs changed to spaces. Are there any other obvious coding conventions that I am violating?

[ChrisABailey]

minidriver_wms.h in fmts/wms project (spaces not tabs)

[ChrisABailey]

I have not heard anything since I made my last response and uploaded the code with spaces instead of tabs. Have you given any more thoughts to my question on weather the caller should define the bands expected in the response vice waiting on the response to get the bands.

[nowakpl]

I don't see any other way than reading the color interpretation settings from the config file, as described above. Also the default mode should be to map local and remote bands 1:1 so band 1 of image returned by server is always stored into band 1 locally, even if local and remote color interpretations differ. Then on top of that could be added some guessing logic, enabled by the config file, that will try to match remote and local color interpretations (possibly doing conversions CMYK - RGB), or reconstruct missing bands like alpha.

[ChrisABailey]

Updated RaserBands which addresses transparency (but does not change the Color Interpretation)

[ChrisABailey]

New version of wms_minidriver which forces the any transparency to UPPER case as described in the WMS Specification.

[ChrisABailey]

I just uploaded new files which only address the transparency (and not the Color interpretation). I also fixed a bug in my earlier implementation where I passed the Transparency flag as "true" instead of "TRUE". This was causing one server to return an error.

I tested with the following web services:

​http://terraserver-usa.com/ogccapabilities.ashx?version=1.1.1&request=getcapabilities&service=wms

​http://mesonet.agron.iastate.edu/cgi-bin/wms/nexrad/n0r.cgi?

​http://onearth.jpl.nasa.gov/wms.cgi?

It appears to be sound. oneearth and mesonet both support transparency as expected. terraserver ignores the transparency and the code creates an all opaque 4th band (since the request xml requested 4 bands.)

I will try to look into the proposed solution for the undefined color interpretation but it was not obvious how to implement the change. If I figure it out, I'll open a separate bug and attach a patch there.

Is there some way to lobby to get this patch included in the next 1.6 and 1.7 releases?

[Even Rouault]

Chris, did you actually test your attached patch in *trunk* ? I see you have resurected the m_bands_count member in wmsdriver.h that I killed a few weeks ago, because it was redundant with the nBands member of GDALDataset. So I suspect your test 'if (ds->GetRasterCount()==3 && m_parent_dataset->m_bands_count == 4 && (eDataType == GDT_Byte))' tests against an uninitialized value...

[ChrisABailey]

You are right that I have not tested all the cases in the trunk. I reteested with 1.6 over the weekend and I built against 1.7.

I will run some tests and report back before I would ask you to check anything in. My main code does not hit the case (of requesting 4 bands and getting back 3) so I need to create a test using some different sample code. I believe terraserver will ignore transparency since it does not support PNG (only JPEG).

[ChrisABailey]

updated Patch to WMS Server

[ChrisABailey]

The attached patch was tested against revision 18285. The transparenct works with:

​http://terraserver-usa.com/ogccapabilities.ashx?version=1.1.1&request=getcapabilities&service=wms

​http://mesonet.agron.iastate.edu/cgi-bin/wms/nexrad/n0r.cgi?

​http://onearth.jpl.nasa.gov/wms.cgi?

​http://mapbender.wheregroup.com/cgi-bin/mapserv?map=/data/umn/osm/osm_basic.map&VERSION=1.1.1&REQUEST=GetCapabilities&SERVICE=WMS

We also tested the tiled map services without transparancy to verify that there was no loss of functionality.

I will update the HTML document shortly.

I think this is stable enough to check in to the trunk but I don't know if there are any automated tests that need to be updated to verify the change.

[Even Rouault]

Adam, can you review the proposed patch ? From my point of view, it looks OK (the wms.py tests still pass after applying it), so if you don't have objections, I'll commit it.

Chris, there are a few tests here for the WMS driver : http://trac.osgeo.org/gdal/browser/trunk/autotest/gdrivers/wms.py

However WMS testing is a particular case as we depend on remote data. So tests tend to be fragile (server goes down, or sometimes fails because of overload, or data is updated, etc...). So only add some if you're confident that the WMS source will be reliable.

[ChrisABailey]

Update to the html document.

[nowakpl]

A bit dirty but acceptable.

[Even Rouault]

r18326 /trunk/gdal/frmts/wms/ (4 files): <CIA-27>WMS: Add support for adding TRANSPARENT=TRUE in the WMS request; Accept that <CIA-27>images returned by the server have only 3 bands when we expect 4 bands, in the <CIA-27>case the fourth band is considered to be fully opaque (255) (contributed by <CIA-27>ChrisABailey, #3042)

I've just edited frmts_wms.html in a minor way to set the value for TRANSPARENT to FALSE as this is the default value and a quick look shows that default values are consistenly used in the exemple.

Thanks.