| Message ID | 1524119780-21206-1-git-send-email-vinod.koul@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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); > >
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
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.
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, ¶ms);
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(-)