[FFmpeg-devel,4/4] lavf/wvdec: remove artificial restrictions on stream parameter changes

They are not forbidden by the specification.
---
 libavformat/wvdec.c | 18 ------------------
 1 file changed, 18 deletions(-)

On 4/5/20 1:32 PM, Anton Khirnov wrote:
> They are not forbidden by the specification.
> ---
>  libavformat/wvdec.c | 18 ------------------
>  1 file changed, 18 deletions(-)
>
> diff --git a/libavformat/wvdec.c b/libavformat/wvdec.c
> index b9fc6a59f9..4159bf1253 100644
> --- a/libavformat/wvdec.c
> +++ b/libavformat/wvdec.c
> @@ -208,24 +208,6 @@ static int wv_read_block_header(AVFormatContext *ctx, AVIOContext *pb)
>      if (!wc->rate)
>          wc->rate   = rate * rate_x;
>  
> -    if (flags && bpp != wc->bpp) {
> -        av_log(ctx, AV_LOG_ERROR,
> -               "Bits per sample differ, this block: %i, header block: %i\n",
> -               bpp, wc->bpp);
> -        return AVERROR_INVALIDDATA;
> -    }
> -    if (flags && !wc->multichannel && chan != wc->chan) {
> -        av_log(ctx, AV_LOG_ERROR,
> -               "Channels differ, this block: %i, header block: %i\n",
> -               chan, wc->chan);
> -        return AVERROR_INVALIDDATA;
> -    }
> -    if (flags && rate != -1 && !(flags & WV_DSD) && rate * rate_x != wc->rate) {
> -        av_log(ctx, AV_LOG_ERROR,
> -               "Sampling rate differ, this block: %i, header block: %i\n",
> -               rate * rate_x, wc->rate);
> -        return AVERROR_INVALIDDATA;
> -    }
>      return 0;
>  }
>  

The specification does not explicitly mention that the data format, the channel count (and layout), and the sample rate cannot
change inside of a WavPack file, but they can't. The first block of a WavPack file that contains audio determines the format of
the entire file, and the documentation will be updated shortly to reflect that. And obviously Konstantin Shishkov was aware of
this, even referring to the first block containing audio as the "header block".

I have no issue with the decoder handling these changes as it might be receiving a WavPack stream from a container that allows
this, but the native WavPack container doesn't. And I actually don't have any problem with this specific change going in because
it doesn't actually break anything. However, it should not be possible to create a WavPack file with format changes such as these.

Quoting David Bryant (2020-04-06 06:56:05)
> On 4/5/20 1:32 PM, Anton Khirnov wrote:
> > They are not forbidden by the specification.
> > ---
> >  libavformat/wvdec.c | 18 ------------------
> >  1 file changed, 18 deletions(-)
> >
> > diff --git a/libavformat/wvdec.c b/libavformat/wvdec.c
> > index b9fc6a59f9..4159bf1253 100644
> > --- a/libavformat/wvdec.c
> > +++ b/libavformat/wvdec.c
> > @@ -208,24 +208,6 @@ static int wv_read_block_header(AVFormatContext *ctx, AVIOContext *pb)
> >      if (!wc->rate)
> >          wc->rate   = rate * rate_x;
> >  
> > -    if (flags && bpp != wc->bpp) {
> > -        av_log(ctx, AV_LOG_ERROR,
> > -               "Bits per sample differ, this block: %i, header block: %i\n",
> > -               bpp, wc->bpp);
> > -        return AVERROR_INVALIDDATA;
> > -    }
> > -    if (flags && !wc->multichannel && chan != wc->chan) {
> > -        av_log(ctx, AV_LOG_ERROR,
> > -               "Channels differ, this block: %i, header block: %i\n",
> > -               chan, wc->chan);
> > -        return AVERROR_INVALIDDATA;
> > -    }
> > -    if (flags && rate != -1 && !(flags & WV_DSD) && rate * rate_x != wc->rate) {
> > -        av_log(ctx, AV_LOG_ERROR,
> > -               "Sampling rate differ, this block: %i, header block: %i\n",
> > -               rate * rate_x, wc->rate);
> > -        return AVERROR_INVALIDDATA;
> > -    }
> >      return 0;
> >  }
> >  
> 
> The specification does not explicitly mention that the data format, the channel count (and layout), and the sample rate cannot
> change inside of a WavPack file, but they can't. The first block of a WavPack file that contains audio determines the format of
> the entire file, and the documentation will be updated shortly to reflect that. And obviously Konstantin Shishkov was aware of
> this, even referring to the first block containing audio as the "header block".
> 
> I have no issue with the decoder handling these changes as it might be receiving a WavPack stream from a container that allows
> this, but the native WavPack container doesn't. And I actually don't have any problem with this specific change going in because
> it doesn't actually break anything. However, it should not be possible to create a WavPack file with format changes such as these.

Okay, then maybe I'll drop this patch. My intent was for it to allow
adding tests for wavpack format changes, but I guess that can also be
done with matroska.