linux-media.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Hans de Goede <hdegoede@redhat.com>
To: Konke Radlow <kradlow@cisco.com>
Cc: linux-media@vger.kernel.org, hverkuil@xs4all.nl, koradlow@gmail.com
Subject: Re: [RFC PATCH 2/2] Add rds-ctl tool (with changes proposed in RFC)
Date: Thu, 09 Aug 2012 14:05:04 +0200	[thread overview]
Message-ID: <5023A770.5080604@redhat.com> (raw)
In-Reply-To: <3bb2b81ed5c186756c83b9136b5aa43005d728a2.1344352285.git.kradlow@cisco.com>

Hi,

Comments inline.

On 08/07/2012 05:11 PM, Konke Radlow wrote:
> ---
>   Makefile.am               |    3 +-
>   configure.ac              |    1 +
>   utils/rds-ctl/Makefile.am |    5 +
>   utils/rds-ctl/rds-ctl.cpp |  959 +++++++++++++++++++++++++++++++++++++++++++++
>   4 files changed, 967 insertions(+), 1 deletion(-)
>   create mode 100644 utils/rds-ctl/Makefile.am
>   create mode 100644 utils/rds-ctl/rds-ctl.cpp
>
> diff --git a/Makefile.am b/Makefile.am
> index 621d8d9..8ef0d00 100644
> --- a/Makefile.am
> +++ b/Makefile.am

<Snip>

