Bandwidth bug...

[IanSav]

Hi Guys,

I would like to confirm what I believe to be a bug in the code that is manifesting in the ServiceName2.py converter.

In line 314 of ServiceName2.py there is a format option "%b" that is meant to return the DVB-T bandwidth:

Code: Select all

elif f == 'b':	# %b - bandwidth (dvb-t)
	if type == 'DVB-T':
		x = self.tpdata.get('bandwidth', 1)
		result += x in range(4) and {0:'8 MHz',1:'7 MHz',2:'6 MHz',3:'Auto'}[x] or ''

This code expects to get an index value for the bandwidth. This index is used to lookup the appropriate bandwidth in a bandwidth table to be returned.

The problem is that somewhere along the line the data returned is no longer an index but rather the actual bandwidth in Hz. The self.tpdata.get('bandwidth', 1) call expects to see a value of "1" for 7 MHz bandwidth but instead gets "7000000". The lookup obviously fails so an empty string is returned.

I have coded a small patch to work around the issue in ServiceName2.py:

Code: Select all

elif f == 'b':	# %b - bandwidth (dvb-t)
	if type == 'DVB-T':
		x = self.tpdata.get('bandwidth', 1)
		if x < 100:
			result += x in range(4) and {0:'8 MHz',1:'7 MHz',2:'6 MHz',3:'Auto'}[x] or ''
		else:
			bands = {1712000: "1.712 MHz", 5000000: "5 MHz", 6000000: "6 MHz", 7000000: "7 MHz", 8000000: "8 MHz", 10000000: "10 MHz"}
			result += bands.get(x, "Auto")

This patch is quite a hack but it works well enough.

My concern is that if this code is expecting data in one format and it arrives in another format then how often is this problem happening elsewhere in the code?

Should this hack be used to fix the problem here, is a similar hack needed elsewhere or should the underlying issue be addressed? I believe it will be in all our interests if the issue is investigated by those with a better understanding and grip on this type of code and the adjustments and/or corrections be applied across the code base. This is an issue that I am unable to investigate.

If it is decided to return the data to an index into a bandwidth table then the original list of options will need to be expanded to encompass all the current bandwidth options (see my patch "bands" dictionary).

Can someone please weigh in on this problem and suggest how it can/should be addressed.

Regards,
Ian.

[prl]

When the bandwidth is expressed in Hz, why do a table lookup rather than something like ("%.3f % (float(x) / 1000000.0)).rstrip('0').rstrip('.')?

[prl]

It looks as though the bandwidth in the frontend parameters (Bandwidth_8MHz, Bandwidth_7MHz, etc) was changed in the C++ code from the set of small integers representing the bandwidth options to the bandwidth in Hz in commits 7df3473 & 24b3f5a back in March 2012. It wouldn't surprise me if the "%b" bandwidth display option hadn't been used by anyone between then and when you tried to use it.

It's not uncommon for me to encounter old code that apparently has never been tested.

I don't think that the old small-integer values should appear in the data returned by getInfoObject(iServiceInformation.sTransponderData).

getInfoObject(iServiceInformation.sTransponderData) is also called in Components/Converter/TransponderInfo.py, but there the number for the bandwidth is simply displayed raw.

The bandwidth conversion in Tools/Transponder.ConvertToHumanReadable() assumes that the bandwidth is always in Hz (and explicitly assumes that 0 -> Auto). However, it uses a lookup table and will return None for the bandwidth if the bandwidth isn't in the lookup table. I haven't looked at uses of the output of ConvertToHumanReadable() to see whether that would be a problem.

Components/TimerSanityCheck.py and Screens/Dish.py use getInfoObject(iServiceInformation.sTransponderData), but don't look at the bandwidth.

Screens/ServiceInfo.py uses getInfoObject(iServiceInformation.sTransponderData) and displays the bandwidth using ConvertToHumanReadable() above.

In Components/Converter/PliExtraInfo.py, PliExtraInfo.createSymbolRate() simply returns the raw bandwidth number, and elsewhere in PliExtraInfo where a bandwidth is returned, it's done using ConvertToHumanReadable().

[IanSav]

Hi Prl,

When the bandwidth is expressed in Hz, why do a table lookup rather than something like ("%.3f % (float(x) / 1000000.0)).rstrip('0').rstrip('.')?

Your suggestion is *much* better than my hack. Your next post about the issue itself is more what I was hoping for.

Regards,
Ian.

[prl]

...

When the bandwidth is expressed in Hz, why do a table lookup rather than something like ("%.3f % (float(x) / 1000000.0)).rstrip('0').rstrip('.')?

Your suggestion is *much* better than my hack. Your next post about the issue itself is more what I was hoping for.
...

It may not be a bad idea to use it in Tools.Transponder.ConvertToHumanReadable(), too.

Of course, bandwidth 0 still has to be special cased.

The second post is really mostly just the result of some find/greps.

[IanSav]

Hi Prl,

Given your investigation and comments I intend to fix the "%b" in ServiceName2.py as:

Code: Select all

elif f == 'b':	# %b - bandwidth (dvb-t)
	if type == 'DVB-T':
		x = self.tpdata.get('bandwidth', 0)
		if isinstance(x, int):
			result += str("%.3f" % (float(x) / 1000000.0)).rstrip('0').rstrip('.') + " MHz" if x else "Auto"

I can code something similar for Transponder.py:

Code: Select all

elif tunertype == "DVB-T":
	ret["tuner_type"] = _("Terrestrial")
	x = tp.get("bandwidth")
	if isinstance(x, int):
		x = str("%.3f" % (float(x) / 1000000.0)).rstrip('0').rstrip('.') + " MHz" if x else "Auto"
	else:
		x = ""
	ret["bandwidth"] = x

Thank you for your opinion and suggestions.

Regards,
Ian.

[prl]

That looks fine to me.

[IanSav]

Hi,

Pull requests #147 - [ServiceName2.py] Fix DVB-T bandwidth conversion and #148 - [Transponder.py] Improve DVB-T bandwidth logic have been filed with OpenViX to address this issue and improvement. When the requests are merged I will post here so PeterU can pull those requests into the Beyonwiz repository.

Regards,
Ian.

[IanSav]

Hi PeterU,

These fixes have been merged into the OpenViX Dev branch.

Regards,
Ian.