* [PATCH 2/5] cplay: remove dead code codec_name_from_id()
2018-04-19 6:36 [PATCH 1/5] compress: remove dead code _is_codec_supported() Vinod Koul
@ 2018-04-19 6:36 ` Vinod Koul
2018-04-19 6:36 ` [PATCH 3/5] cplay: remove dead code check_codec_format_supported() Vinod Koul
` (3 subsequent siblings)
4 siblings, 0 replies; 8+ messages in thread
From: Vinod Koul @ 2018-04-19 6:36 UTC (permalink / raw)
To: alsa-devel; +Cc: Vinod Koul, Katsuhiro Suzuki
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
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH 3/5] cplay: remove dead code check_codec_format_supported()
2018-04-19 6:36 [PATCH 1/5] compress: remove dead code _is_codec_supported() Vinod Koul
2018-04-19 6:36 ` [PATCH 2/5] cplay: remove dead code codec_name_from_id() Vinod Koul
@ 2018-04-19 6:36 ` Vinod Koul
2018-04-19 6:36 ` [PATCH 4/5] cplay: fix incorrect print specifier warning Vinod Koul
` (2 subsequent siblings)
4 siblings, 0 replies; 8+ messages in thread
From: Vinod Koul @ 2018-04-19 6:36 UTC (permalink / raw)
To: alsa-devel; +Cc: Vinod Koul, Katsuhiro Suzuki
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
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH 4/5] cplay: fix incorrect print specifier warning
2018-04-19 6:36 [PATCH 1/5] compress: remove dead code _is_codec_supported() Vinod Koul
2018-04-19 6:36 ` [PATCH 2/5] cplay: remove dead code codec_name_from_id() Vinod Koul
2018-04-19 6:36 ` [PATCH 3/5] cplay: remove dead code check_codec_format_supported() Vinod Koul
@ 2018-04-19 6:36 ` Vinod Koul
2018-04-19 6:36 ` [PATCH 5/5] crecord: " Vinod Koul
2018-04-19 7:19 ` [PATCH 1/5] compress: remove dead code _is_codec_supported() Pierre-Louis Bossart
4 siblings, 0 replies; 8+ messages in thread
From: Vinod Koul @ 2018-04-19 6:36 UTC (permalink / raw)
To: alsa-devel; +Cc: 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
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 5/5] crecord: fix incorrect print specifier warning
2018-04-19 6:36 [PATCH 1/5] compress: remove dead code _is_codec_supported() Vinod Koul
` (2 preceding siblings ...)
2018-04-19 6:36 ` [PATCH 4/5] cplay: fix incorrect print specifier warning Vinod Koul
@ 2018-04-19 6:36 ` Vinod Koul
2018-04-19 7:19 ` [PATCH 1/5] compress: remove dead code _is_codec_supported() Pierre-Louis Bossart
4 siblings, 0 replies; 8+ messages in thread
From: Vinod Koul @ 2018-04-19 6:36 UTC (permalink / raw)
To: alsa-devel; +Cc: 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
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH 1/5] compress: remove dead code _is_codec_supported()
2018-04-19 6:36 [PATCH 1/5] compress: remove dead code _is_codec_supported() Vinod Koul
` (3 preceding siblings ...)
2018-04-19 6:36 ` [PATCH 5/5] crecord: " Vinod Koul
@ 2018-04-19 7:19 ` Pierre-Louis Bossart
2018-04-19 8:08 ` Vinod Koul
4 siblings, 1 reply; 8+ messages in thread
From: Pierre-Louis Bossart @ 2018-04-19 7:19 UTC (permalink / raw)
To: Vinod Koul, alsa-devel
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, ¶ms);
>
>
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH 1/5] compress: remove dead code _is_codec_supported()
2018-04-19 7:19 ` [PATCH 1/5] compress: remove dead code _is_codec_supported() Pierre-Louis Bossart
@ 2018-04-19 8:08 ` Vinod Koul
2018-04-20 22:20 ` Pierre-Louis Bossart
0 siblings, 1 reply; 8+ messages in thread
From: Vinod Koul @ 2018-04-19 8:08 UTC (permalink / raw)
To: Pierre-Louis Bossart; +Cc: alsa-devel
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
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH 1/5] compress: remove dead code _is_codec_supported()
2018-04-19 8:08 ` Vinod Koul
@ 2018-04-20 22:20 ` Pierre-Louis Bossart
0 siblings, 0 replies; 8+ messages in thread
From: Pierre-Louis Bossart @ 2018-04-20 22:20 UTC (permalink / raw)
To: Vinod Koul; +Cc: alsa-devel
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.
^ permalink raw reply [flat|nested] 8+ messages in thread