> +static void print_rds_data(const struct v4l2_rds *handle, uint32_t updated_fields)
> +{
> +	if (params.options[OptPrintBlock])
> +		updated_fields = 0xffffffff;
> +
> +	if ((updated_fields & V4L2_RDS_PI) &&
> +			(handle->valid_fields & V4L2_RDS_PI)) {
> +		printf("\nPI: %04x", handle->pi);
> +		print_rds_pi(handle);
> +	}
> +
> +	if (updated_fields & V4L2_RDS_PS &&
> +			handle->valid_fields & V4L2_RDS_PS) {
> +		printf("\nPS: ");
> +		for (int i = 0; i < 8; ++i) {
> +			/* filter out special characters */
> +			if (handle->ps[i] >= 0x20 && handle->ps[i] <= 0x7E)
> +				printf("%lc",handle->ps[i]);
> +			else
> +				printf(" ");
> +		}


Since ps now is a 0 terminated UTF-8 string you should be able to just do:
		printf("\nPS: %s", handle->ps);

And likewise for the other strings.

> +	}
> +
> +	if (updated_fields & V4L2_RDS_PTY && handle->valid_fields & V4L2_RDS_PTY)
> +		printf("\nPTY: %0u -> %s",handle->pty, v4l2_rds_get_pty_str(handle));
> +
> +	if (updated_fields & V4L2_RDS_PTYN && handle->valid_fields & V4L2_RDS_PTYN) {
> +		printf("\nPTYN: ");
> +		for (int i = 0; i < 8; ++i) {
> +			/* filter out special characters */
> +			if (handle->ptyn[i] >= 0x20 && handle->ptyn[i] <= 0x7E)
> +				printf("%lc",handle->ptyn[i]);
> +			else
> +				printf(" ");
> +		}

Likewise.

> +	}
> +
> +	if (updated_fields & V4L2_RDS_TIME) {
> +		printf("\nTime: %s", ctime(&handle->time));
> +	}
> +	if (updated_fields & V4L2_RDS_RT && handle->valid_fields & V4L2_RDS_RT) {
> +		printf("\nRT: ");
> +		for (int i = 0; i < handle->rt_length; ++i) {
> +			/* filter out special characters */
> +			if (handle->rt[i] >= 0x20 && handle->rt[i] <= 0x7E)
> +				printf("%lc",handle->rt[i]);
> +			else
> +				printf(" ");
> +		}

Likewise.

> +	}
> +
> +	if (updated_fields & V4L2_RDS_TP && handle->valid_fields & V4L2_RDS_TP)
> +		printf("\nTP: %s  TA: %s", (handle->tp)? "yes":"no",
> +			handle->ta? "yes":"no");
> +	if (updated_fields & V4L2_RDS_MS && handle->valid_fields & V4L2_RDS_MS)
> +		printf("\nMS Flag: %s", (handle->ms)? "Music" : "Speech");
> +	if (updated_fields & V4L2_RDS_ECC && handle->valid_fields & V4L2_RDS_ECC)
> +		printf("\nECC: %X%x, Country: %u -> %s",
> +		handle->ecc >> 4, handle->ecc & 0x0f, handle->pi >> 12,
> +		v4l2_rds_get_country_str(handle));
> +	if (updated_fields & V4L2_RDS_LC && handle->valid_fields & V4L2_RDS_LC)
> +		printf("\nLanguage: %u -> %s ", handle->lc,
> +		v4l2_rds_get_language_str(handle));
> +	if (updated_fields & V4L2_RDS_DI && handle->valid_fields & V4L2_RDS_DI)
> +		print_decoder_info(handle->di);
> +	if (updated_fields & V4L2_RDS_ODA &&
> +			handle->decode_information & V4L2_RDS_ODA) {
> +		for (int i = 0; i < handle->rds_oda.size; ++i)
> +			printf("\nODA Group: %02u%c, AID: %08x",handle->rds_oda.oda[i].group_id,
> +			handle->rds_oda.oda[i].group_version, handle->rds_oda.oda[i].aid);
> +	}
> +	if (updated_fields & V4L2_RDS_AF && handle->valid_fields & V4L2_RDS_AF)
> +		print_rds_af(&handle->rds_af);
> +	if (params.options[OptPrintBlock])
> +		printf("\n");
> +}
> +
> +static void read_rds(struct v4l2_rds *handle, const int fd, const int wait_limit)
> +{
> +	int byte_cnt = 0;
> +	int error_cnt = 0;
> +	uint32_t updated_fields = 0x00;
> +	struct v4l2_rds_data rds_data; /* read buffer for rds blocks */
> +
> +	while (!params.terminate_decoding) {
> +		memset(&rds_data, 0, sizeof(rds_data));
> +		if ((byte_cnt=read(fd, &rds_data, 3)) != 3) {
> +			if (byte_cnt == 0) {
> +				printf("\nEnd of input file reached \n");
> +				break;
> +			} else if(++error_cnt > 2) {
> +				fprintf(stderr, "\nError reading from "
> +					"device (no RDS data available)\n");
> +				break;
> +			}
> +			/* wait for new data to arrive: transmission of 1
> +			 * group takes ~88.7ms */
> +			usleep(wait_limit * 1000);
> +		}
> +		else if (byte_cnt == 3) {
> +			error_cnt = 0;
> +			/* true if a new group was decoded */
> +			if ((updated_fields = v4l2_rds_add(handle, &rds_data))) {
> +				print_rds_data(handle, updated_fields);
> +				if (params.options[OptVerbose])
> +					 print_rds_group(v4l2_rds_get_group(handle));
> +			}
> +		}
> +	}
> +	/* print a summary of all valid RDS-fields before exiting */
> +	printf("\nSummary of valid RDS-fields:");
> +	print_rds_data(handle, 0xFFFFFFFF);
> +}
> +
> +static void read_rds_from_fd(const int fd)
> +{
> +	struct v4l2_rds *rds_handle;
> +
> +	/* create an rds handle for the current device */
> +	if (!(rds_handle = v4l2_rds_create(true))) {
> +		fprintf(stderr, "Failed to init RDS lib: %s\n", strerror(errno));
> +		exit(1);
> +	}
> +
> +	/* try to receive and decode RDS data */
> +	read_rds(rds_handle, fd, params.wait_limit);
> +	print_rds_statistics(&rds_handle->rds_statistics);
> +
> +	v4l2_rds_destroy(rds_handle);
> +}
> +
> +static int parse_cl(int argc, char **argv)
> +{
> +	int i = 0;
> +	int idx = 0;
> +	int opt = 0;
> +	/* 26 letters in the alphabet, case sensitive = 26 * 2 possible
> +	 * short options, where each option requires at most two chars
> +	 * {option, optional argument} */
> +	char short_options[26 * 2 * 2 + 1];
> +
> +	if (argc == 1) {
> +		usage_hint();
> +		exit(1);
> +	}
> +	for (i = 0; long_options[i].name; i++) {
> +		if (!isalpha(long_options[i].val))
> +			continue;
> +		short_options[idx++] = long_options[i].val;
> +		if (long_options[i].has_arg == required_argument)
> +			short_options[idx++] = ':';
> +	}
> +	while (1) {
> +		int option_index = 0;
> +
> +		short_options[idx] = 0;
> +		opt = getopt_long(argc, argv, short_options,
> +				 long_options, &option_index);
> +		if (opt == -1)
> +			break;
> +
> +		params.options[(int)opt] = 1;
> +		switch (opt) {
> +		case OptSetDevice:
> +			strncpy(params.fd_name, optarg, 80);
> +			if (isdigit(optarg[0]) && optarg[1] == 0) {
> +				char newdev[20];
> +				sprintf(newdev, "/dev/radio%c", optarg[0]);
> +				strncpy(params.fd_name, newdev, 20);
> +			}
> +			break;
> +		case OptSetFreq:
> +			params.freq = strtod(optarg, NULL);
> +			break;
> +		case OptListDevices:
> +			print_devices(list_devices());
> +			break;
> +		case OptFreqSeek:
> +			parse_freq_seek(optarg, params.freq_seek);
> +			break;
> +		case OptTunerIndex:
> +			params.tuner_index = strtoul(optarg, NULL, 0);
> +			break;
> +		case OptOpenFile:
> +		{
> +			if (access(optarg, F_OK) != -1) {
> +				params.filemode_active = true;
> +				strncpy(params.fd_name, optarg, 80);
> +			} else {
> +				fprintf(stderr, "Unable to open file: %s\n", optarg);
> +				return -1;
> +			}
> +			/* enable the read-rds option by default for convenience */
> +			params.options[OptReadRds] = 1;
> +			break;
> +		}
> +		case OptWaitLimit:
> +			params.wait_limit = strtoul(optarg, NULL, 0);
> +			break;
> +		case ':':
> +			fprintf(stderr, "Option '%s' requires a value\n",
> +				argv[optind]);
> +			usage_hint();
> +			return 1;
> +		case '?':
> +			if (argv[optind])
> +				fprintf(stderr, "Unknown argument '%s'\n", argv[optind]);
> +			usage_hint();
> +			return 1;
> +		}
> +	}
> +	if (optind < argc) {
> +		printf("unknown arguments: ");
> +		while (optind < argc)
> +			printf("%s ", argv[optind++]);
> +		printf("\n");
> +		usage_hint();
> +		return 1;
> +	}
> +	if (params.options[OptAll]) {
> +		params.options[OptGetDriverInfo] = 1;
> +		params.options[OptGetFreq] = 1;
> +		params.options[OptGetTuner] = 1;
> +		params.options[OptSilent] = 1;
> +	}
> +
> +	return 0;
> +}
> +
> +static void print_driver_info(const struct v4l2_capability *vcap)
> +{
> +
> +	printf("Driver Info (%susing libv4l2):\n",
> +			params.options[OptUseWrapper] ? "" : "not ");
> +	printf("\tDriver name   : %s\n", vcap->driver);
> +	printf("\tCard type     : %s\n", vcap->card);
> +	printf("\tBus info      : %s\n", vcap->bus_info);
> +	printf("\tDriver version: %d.%d.%d\n",
> +			vcap->version >> 16,
> +			(vcap->version >> 8) & 0xff,
> +			vcap->version & 0xff);
> +	printf("\tCapabilities  : 0x%08X\n", vcap->capabilities);
> +	printf("%s", cap2s(vcap->capabilities).c_str());
> +	if (vcap->capabilities & V4L2_CAP_DEVICE_CAPS) {
> +		printf("\tDevice Caps   : 0x%08X\n", vcap->device_caps);
> +		printf("%s", cap2s(vcap->device_caps).c_str());
> +	}
> +}
> +
> +static void set_options(const int fd, const int capabilities, struct v4l2_frequency *vf,
> +			struct v4l2_tuner *tuner)
> +{
> +	int mode = -1;			/* set audio mode */
> +	double fac = 16;		/* factor for frequency division */
> +
> +	if (params.options[OptSetFreq]) {
> +		vf->type = V4L2_TUNER_RADIO;
> +		tuner->index = params.tuner_index;
> +		if (doioctl(fd, VIDIOC_G_TUNER, tuner) == 0) {
> +			fac = (tuner->capability & V4L2_TUNER_CAP_LOW) ? 16000 : 16;
> +			vf->type = tuner->type;
> +		}
> +
> +		vf->tuner = params.tuner_index;
> +		vf->frequency = __u32(params.freq * fac);
> +		if (doioctl(fd, VIDIOC_S_FREQUENCY, vf) == 0)
> +			printf("Frequency for tuner %d set to %d (%f MHz)\n",
> +				vf->tuner, vf->frequency, vf->frequency / fac);
> +	}
> +
> +	if (params.options[OptSetTuner]) {
> +		struct v4l2_tuner vt;
> +
> +		memset(&vt, 0, sizeof(struct v4l2_tuner));
> +		vt.index = params.tuner_index;
> +		if (doioctl(fd, VIDIOC_G_TUNER, &vt) == 0) {
> +			if (mode != -1)
> +				vt.audmode = mode;
> +			doioctl(fd, VIDIOC_S_TUNER, &vt);
> +		}
> +	}
> +
> +	if (params.options[OptFreqSeek]) {
> +		params.freq_seek.tuner = params.tuner_index;
> +		params.freq_seek.type = V4L2_TUNER_RADIO;
> +		doioctl(fd, VIDIOC_S_HW_FREQ_SEEK, &params.freq_seek);
> +	}
> +}
> +
> +static void get_options(const int fd, const int capabilities, struct v4l2_frequency *vf,
> +			struct v4l2_tuner *tuner)
> +{
> +	double fac = 16;		/* factor for frequency division */
> +
> +	if (params.options[OptGetFreq]) {
> +		vf->type = V4L2_TUNER_RADIO;
> +		tuner->index = params.tuner_index;
> +		if (doioctl(fd, VIDIOC_G_TUNER, tuner) == 0) {
> +			fac = (tuner->capability & V4L2_TUNER_CAP_LOW) ? 16000 : 16;
> +			vf->type = tuner->type;
> +		}
> +		vf->tuner = params.tuner_index;
> +		if (doioctl(fd, VIDIOC_G_FREQUENCY, vf) == 0)
> +			printf("Frequency for tuner %d: %d (%f MHz)\n",
> +				   vf->tuner, vf->frequency, vf->frequency / fac);
> +	}
> +
> +	if (params.options[OptGetTuner]) {
> +		struct v4l2_tuner vt;
> +
> +		memset(&vt, 0, sizeof(struct v4l2_tuner));
> +		vt.index = params.tuner_index;
> +		if (doioctl(fd, VIDIOC_G_TUNER, &vt) == 0) {
> +			printf("Tuner %d:\n", vt.index);
> +			printf("\tName                 : %s\n", vt.name);
> +			printf("\tCapabilities         : %s\n",
> +				tcap2s(vt.capability).c_str());
> +			if (vt.capability & V4L2_TUNER_CAP_LOW)
> +				printf("\tFrequency range      : %.1f MHz - %.1f MHz\n",
> +					 vt.rangelow / 16000.0, vt.rangehigh / 16000.0);
> +			else
> +				printf("\tFrequency range      : %.1f MHz - %.1f MHz\n",
> +					 vt.rangelow / 16.0, vt.rangehigh / 16.0);
> +			printf("\tSignal strength/AFC  : %d%%/%d\n",
> +				(int)((vt.signal / 655.35)+0.5), vt.afc);
> +			printf("\tCurrent audio mode   : %s\n", audmode2s(vt.audmode));
> +			printf("\tAvailable subchannels: %s\n",
> +					rxsubchans2s(vt.rxsubchans).c_str());
> +		}
> +	}
> +
> +	if (params.options[OptListFreqBands]) {
> +		struct v4l2_frequency_band band;
> +
> +		memset(&band, 0, sizeof(band));
> +		band.tuner = params.tuner_index;
> +		band.type = V4L2_TUNER_RADIO;
> +		band.index = 0;
> +		printf("ioctl: VIDIOC_ENUM_FREQ_BANDS\n");
> +		while (test_ioctl(fd, VIDIOC_ENUM_FREQ_BANDS, &band) >= 0) {
> +			if (band.index)
> +				printf("\n");
> +			printf("\tIndex          : %d\n", band.index);
> +			printf("\tModulation     : %s\n", modulation2s(band.modulation).c_str());
> +			printf("\tCapability     : %s\n", tcap2s(band.capability).c_str());
> +			if (band.capability & V4L2_TUNER_CAP_LOW)
> +				printf("\tFrequency Range: %.3f MHz - %.3f MHz\n",
> +				     band.rangelow / 16000.0, band.rangehigh / 16000.0);
> +			else
> +				printf("\tFrequency Range: %.3f MHz - %.3f MHz\n",
> +				     band.rangelow / 16.0, band.rangehigh / 16.0);
> +			band.index++;
> +		}
> +	}
> +}
> +
> +int main(int argc, char **argv)
> +{
> +	int fd = -1;
> +
> +	/* command args */
> +	struct v4l2_tuner tuner;	/* set_freq/get_freq */
> +	struct v4l2_capability vcap;	/* list_cap */
> +	struct v4l2_frequency vf;	/* get_freq/set_freq */
> +
> +	memset(&tuner, 0, sizeof(tuner));
> +	memset(&vcap, 0, sizeof(vcap));
> +	memset(&vf, 0, sizeof(vf));
> +	strcpy(params.fd_name, "/dev/radio0");
> +
> +	/* define locale for unicode support */
> +	if (!setlocale(LC_CTYPE, "")) {
> +		fprintf(stderr, "Can't set the specified locale!\n");
> +		return 1;
> +	}
> +	/* register signal handler for interrupt signal, to exit gracefully */
> +	signal(SIGINT, signal_handler_interrupt);
> +
> +	/* try to parse the command line */
> +	parse_cl(argc, argv);
> +	if (params.options[OptHelp]) {
> +		usage();
> +		exit(0);
> +	}
> +
> +	/* File Mode: disables all other features, except for RDS decoding */
> +	if (params.filemode_active) {
> +		if ((fd = open(params.fd_name, O_RDONLY|O_NONBLOCK)) < 0){
> +			perror("error opening file");
> +			exit(1);
> +		}
> +		read_rds_from_fd(fd);
> +		test_close(fd);
> +		exit(0);
> +	}
> +
> +	/* Device Mode: open the radio device as read-only and non-blocking */
> +	if (!params.options[OptSetDevice]) {
> +		/* check the system for RDS capable devices */
> +		dev_vec devices = list_devices();
> +		if (devices.size() == 0) {
> +			fprintf(stderr, "No RDS-capable device found\n");
> +			exit(1);
> +		}
> +		strncpy(params.fd_name, devices[0].c_str(), 80);
> +		printf("Using device: %s\n", params.fd_name);
> +	}
> +	if ((fd = test_open(params.fd_name, O_RDONLY | O_NONBLOCK)) < 0) {
> +		fprintf(stderr, "Failed to open %s: %s\n", params.fd_name,
> +			strerror(errno));
> +		exit(1);
> +	}
> +	doioctl(fd, VIDIOC_QUERYCAP, &vcap);
> +
> +	/* Info options */
> +	if (params.options[OptGetDriverInfo])
> +		print_driver_info(&vcap);
> +	/* Set options */
> +	set_options(fd, vcap.capabilities, &vf, &tuner);
> +	/* Get options */
> +	get_options(fd, vcap.capabilities, &vf, &tuner);
> +	/* RDS decoding */
> +	if (params.options[OptReadRds])
> +		read_rds_from_fd(fd);
> +
> +	test_close(fd);
> +	exit(app_result);
> +}
>


