Re: [FFmpeg-devel] [PATCH] swresample/swresample: check for invalid sample rates

2019-05-25 Thread Paul B Mahol
On 5/25/19, Michael Niedermayer  wrote:
> On Fri, May 24, 2019 at 06:05:42PM +0200, Paul B Mahol wrote:
>> Signed-off-by: Paul B Mahol 
>> ---
>>  libswresample/swresample.c | 8 
>>  1 file changed, 8 insertions(+)
>>
>> diff --git a/libswresample/swresample.c b/libswresample/swresample.c
>> index 6d28e6a798..1ac5ef9a30 100644
>> --- a/libswresample/swresample.c
>> +++ b/libswresample/swresample.c
>> @@ -164,6 +164,14 @@ av_cold int swr_init(struct SwrContext *s){
>>  return AVERROR(EINVAL);
>>  }
>>
>> +if(s-> in_sample_rate <= 0){
>> +av_log(s, AV_LOG_ERROR, "Requested input sample rate %d is
>> invalid\n", s->in_sample_rate);
>> +return AVERROR(EINVAL);
>> +}
>> +if(s->out_sample_rate <= 0){
>> +av_log(s, AV_LOG_ERROR, "Requested output sample rate %d is
>> invalid\n", s->out_sample_rate);
>> +return AVERROR(EINVAL);
>> +}
>
> probably ok
>
> Only hypothetical issue i could see is convering sample types or channel
> layout with unspecified sample rate, that is both 0.
> dont know if that works currently but it at least semantically would make
> sense

It does not work, it reach floating-point exception and crashes.
(outlink sample rate set to 0 and input sample rate set to 48000)

>
> Thanks
>
> [...]
> --
> Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> I know you won't believe me, but the highest form of Human Excellence is
> to question oneself and others. -- Socrates
>
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] swresample/swresample: check for invalid sample rates

2019-05-25 Thread Michael Niedermayer
On Sat, May 25, 2019 at 06:45:01PM +0200, Hendrik Leppkes wrote:
> On Sat, May 25, 2019 at 5:58 PM Michael Niedermayer
>  wrote:
> >
> > On Fri, May 24, 2019 at 06:05:42PM +0200, Paul B Mahol wrote:
> > > Signed-off-by: Paul B Mahol 
> > > ---
> > >  libswresample/swresample.c | 8 
> > >  1 file changed, 8 insertions(+)
> > >
> > > diff --git a/libswresample/swresample.c b/libswresample/swresample.c
> > > index 6d28e6a798..1ac5ef9a30 100644
> > > --- a/libswresample/swresample.c
> > > +++ b/libswresample/swresample.c
> > > @@ -164,6 +164,14 @@ av_cold int swr_init(struct SwrContext *s){
> > >  return AVERROR(EINVAL);
> > >  }
> > >
> > > +if(s-> in_sample_rate <= 0){
> > > +av_log(s, AV_LOG_ERROR, "Requested input sample rate %d is 
> > > invalid\n", s->in_sample_rate);
> > > +return AVERROR(EINVAL);
> > > +}
> > > +if(s->out_sample_rate <= 0){
> > > +av_log(s, AV_LOG_ERROR, "Requested output sample rate %d is 
> > > invalid\n", s->out_sample_rate);
> > > +return AVERROR(EINVAL);
> > > +}
> >
> > probably ok
> >
> > Only hypothetical issue i could see is convering sample types or channel
> > layout with unspecified sample rate, that is both 0.
> > dont know if that works currently but it at least semantically would make
> > sense
> >
> 
> Unspecified channel layout can at least make some sense, since you
> still have the channel count to properly handle the audio data, but
> unspecified sample rate? How would that ever work?

you sure can convert 0.5 0.0 1.0 to 128 0 255 you do not need to know
the sample rate.
For a real life example, that would be a raw pcm file, to convert it
from 32bit float to 16bit signed int only that needs to be known
neither the number of channels nor the sample rate is needed

or said differently, for this example, you could set the sample rate
to anything it would not affect the output. So it is basically not
needed for the process

not saying this is worth supporting. Just that it could be supported
and the case at least to me makes sense ...

thanks

[...]
-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Opposition brings concord. Out of discord comes the fairest harmony.
-- Heraclitus


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] swresample/swresample: check for invalid sample rates

2019-05-25 Thread Hendrik Leppkes
On Sat, May 25, 2019 at 5:58 PM Michael Niedermayer
 wrote:
>
> On Fri, May 24, 2019 at 06:05:42PM +0200, Paul B Mahol wrote:
> > Signed-off-by: Paul B Mahol 
> > ---
> >  libswresample/swresample.c | 8 
> >  1 file changed, 8 insertions(+)
> >
> > diff --git a/libswresample/swresample.c b/libswresample/swresample.c
> > index 6d28e6a798..1ac5ef9a30 100644
> > --- a/libswresample/swresample.c
> > +++ b/libswresample/swresample.c
> > @@ -164,6 +164,14 @@ av_cold int swr_init(struct SwrContext *s){
> >  return AVERROR(EINVAL);
> >  }
> >
> > +if(s-> in_sample_rate <= 0){
> > +av_log(s, AV_LOG_ERROR, "Requested input sample rate %d is 
> > invalid\n", s->in_sample_rate);
> > +return AVERROR(EINVAL);
> > +}
> > +if(s->out_sample_rate <= 0){
> > +av_log(s, AV_LOG_ERROR, "Requested output sample rate %d is 
> > invalid\n", s->out_sample_rate);
> > +return AVERROR(EINVAL);
> > +}
>
> probably ok
>
> Only hypothetical issue i could see is convering sample types or channel
> layout with unspecified sample rate, that is both 0.
> dont know if that works currently but it at least semantically would make
> sense
>

Unspecified channel layout can at least make some sense, since you
still have the channel count to properly handle the audio data, but
unspecified sample rate? How would that ever work?

- Hendrik
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] swresample/swresample: check for invalid sample rates

2019-05-25 Thread Michael Niedermayer
On Fri, May 24, 2019 at 06:05:42PM +0200, Paul B Mahol wrote:
> Signed-off-by: Paul B Mahol 
> ---
>  libswresample/swresample.c | 8 
>  1 file changed, 8 insertions(+)
> 
> diff --git a/libswresample/swresample.c b/libswresample/swresample.c
> index 6d28e6a798..1ac5ef9a30 100644
> --- a/libswresample/swresample.c
> +++ b/libswresample/swresample.c
> @@ -164,6 +164,14 @@ av_cold int swr_init(struct SwrContext *s){
>  return AVERROR(EINVAL);
>  }
>  
> +if(s-> in_sample_rate <= 0){
> +av_log(s, AV_LOG_ERROR, "Requested input sample rate %d is 
> invalid\n", s->in_sample_rate);
> +return AVERROR(EINVAL);
> +}
> +if(s->out_sample_rate <= 0){
> +av_log(s, AV_LOG_ERROR, "Requested output sample rate %d is 
> invalid\n", s->out_sample_rate);
> +return AVERROR(EINVAL);
> +}

probably ok

Only hypothetical issue i could see is convering sample types or channel
layout with unspecified sample rate, that is both 0.
dont know if that works currently but it at least semantically would make
sense

Thanks

[...]
-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

I know you won't believe me, but the highest form of Human Excellence is
to question oneself and others. -- Socrates


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] swresample/swresample: check for invalid sample rates

2019-05-24 Thread Paul B Mahol
On 5/24/19, Derek Buitenhuis  wrote:
> On 24/05/2019 17:05, Paul B Mahol wrote:
>> Signed-off-by: Paul B Mahol 
>> ---
>>  libswresample/swresample.c | 8 
>>  1 file changed, 8 insertions(+)
>
> Seems reasonable. What happens if these aren't in place?

It may divide by zero and crash with floating-point exception.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] swresample/swresample: check for invalid sample rates

2019-05-24 Thread Derek Buitenhuis
On 24/05/2019 17:05, Paul B Mahol wrote:
> Signed-off-by: Paul B Mahol 
> ---
>  libswresample/swresample.c | 8 
>  1 file changed, 8 insertions(+)

Seems reasonable. What happens if these aren't in place?

- Derek
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".