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

[Paul B Mahol]

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?