Other then that this looks good to me.

Regards,

Hans

  reply	other threads:[~2012-08-09 12:04 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <[RFC PATCH 0/2] Add support for RDS decoding>
2012-08-07 15:11 ` [RFC PATCH 0/2] Add support for RDS decoding (updated) Konke Radlow
2012-08-07 15:11   ` [RFC PATCH 1/2] Add libv4l2rds library (with changes proposed in RFC) Konke Radlow
2012-08-07 15:11     ` [RFC PATCH 2/2] Add rds-ctl tool " Konke Radlow
2012-08-09 12:05       ` Hans de Goede [this message]
2012-08-09 14:34         ` Konke Radlow
2012-08-09  7:22     ` [RFC PATCH 1/2] Add libv4l2rds library " Gregor Jasny
2012-08-09 14:12       ` Konke Radlow
2012-08-09 11:58     ` Hans de Goede
2012-08-09 12:14       ` Hans Verkuil
2012-08-10  7:16         ` Hans de Goede
2012-08-10  7:36           ` Hans Verkuil
2012-08-10  8:20             ` Hans de Goede
2012-08-09 14:30       ` Konke Radlow
2012-08-09 12:17     ` Hans Verkuil
2012-08-08 12:03   ` [RFC PATCH 0/2] Add support for RDS decoding (updated) Konke Radlow

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5023A770.5080604@redhat.com \
    --to=hdegoede@redhat.com \
    --cc=hverkuil@xs4all.nl \
    --cc=koradlow@gmail.com \
    --cc=kradlow@cisco.com \
    --cc=linux-media@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).