[MPlayer-dev-eng] [PATCH] dvb: accept shorted case-insensitive names for channels

[Alexander Strasser]

Hi,

Roberto Togni wrote:
>  this patch lets MPlayer accept a shorted case-insensitive name for a
>  channel, as long as it's not ambiguous.

  Nice :)

> Eg. if you have two channels named "Abcd 1" and "Abef 5", you can
> select the first one as "dvb://abc".
> 
> If you do "dvb://ab" instead, MPlayer will complain because your
> selection is not unique.
> 
> The exact match always has higher priority, so if you have channels
> named "abc" and "Abc" you can select the second with no problem.
> 
> The patch as is now stops the search as soon as it's clear that the
> input matches more than one channel; maybe it's better to complete the
> scan and report all the names that match the input. Opinions?

  Not sure if it is worth it to do the complete search.  

> Will commit in a few days if no complaints.
[...]
> Index: stream/stream_dvb.c
[...]
> +	if(channel == NULL)
> +	{

  I see you adhere to the surrounding coding style of that file, which
is IMHO good. But...

> +		size_t progname_len = strlen(progname);
> +		int matches = 0;
> +		int j = 0;
> +
> +		mp_msg(MSGT_DEMUX, MSGL_INFO, "DVBIN: channel not found, trying partial match\n");
> +		while((matches < 2) && j < priv->list->NUM_CHANNELS)
> +		{
> +			if(! av_strncasecmp(priv->list->channels[j].name, progname,
> +				FFMIN(progname_len, strlen(priv->list->channels[j].name))))
> +				{
> +					mp_msg(MSGT_DEMUX, MSGL_INFO, "DVBIN: matched %s for %s\n",
> +						priv->list->channels[j].name, progname);
> +					channel = &(priv->list->channels[j]);
> +					i = j + 1;
> +					matches++;
> +				}

  ...I think here the whole if-block is indented 1 tab to much.

> +			j++;
> +		}

  If I am not missing something, rewriting as for-loop would be a little
bit easier to read.

> +
> +		if(matches > 1)
> +		{
> +			mp_msg(MSGT_DEMUX, MSGL_ERR, "DVBIN: multiple matches for %s, add more details\n", progname);
> +			channel = NULL;

  Maybe doing the messaging the other way around would be simpler. E.g.
you just drop the negative message here and say at level MSGL_INFO

  "Selected <xy> via case-insensitive prefix-search (there is no exact match for <x>)"

if we found exactly one match. 

  Then the previous "trying partial match" could probably be dropped too.

  About error message printing, that will be done later anyway because
you set the channel to NULL. I do see that this error message adds a
information though (we did not find an exact match/fuzzy matching found
to many matches).

  Not sure if it is better as it is not exactly equivalent. Chose whatever
you like best.

> +		}
> +	}
> +
>  	if(channel != NULL)
>  	{
>  		priv->list->current = i-1;


  All in all your patch looks good to me! My comments are just nitpicking...


  Alexander