[MPlayer-dev-eng] [PATCH] Allows for MMS audio streams to be selected via -alang

[Reimar Döffinger]

Hello,
On Mon, Jan 28, 2008 at 03:44:08AM +0000, Peter Collingbourne wrote:
> This patch allows for the audio language for MMS streams to be selected
> via the -alang option (MMS over HTTP only).


Not a full review...

> @@ -335,6 +338,98 @@

That is a lot of code to add to this block, you should consider using
functions...

> +	char **langs = 0;
> +	int *langs_lengths = 0;

I think using NULL for pointers is preferred.

> +		if (langs_len == 1) {
> +			langs = malloc(sizeof(char *));
> +			langs_lengths = malloc(sizeof(int));
> +		} else {
> +			langs = realloc(langs, langs_len*sizeof(char *));
> +			langs_lengths = realloc(langs_lengths, langs_len*sizeof(int));
> +		}

There is no reason to treat these two cases differently, realloc should
work fine both times.

> +		new_pos = strchr(al_pos, ',');
> +		langs[langs_len-1] = al_pos;
> +		if (new_pos) {
> +			langs_lengths[langs_len-1] = new_pos-al_pos;
> +			al_pos = new_pos+1;
> +		} else {
> +			langs_lengths[langs_len-1] = strlen(al_pos);
> +			al_pos = 0;
> +		}

Using strcspn would probably simplify this.
But either way I think this is a rather bad way to do this, if you do
not care about the corner cases, just
strstr(audio_lang, langname);
will do just fine.
Checking the corner case will be only one line more too.

> +	}
> +	audio_langid_idx = langs_len;
> +		
> +	for (i = 0; i < langcount; i++) {
> +		char langname[64];
> +		int unk = AV_RL8(block+blkpos);
> +		int j;
> +		++blkpos;
> +		for (j = 0; j < 63 && (block[blkpos] || block[blkpos+1]); j++, blkpos+=2) {
> +		  if (block[blkpos+1] != 0) {
> +		    mp_msg(MSGT_NETWORK, MSGL_DBG2, "Warning: language name uses extended character set\n");
> +		  }
> +		  langname[j] = block[blkpos];
> +		}

You could also use AV_RL16 and PUT_UTF8 to support at least some
characters beyond ASCII or iconv to support them all...