[PATCH 1/5] compress: remove dead code _is_codec_supported()

[PATCH 1/5] compress: remove dead code _is_codec_supported()

[Vinod Koul]

The _is_codec_supported() was dead and none using it, so remove this and
eliminate unused function warning

compress.c:145:13: warning: ‘_is_codec_supported’ defined but not used [-Wunused-function]

We can take from git if user appears

Signed-off-by: Vinod Koul <vinod.koul@intel.com>
---
 src/lib/compress.c | 53 -----------------------------------------------------
 1 file changed, 53 deletions(-)

diff --git a/src/lib/compress.c b/src/lib/compress.c
index 8ae6bbda7a89..934690e39ed0 100644
--- a/src/lib/compress.c
+++ b/src/lib/compress.c
@@ -142,49 +142,6 @@ static int get_compress_version(struct compress *compress)
 	return version;
 }
 
-static bool _is_codec_supported(struct compress *compress, struct compr_config *config,
-				const struct snd_compr_caps *caps)
-{
-	bool codec = false;
-	unsigned int i;
-
-	for (i = 0; i < caps->num_codecs; i++) {
-		if (caps->codecs[i] == config->codec->id) {
-			/* found the codec */
-			codec = true;
-			break;
-		}
-	}
-	if (codec == false) {
-		oops(compress, ENXIO, "this codec is not supported");
-		return false;
-	}
-
-	if (config->fragment_size < caps->min_fragment_size) {
-		oops(compress, EINVAL, "requested fragment size %d is below min supported %d",
-			config->fragment_size, caps->min_fragment_size);
-		return false;
-	}
-	if (config->fragment_size > caps->max_fragment_size) {
-		oops(compress, EINVAL, "requested fragment size %d is above max supported %d",
-			config->fragment_size, caps->max_fragment_size);
-		return false;
-	}
-	if (config->fragments < caps->min_fragments) {
-		oops(compress, EINVAL, "requested fragments %d are below min supported %d",
-			config->fragments, caps->min_fragments);
-		return false;
-	}
-	if (config->fragments > caps->max_fragments) {
-		oops(compress, EINVAL, "requested fragments %d are above max supported %d",
-			config->fragments, caps->max_fragments);
-		return false;
-	}
-
-	/* TODO: match the codec properties */
-	return true;
-}
-
 static bool _is_codec_type_supported(int fd, struct snd_codec *codec)
 {
 	struct snd_compr_caps caps;
@@ -271,16 +228,6 @@ struct compress *compress_open(unsigned int card, unsigned int device,
 		config->fragments = caps.max_fragments;
 	}
 
-#if 0
-	/* FIXME need to turn this On when DSP supports
-	 * and treat in no support case
-	 */
-	if (_is_codec_supported(compress, config, &caps) == false) {
-		oops(compress, errno, "codec not supported\n");
-		goto codec_fail;
-	}
-#endif
-
 	memcpy(compress->config, config, sizeof(*compress->config));
 	fill_compress_params(config, &params);
 
-- 
2.7.4

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

[PATCH 2/5] cplay: remove dead code codec_name_from_id()

[Vinod Koul]

The codec_name_from_id() was added but never used so remove it and
eliminate unused function warning

cplay.c:103:20: warning: ‘codec_name_from_id’ defined but not used [-Wunused-function]

We can take from git if user appears

Signed-off-by: Vinod Koul <vinod.koul@intel.com>
---
CC: Katsuhiro Suzuki <suzuki.katsuhiro@socionext.com>

 src/utils/cplay.c | 14 --------------
 1 file changed, 14 deletions(-)

diff --git a/src/utils/cplay.c b/src/utils/cplay.c
index 98d71a2199da..b02644a3f2ea 100644
--- a/src/utils/cplay.c
+++ b/src/utils/cplay.c
@@ -100,20 +100,6 @@ static const struct {
 };
 #define CPLAY_NUM_CODEC_IDS (sizeof(codec_ids) / sizeof(codec_ids[0]))
 
-static const char *codec_name_from_id(unsigned int id)
-{
-	static char hexname[12];
-	int i;
-
-	for (i = 0; i < CPLAY_NUM_CODEC_IDS; ++i) {
-		if (codec_ids[i].id == id)
-			return codec_ids[i].name;
-	}
-
-	snprintf(hexname, sizeof(hexname), "0x%x", id);
-	return hexname; /* a static is safe because we're single-threaded */
-}
-
 static void usage(void)
 {
 	int i;
-- 
2.7.4

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

[PATCH 3/5] cplay: remove dead code check_codec_format_supported()

[Vinod Koul]

The check_codec_format_supported() was added but never used so remove
it and eliminate unused function warning

cplay.c:103:20: warning: ‘codec_name_from_id’ defined but not used [-Wunused-function]

We can take from git if user appears

Signed-off-by: Vinod Koul <vinod.koul@intel.com>
---
CC: Katsuhiro Suzuki <suzuki.katsuhiro@socionext.com>

 src/utils/cplay.c | 9 ---------
 1 file changed, 9 deletions(-)

diff --git a/src/utils/cplay.c b/src/utils/cplay.c
index b02644a3f2ea..d2c60f976c6c 100644
--- a/src/utils/cplay.c
+++ b/src/utils/cplay.c
@@ -165,15 +165,6 @@ static int parse_mp3_header(struct mp3_header *header, unsigned int *num_channel
 	return 0;
 }
 
-static int check_codec_format_supported(unsigned int card, unsigned int device, struct snd_codec *codec)
-{
-	if (is_codec_supported(card, device, COMPRESS_IN, codec) == false) {
-		fprintf(stderr, "Error: This codec or format is not supported by DSP\n");
-		return -1;
-	}
-	return 0;
-}
-
 static int print_time(struct compress *compress)
 {
 	unsigned int avail;
-- 
2.7.4

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

[PATCH 4/5] cplay: fix incorrect print specifier warning

[Vinod Koul]

We get a warning in cplay for incorrect print specifier, fix it by using
right one %ld for long unsigned int.

cplay.c:327:19: warning: format ‘%d’ expects argument of type ‘int’, but argument 3 has type ‘long unsigned int’ [-Wformat=]
   fprintf(stderr, "codec ID %d is not supported\n", codec_id);

Signed-off-by: Vinod Koul <vinod.koul@intel.com>
---
 src/utils/cplay.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/utils/cplay.c b/src/utils/cplay.c
index d2c60f976c6c..ea36b8771caf 100644
--- a/src/utils/cplay.c
+++ b/src/utils/cplay.c
@@ -324,7 +324,7 @@ void play_samples(char *name, unsigned int card, unsigned int device,
 		get_codec_iec(file, &config, &codec);
 		break;
 	default:
-		fprintf(stderr, "codec ID %d is not supported\n", codec_id);
+		fprintf(stderr, "codec ID %ld is not supported\n", codec_id);
 		exit(EXIT_FAILURE);
 	}
 
-- 
2.7.4

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

[PATCH 5/5] crecord: fix incorrect print specifier warning

[Vinod Koul]

We get a warning in crecord for incorrect print specifier, fix it by using
right one %u for unsigned int

crecord.c:380:17: warning: format ‘%lu’ expects argument of type ‘long unsigned int’, but argument 6 has type ‘unsigned int’ [-Wformat=]
  fprintf(finfo, "Recording file %s On Card %u device %u, with buffer of %lu bytes\n",

Signed-off-by: Vinod Koul <vinod.koul@intel.com>
---
 src/utils/crecord.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/utils/crecord.c b/src/utils/crecord.c
index 8728c5d9a36e..c55bae9f8d6e 100644
--- a/src/utils/crecord.c
+++ b/src/utils/crecord.c
@@ -377,7 +377,7 @@ static void capture_samples(char *name, unsigned int card, unsigned int device,
 		goto comp_exit;
 	}
 
-	fprintf(finfo, "Recording file %s On Card %u device %u, with buffer of %lu bytes\n",
+	fprintf(finfo, "Recording file %s On Card %u device %u, with buffer of %u bytes\n",
 	       name, card, device, size);
 	fprintf(finfo, "Codec %u Format %u Channels %u, %u Hz\n",
 	       codec.id, codec.format, codec.ch_out, rate);
-- 
2.7.4

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [PATCH 1/5] compress: remove dead code _is_codec_supported()

[Pierre-Louis Bossart]

On 4/18/18 11:36 PM, Vinod Koul wrote:
> The _is_codec_supported() was dead and none using it, so remove this and
> eliminate unused function warning
> 
> compress.c:145:13: warning: ‘_is_codec_supported’ defined but not used [-Wunused-function]
> 
> We can take from git if user appears
> 
> Signed-off-by: Vinod Koul <vinod.koul@intel.com>
> ---
>   src/lib/compress.c | 53 -----------------------------------------------------
>   1 file changed, 53 deletions(-)
> 
> diff --git a/src/lib/compress.c b/src/lib/compress.c
> index 8ae6bbda7a89..934690e39ed0 100644
> --- a/src/lib/compress.c
> +++ b/src/lib/compress.c
> @@ -142,49 +142,6 @@ static int get_compress_version(struct compress *compress)
>   	return version;
>   }
>   
> -static bool _is_codec_supported(struct compress *compress, struct compr_config *config,
> -				const struct snd_compr_caps *caps)
> -{
> -	bool codec = false;
> -	unsigned int i;
> -
> -	for (i = 0; i < caps->num_codecs; i++) {
> -		if (caps->codecs[i] == config->codec->id) {
> -			/* found the codec */
> -			codec = true;
> -			break;
> -		}
> -	}
> -	if (codec == false) {
> -		oops(compress, ENXIO, "this codec is not supported");
> -		return false;
> -	}
> -
> -	if (config->fragment_size < caps->min_fragment_size) {
> -		oops(compress, EINVAL, "requested fragment size %d is below min supported %d",
> -			config->fragment_size, caps->min_fragment_size);
> -		return false;
> -	}
> -	if (config->fragment_size > caps->max_fragment_size) {
> -		oops(compress, EINVAL, "requested fragment size %d is above max supported %d",
> -			config->fragment_size, caps->max_fragment_size);
> -		return false;
> -	}
> -	if (config->fragments < caps->min_fragments) {
> -		oops(compress, EINVAL, "requested fragments %d are below min supported %d",
> -			config->fragments, caps->min_fragments);
> -		return false;
> -	}
> -	if (config->fragments > caps->max_fragments) {
> -		oops(compress, EINVAL, "requested fragments %d are above max supported %d",
> -			config->fragments, caps->max_fragments);
> -		return false;
> -	}
> -
> -	/* TODO: match the codec properties */
> -	return true;
> -}
> -
>   static bool _is_codec_type_supported(int fd, struct snd_codec *codec)
>   {
>   	struct snd_compr_caps caps;
> @@ -271,16 +228,6 @@ struct compress *compress_open(unsigned int card, unsigned int device,
>   		config->fragments = caps.max_fragments;
>   	}
>   
> -#if 0
> -	/* FIXME need to turn this On when DSP supports
> -	 * and treat in no support case
> -	 */
> -	if (_is_codec_supported(compress, config, &caps) == false) {
> -		oops(compress, errno, "codec not supported\n");
> -		goto codec_fail;
> -	}
> -#endif

Why was this commented out in the first place?

This seems like a valid check to me. If the application is asking for a 
codec that isn't supported by hardware, should it be allowed to proceed?

> -
>   	memcpy(compress->config, config, sizeof(*compress->config));
>   	fill_compress_params(config, &params);
>   
> 

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [PATCH 1/5] compress: remove dead code _is_codec_supported()

[Vinod Koul]

On Thu, Apr 19, 2018 at 12:19:28AM -0700, Pierre-Louis Bossart wrote:
> On 4/18/18 11:36 PM, Vinod Koul wrote:
> >  static bool _is_codec_type_supported(int fd, struct snd_codec *codec)
> >  {
> >  	struct snd_compr_caps caps;
> >@@ -271,16 +228,6 @@ struct compress *compress_open(unsigned int card, unsigned int device,
> >  		config->fragments = caps.max_fragments;
> >  	}
> >-#if 0
> >-	/* FIXME need to turn this On when DSP supports
> >-	 * and treat in no support case
> >-	 */
> >-	if (_is_codec_supported(compress, config, &caps) == false) {
> >-		oops(compress, errno, "codec not supported\n");
> >-		goto codec_fail;
> >-	}
> >-#endif
> 
> Why was this commented out in the first place?

It depends on capabilities being reported properly which wasn't the case so we
had to turn it off...

> This seems like a valid check to me. If the application is asking for a
> codec that isn't supported by hardware, should it be allowed to proceed?

It has been dead for quite some time, I don't know if ppl are reporting
properly. Turning it on might break which is something I would like to
avoid

-- 
~Vinod

Re: [PATCH 1/5] compress: remove dead code _is_codec_supported()

[Pierre-Louis Bossart]

On 4/19/18 1:08 AM, Vinod Koul wrote:
> On Thu, Apr 19, 2018 at 12:19:28AM -0700, Pierre-Louis Bossart wrote:
>> On 4/18/18 11:36 PM, Vinod Koul wrote:
>>>   static bool _is_codec_type_supported(int fd, struct snd_codec *codec)
>>>   {
>>>   	struct snd_compr_caps caps;
>>> @@ -271,16 +228,6 @@ struct compress *compress_open(unsigned int card, unsigned int device,
>>>   		config->fragments = caps.max_fragments;
>>>   	}
>>> -#if 0
>>> -	/* FIXME need to turn this On when DSP supports
>>> -	 * and treat in no support case
>>> -	 */
>>> -	if (_is_codec_supported(compress, config, &caps) == false) {
>>> -		oops(compress, errno, "codec not supported\n");
>>> -		goto codec_fail;
>>> -	}
>>> -#endif
>>
>> Why was this commented out in the first place?
> 
> It depends on capabilities being reported properly which wasn't the case so we
> had to turn it off...
> 
>> This seems like a valid check to me. If the application is asking for a
>> codec that isn't supported by hardware, should it be allowed to proceed?
> 
> It has been dead for quite some time, I don't know if ppl are reporting
> properly. Turning it on might break which is something I would like to
> avoid

It was broken so it's better to remain broken to avoid breaking things? 
TGIF.