All of lore.kernel.org
 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.