[FFmpeg-devel] avcodec: add a WavPack DSD decoder

[Paul B Mahol]

On Sun, Jul 28, 2019 at 7:18 PM Paul B Mahol <onemda at gmail.com> wrote:

>
>
> On Sun, Jul 28, 2019 at 6:02 AM David Bryant <david at wavpack.com> wrote:
>
>> On 7/24/19 12:26 AM, Paul B Mahol wrote:
>> > On 7/23/19, David Bryant <david at wavpack.com> wrote:
>> >> On 7/23/19 12:47 AM, Paul B Mahol wrote:
>> >>> On 7/23/19, David Bryant <david at wavpack.com> wrote:
>> >>>> On 7/21/19 11:23 PM, Paul B Mahol wrote:
>> >>>>> On 7/22/19, David Bryant <david at wavpack.com> wrote:
>> >>>>>> Hi,
>> >>>>>>
>> >>>>>> As I promised late last year, here is a patch to add a WavPack DSD
>> >>>>>> decoder.
>> >>>>>>
>> >>>>>> Thanks!
>> >>>>>>
>> >>>>>> -David Bryant
>> >>>>>>
>> >>>>>>
>> >>>>> Please correct me if I'm wrong, but why this uses new codec id?
>> >>>>> Apparently is also copies some logic from other files.
>> >>>> Yes, I created a new codec ID for WavPack DSD. It would be possible
>> to
>> >>>> just
>> >>>> piggyback on the existing WavPack codec by silently
>> >>>> decimating the DSD to PCM, but that seemed weird and wrong to me.
>> For one
>> >>>> thing, the user would have no idea that the file was
>> >>>> actually DSD and not high sample-rate PCM.
>> >>>>
>> >>>> Also, since regular WavPack has threading enabled but WavPack DSD
>> can't
>> >>>> (because of the dsd2pcm conversion) I would have to turn
>> >>>> off threading for all WavPack (unless there's some way of making that
>> >>>> conditional at runtime). It would also mean that regular
>> >>>> WavPack would be dependent on the dsd2pcm component even if DSD was
>> not
>> >>>> required (not everyone needs DSD). And of course I was
>> >>>> looking closely at the only other DSD codec in FFmpeg (DST) which
>> has its
>> >>>> own codec ID.
>> >>>>
>> >>>> Because regular WavPack PCM and DSD share the same block format and
>> >>>> metadata
>> >>>> structure, there is a little code that is shared
>> >>>> between the two codecs (although they are no longer identical
>> because of
>> >>>> the
>> >>>> threading difference). Is this a problem? I could
>> >>>> combine the two codecs into one file and sprinkle in a few
>> conditionals,
>> >>>> but
>> >>>> I don't think it would be as clean or clear (but
>> >>>> might save a little duplicate code).
>> >>>>
>> >>>> That's my thinking, but obviously I am not as familiar with the
>> FFmpeg
>> >>>> philosophy as you guys.
>> >>> Looking carefully at dsd2pcm code in your patch, it appears it would
>> >>> work only with 1 or 2 channels.
>> >>> Is this correct?
>> >> Individual WavPack blocks can only be 1 or 2 channels. Multichannel
>> files
>> >> contains sequences of blocks which ends up creating
>> >> multiple frame contexts. This was originally implemented for PCM
>> WavPack
>> >> years ago and so it "just works" for DSD WavPack
>> >> because I didn't touch any of that.
>> >>
>> >> I have tested this extensively with multichannel DSD files and it works
>> >> great, including seeking.
>> > Than I fail to see why it could not work for multi-threading too.
>>
>> Really? Because I just looked back at the FFmpeg devel archives (May
>> 2016) and you supplied the patch for the DST codec, the
>> only other DSD compression codec in existence before my recent
>> development. And dstdec.c does not enable multi-threading
>> because, of course, you get clicks. This seems like an unlikely oversight
>> since DST is a well-documented CPU hog and would
>> benefit greatly from multi-threading.
>>
>
> Well, how you explain fact that you use same dsd context in wavpack for
> different channels.
> IMHO you should use separate context for each channel. Am I missing
> something here?
>

I just looked again at patch, and I see that it is used as I expected it
should be.
So that part is handled fine.