[FFmpeg-devel] [PATCH 1/3] avformat/matroskadec: Add a workaround for missing WavPack extradata

[Andreas Rheinhardt]

David Bryant:
> On 4/1/20 12:53 AM, Andreas Rheinhardt wrote:
>> Andreas Rheinhardt:
>>> mkvmerge versions 6.2 to 40.0 had a bug that made it not propagate the
>>> WavPack extradata (containing the WavPack version) during remuxing from
>>> a Matroska file; currently our demuxer would treat every WavPack block
>>> encountered as invalid data (unless the WavPack stream is to be
>>> discarded (i.e. the streams discard is >= AVDISCARD_ALL)) and try to
>>> resync to the next level 1 element.
>>>
>>> Luckily, the WavPack version is currently not really important; so we
>>> fix this problem by assuming a version. David Bryant, the creator of
>>> WavPack, recommended using version 0x410 (the most recent version) for
>>> this. And this is what this commit does.
>>>
>>> A FATE-test for this has been added.
>>>
>>> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt at gmail.com>
>>> ---
>>> James has already uploaded the sample. Thanks for this.
>>>
>>>  libavformat/matroskadec.c                            | 11 ++++++++++-
>>>  tests/fate/matroska.mak                              |  5 +++++
>>>  tests/ref/fate/matroska-wavpack-missing-codecprivate |  9 +++++++++
>>>  3 files changed, 24 insertions(+), 1 deletion(-)
>>>  create mode 100644 tests/ref/fate/matroska-wavpack-missing-codecprivate
>>>
>>> diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c
>>> index 4d7fdab99f..5b55606b98 100644
>>> --- a/libavformat/matroskadec.c
>>> +++ b/libavformat/matroskadec.c
>>> @@ -2613,6 +2613,14 @@ static int matroska_parse_tracks(AVFormatContext *s)
>>>              ret = matroska_parse_flac(s, track, &extradata_offset);
>>>              if (ret < 0)
>>>                  return ret;
>>> +        } else if (codec_id == AV_CODEC_ID_WAVPACK && track->codec_priv.size < 2) {
>>> +            av_log(matroska->ctx, AV_LOG_INFO, "Assuming WavPack version 4.10 "
>>> +                   "in absence of valid CodecPrivate.\n");
>>> +            extradata_size = 2;
>>> +            extradata = av_mallocz(2 + AV_INPUT_BUFFER_PADDING_SIZE);
>>> +            if (!extradata)
>>> +                return AVERROR(ENOMEM);
>>> +            AV_WL16(extradata, 0x410);
>>>          } else if (codec_id == AV_CODEC_ID_PRORES && track->codec_priv.size == 4) {
>>>              fourcc = AV_RL32(track->codec_priv.data);
>>>          } else if (codec_id == AV_CODEC_ID_VP9 && track->codec_priv.size) {
>>> @@ -3165,9 +3173,10 @@ static int matroska_parse_wavpack(MatroskaTrack *track, uint8_t *src,
>>>      uint16_t ver;
>>>      int ret, offset = 0;
>>>  
>>> -    if (srclen < 12 || track->stream->codecpar->extradata_size < 2)
>>> +    if (srclen < 12)
>>>          return AVERROR_INVALIDDATA;
>>>  
>>> +    av_assert1(track->stream->codecpar->extradata_size >= 2);
>>>      ver = AV_RL16(track->stream->codecpar->extradata);
>>>  
>>>      samples = AV_RL32(src);
>>> diff --git a/tests/fate/matroska.mak b/tests/fate/matroska.mak
>>> index b9ed7322fd..93b5bff89a 100644
>>> --- a/tests/fate/matroska.mak
>>> +++ b/tests/fate/matroska.mak
>>> @@ -17,6 +17,11 @@ fate-matroska-remux: REF = 49a60ef91cf7302ab7276f9373f8a429
>>>  FATE_MATROSKA-$(call ALLYES, MATROSKA_DEMUXER VORBIS_PARSER) += fate-matroska-xiph-lacing
>>>  fate-matroska-xiph-lacing: CMD = framecrc -i $(TARGET_SAMPLES)/mkv/xiph_lacing.mka -c:a copy
>>>  
>>> +# This tests that the Matroska demuxer correctly demuxes WavPack
>>> +# without CodecPrivate; it also tests zlib compressed WavPack.
>>> +FATE_MATROSKA-$(call ALLYES, MATROSKA_DEMUXER ZLIB) += fate-matroska-wavpack-missing-codecprivate
>>> +fate-matroska-wavpack-missing-codecprivate: CMD = framecrc -i $(TARGET_SAMPLES)/mkv/wavpack_missing_codecprivate.mka -c copy
>>> +
>>>  # This tests that the matroska demuxer supports decompressing
>>>  # zlib compressed tracks (both the CodecPrivate as well as the actual frames).
>>>  FATE_MATROSKA-$(call ALLYES, MATROSKA_DEMUXER ZLIB) += fate-matroska-zlib-decompression
>>> diff --git a/tests/ref/fate/matroska-wavpack-missing-codecprivate b/tests/ref/fate/matroska-wavpack-missing-codecprivate
>>> new file mode 100644
>>> index 0000000000..4645a86ff6
>>> --- /dev/null
>>> +++ b/tests/ref/fate/matroska-wavpack-missing-codecprivate
>>> @@ -0,0 +1,9 @@
>>> +#extradata 0:        2, 0x00240014
>>> +#tb 0: 11337/500000000
>>> +#media_type 0: audio
>>> +#codec_id 0: wavpack
>>> +#sample_rate 0: 44100
>>> +#channel_layout 0: 3
>>> +#channel_layout_name 0: stereo
>>> +0,          0,          0,    22051,    14778, 0x02819286
>>> +0,      22051,      22051,    22052,    14756, 0x21976243
>>>
>> Any comments on this patchset? If not, I'd like to apply it tomorrow.
> 
> This looks good to me. Thanks!
> 
> -David
> 
Applied the set, thanks.

- Andreas